unless (exists($borrower{$code}) && $borrower{$code} !~ m/^\s*$/ ) {
next;
}
- if (C4::Members::Attributes::CheckUniqueness($code, $borrower{$code}, $borrowernumber)) {
- my $patron = Koha::Patrons->find($borrowernumber);
- if ( $patron ) { # Should not be needed, but we are in C4::Auth LDAP...
+ my $patron = Koha::Patrons->find($borrowernumber);
+ if ( $patron ) { # Should not be needed, but we are in C4::Auth LDAP...
+ eval {
my $attribute = Koha::Patron::Attribute->new({code => $code, attribute => $borrower{$code}});
$patron->extended_attributes([$attribute]);
+ };
+ if ($@) { # FIXME Test if Koha::Exceptions::Patron::Attribute::NonRepeatable
+ warn "ERROR_extended_unique_id_failed $code $borrower{$code}";
}
- } else {
- warn "ERROR_extended_unique_id_failed $code $borrower{$code}";
}
}
}
BEGIN {
@ISA = qw(Exporter);
- @EXPORT_OK = qw(CheckUniqueness
+ @EXPORT_OK = qw(
extended_attributes_code_value_arrayref extended_attributes_merge
SearchIdMatchingAttribute);
%EXPORT_TAGS = ( all => \@EXPORT_OK );
return [map $_->[0], @{ $sth->fetchall_arrayref }];
}
-=head2 CheckUniqueness
-
- my $ok = CheckUniqueness($code, $value[, $borrowernumber]);
-
-Given an attribute type and value, verify if would violate
-a unique_id restriction if added to the patron. The
-optional C<$borrowernumber> is the patron that the attribute
-value would be added to, if known.
-
-Returns false if the C<$code> is not valid or the
-value would violate the uniqueness constraint.
-
-=cut
-
-sub CheckUniqueness {
- my $code = shift;
- my $value = shift;
- my $borrowernumber = @_ ? shift : undef;
-
- my $attr_type = C4::Members::AttributeTypes->fetch($code);
-
- return 0 unless defined $attr_type;
- return 1 unless $attr_type->unique_id();
-
- my $dbh = C4::Context->dbh;
- my $sth;
- if (defined($borrowernumber)) {
- $sth = $dbh->prepare("SELECT COUNT(*)
- FROM borrower_attributes
- WHERE code = ?
- AND attribute = ?
- AND borrowernumber <> ?");
- $sth->execute($code, $value, $borrowernumber);
- } else {
- $sth = $dbh->prepare("SELECT COUNT(*)
- FROM borrower_attributes
- WHERE code = ?
- AND attribute = ?");
- $sth->execute($code, $value);
- }
- my ($count) = $sth->fetchrow_array;
- return ($count == 0);
-}
-
=head2 extended_attributes_code_value_arrayref
my $patron_attributes = "homeroom:1150605,grade:01,extradata:foobar";
my $self = shift;
$self->_check_repeatable;
- $self->_check_unique_id;
+ $self->check_unique_id;
return $self->SUPER::store();
}
return $self;
}
-=head3 _check_unique_id
+=head3 check_unique_id
-_check_unique_id checks if the attribute type is marked as unique id and throws and exception
+check_unique_id checks if the attribute type is marked as unique id and throws and exception
if the attribute type is a unique id and there's already an attribute with the same
code and value on the database.
=cut
-sub _check_unique_id {
+sub check_unique_id {
my $self = shift;
if ( $self->type->unique_id ) {
+ my $params = { code => $self->code, attribute => $self->attribute };
+
+ $params->{borrowernumber} = { '!=' => $self->borrowernumber } if $self->borrowernumber;
+ $params->{id} = { '!=' => $self->id } if $self->in_storage;
+
my $unique_count = Koha::Patron::Attributes
- ->search( { code => $self->code, attribute => $self->attribute } )
+ ->search( $params )
->count;
Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw()
if $unique_count > 0;
}
$debug and warn join "\t", map {"$_: $newdata{$_}"} qw(dateofbirth dateenrolled dateexpiry);
-my $extended_patron_attributes = ();
+my $extended_patron_attributes;
if ($op eq 'save' || $op eq 'insert'){
output_and_exit( $input, $cookie, $template, 'wrong_csrf_token' )
push (@errors, "ERROR_bad_email_alternative") if (!Email::Valid->address($emailalt));
}
- if (C4::Context->preference('ExtendedPatronAttributes')) {
- $extended_patron_attributes = parse_extended_patron_attributes($input);
- foreach my $attr (@$extended_patron_attributes) {
- unless (C4::Members::Attributes::CheckUniqueness($attr->{code}, $attr->{value}, $borrowernumber)) {
- my $attr_info = C4::Members::AttributeTypes->fetch($attr->{code});
- push @errors, "ERROR_extended_unique_id_failed";
- $template->param(
- ERROR_extended_unique_id_failed_code => $attr->{code},
- ERROR_extended_unique_id_failed_value => $attr->{value},
- ERROR_extended_unique_id_failed_description => $attr_info->description()
- );
- }
- }
+ if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
+ $extended_patron_attributes = parse_extended_patron_attributes($input);
+ for my $attr ( @$extended_patron_attributes ) {
+ $attr->{borrowernumber} = $borrowernumber if $borrowernumber;
+ my $attribute = Koha::Patron::Attribute->new($attr);
+ eval {$attribute->check_unique_id};
+ if ( $@ ) {
+ push @errors, "ERROR_extended_unique_id_failed";
+ my $attr_info = C4::Members::AttributeTypes->fetch($attr->{code});
+ $template->param(
+ ERROR_extended_unique_id_failed_code => $attr->{code},
+ ERROR_extended_unique_id_failed_value => $attr->{attribute},
+ ERROR_extended_unique_id_failed_description => $attr_info->description()
+ );
+ }
+ }
}
}
}
}
- if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
- $patron->extended_attributes->filter_by_branch_limitations->delete;
- $patron->extended_attributes($extended_patron_attributes);
- }
if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) {
C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template, 1, $newdata{'categorycode'});
}
}
add_guarantors( $patron, $input );
- if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
- $patron->extended_attributes->filter_by_branch_limitations->delete;
- $patron->extended_attributes($extended_patron_attributes);
- }
if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) {
C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template);
}
}
+ if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
+ $patron->extended_attributes->filter_by_branch_limitations->delete;
+ $patron->extended_attributes($extended_patron_attributes);
+ }
+
if ( $destination eq 'circ' and not C4::Auth::haspermission( C4::Context->userenv->{id}, { circulate => 'circulate_remaining_permissions' } ) ) {
# If we want to redirect to circulation.pl and need to check if the logged in user has the necessary permission
$destination = 'not_circ';
output_html_with_http_headers $input, $cookie, $template->output;
-sub parse_extended_patron_attributes {
+sub parse_extended_patron_attributes {
my ($input) = @_;
my @patron_attr = grep { /^patron_attr_\d+$/ } $input->multi_param();
my $code = $input->param("${key}_code");
next if exists $dups{$code}->{$value};
$dups{$code}->{$value} = 1;
- push @attr, { code => $code, value => $value };
+ push @attr, { code => $code, attribute => $value };
}
return \@attr;
}
$template->param(no_patron_attribute_types => 1);
return;
}
- my $patron = Koha::Patrons->find($borrowernumber); # Already fetched but outside of this sub
- my @attributes = $patron->extended_attributes->as_list; # FIXME Must be improved!
- my @classes = uniq( map {$_->type->class} @attributes );
- @classes = sort @classes;
+ my @attributes;
+ if ( $borrowernumber ) {
+ my $patron = Koha::Patrons->find($borrowernumber); # Already fetched but outside of this sub
+ @attributes = $patron->extended_attributes->as_list; # FIXME Must be improved!
+ }
# map patron's attributes into a more convenient structure
my %attr_hash = ();
foreach my $attr (@attributes) {
- push @{ $attr_hash{$attr->{code}} }, $attr;
+ push @{ $attr_hash{$attr->code} }, $attr;
}
my @attribute_loop = ();
if (exists $attr_hash{$attr_type->code()}) {
foreach my $attr (@{ $attr_hash{$attr_type->code()} }) {
my $newentry = { %$entry };
- $newentry->{value} = $attr->{value};
+ $newentry->{value} = $attr->attribute;
$newentry->{use_dropdown} = 0;
if ($attr_type->authorised_value_category()) {
$newentry->{use_dropdown} = 1;
- $newentry->{auth_val_loop} = GetAuthorisedValues($attr_type->authorised_value_category(), $attr->{value});
+ $newentry->{auth_val_loop} = GetAuthorisedValues($attr_type->authorised_value_category(), $attr->attribute);
}
$i++;
undef $newentry->{value} if ($attr_type->unique_id() && $op eq 'duplicate');
my $conflicting_attribute = 0;
foreach my $attr (@$attributes) {
- unless ( C4::Members::Attributes::CheckUniqueness($attr->{code}, $attr->{value}, $borrowernumber) ) {
+ my $attribute = Koha::Patron::Attribute->new($attr);
+ eval {$attribute->check_unique_id};
+ if ( $@ ) {
my $attr_info = C4::Members::AttributeTypes->fetch($attr->{code});
$template->param(
extended_unique_id_failed_code => $attr->{code},
- extended_unique_id_failed_value => $attr->{value},
+ extended_unique_id_failed_value => $attr->{attribute},
extended_unique_id_failed_description => $attr_info->description()
);
$conflicting_attribute = 1;
}
else {
# we've got a value
- push @attributes, { code => $code, value => $value };
+ push @attributes, { code => $code, attribute => $value };
# 'code' is no longer a delete candidate
delete $delete_candidates->{$code}
if ( Koha::Patron::Attributes->search({
borrowernumber => $borrowernumber, code => $code })->count > 0 )
{
- push @attributes, { code => $code, value => '' }
+ push @attributes, { code => $code, attribute => '' }
unless any { $_->{code} eq $code } @attributes;
}
}
use t::lib::TestBuilder;
use t::lib::Mocks;
-use Test::More tests => 39;
+use Test::Exception;
+use Test::More tests => 33;
use_ok('C4::Members::Attributes');
is( $attr_0->attribute, $attribute->{attribute}, 'delete then add a new attribute updates the field value correctly' );
-my $check_uniqueness = C4::Members::Attributes::CheckUniqueness();
-is( $check_uniqueness, 0, 'CheckUniqueness without arguments returns false' );
-$check_uniqueness = C4::Members::Attributes::CheckUniqueness($attribute->{code});
-is( $check_uniqueness, 1, 'CheckUniqueness with a valid argument code returns true' );
-$check_uniqueness = C4::Members::Attributes::CheckUniqueness(undef, $attribute->{attribute});
-is( $check_uniqueness, 0, 'CheckUniqueness without the argument code returns false' );
-$check_uniqueness = C4::Members::Attributes::CheckUniqueness('my invalid code');
-is( $check_uniqueness, 0, 'CheckUniqueness with an invalid argument code returns false' );
-$check_uniqueness = C4::Members::Attributes::CheckUniqueness('my invalid code', $attribute->{attribute});
-is( $check_uniqueness, 0, 'CheckUniqueness with an invalid argument code returns fale' );
-$check_uniqueness = C4::Members::Attributes::CheckUniqueness($attribute->{code}, 'new value');
-is( $check_uniqueness, 1, 'CheckUniqueness with a new value returns true' );
-$check_uniqueness = C4::Members::Attributes::CheckUniqueness('my invalid code', 'new value');
-is( $check_uniqueness, 0, 'CheckUniqueness with an invalid argument code and a new value returns false' );
-$check_uniqueness = C4::Members::Attributes::CheckUniqueness($attributes->[1]->{code}, $attributes->[1]->{attribute});
-is( $check_uniqueness, 1, 'CheckUniqueness with an attribute unique_id=0 returns true' );
-$check_uniqueness = C4::Members::Attributes::CheckUniqueness($attribute->{code}, $attribute->{attribute});
-is( $check_uniqueness, '', 'CheckUniqueness returns false' );
+lives_ok { # Editing, new value, same patron
+ Koha::Patron::Attribute->new(
+ {
+ code => $attribute->{code},
+ attribute => 'new value',
+ borrowernumber => $patron->borrowernumber
+ }
+ )->check_unique_id;
+} 'no exception raised';
+lives_ok { # Editing, same value, same patron
+ Koha::Patron::Attribute->new(
+ {
+ code => $attributes->[1]->{code},
+ attribute => $attributes->[1]->{attribute},
+ borrowernumber => $patron->borrowernumber
+ }
+ )->check_unique_id;
+} 'no exception raised';
+throws_ok { # Creating a new one, but already exists!
+ Koha::Patron::Attribute->new(
+ { code => $attribute->{code}, attribute => $attribute->{attribute} } )
+ ->check_unique_id;
+} 'Koha::Exceptions::Patron::Attribute::UniqueIDConstraint';
my $borrower_numbers = C4::Members::Attributes::SearchIdMatchingAttribute('attribute1');