View | Details | Raw Unified | Return to bug 20443
Collapse All | Expand All

(-)a/C4/Auth_with_ldap.pm (-5 / +6 lines)
Lines 238-251 sub checkpw_ldap { Link Here
238
            unless (exists($borrower{$code}) && $borrower{$code} !~ m/^\s*$/ ) {
238
            unless (exists($borrower{$code}) && $borrower{$code} !~ m/^\s*$/ ) {
239
                next;
239
                next;
240
            }
240
            }
241
            if (C4::Members::Attributes::CheckUniqueness($code, $borrower{$code}, $borrowernumber)) {
241
            my $patron = Koha::Patrons->find($borrowernumber);
242
                my $patron = Koha::Patrons->find($borrowernumber);
242
            if ( $patron ) { # Should not be needed, but we are in C4::Auth LDAP...
243
                if ( $patron ) { # Should not be needed, but we are in C4::Auth LDAP...
243
                eval {
244
                    my $attribute = Koha::Patron::Attribute->new({code => $code, attribute => $borrower{$code}});
244
                    my $attribute = Koha::Patron::Attribute->new({code => $code, attribute => $borrower{$code}});
245
                    $patron->extended_attributes([$attribute]);
245
                    $patron->extended_attributes([$attribute]);
246
                };
247
                if ($@) { # FIXME Test if Koha::Exceptions::Patron::Attribute::NonRepeatable
248
                    warn "ERROR_extended_unique_id_failed $code $borrower{$code}";
246
                }
249
                }
247
            } else {
248
                warn "ERROR_extended_unique_id_failed $code $borrower{$code}";
249
            }
250
            }
250
        }
251
        }
251
    }
252
    }
(-)a/C4/Members/Attributes.pm (-45 / +1 lines)
Lines 29-35 our ($csv, $AttributeTypes); Link Here
29
29
30
BEGIN {
30
BEGIN {
31
    @ISA = qw(Exporter);
31
    @ISA = qw(Exporter);
32
    @EXPORT_OK = qw(CheckUniqueness
32
    @EXPORT_OK = qw(
33
                    extended_attributes_code_value_arrayref extended_attributes_merge
33
                    extended_attributes_code_value_arrayref extended_attributes_merge
34
                    SearchIdMatchingAttribute);
34
                    SearchIdMatchingAttribute);
35
    %EXPORT_TAGS = ( all => \@EXPORT_OK );
35
    %EXPORT_TAGS = ( all => \@EXPORT_OK );
Lines 67-116 AND (} . join (" OR ", map "attribute like ?", @$filter) .qq{)}; Link Here
67
    return [map $_->[0], @{ $sth->fetchall_arrayref }];
67
    return [map $_->[0], @{ $sth->fetchall_arrayref }];
68
}
68
}
69
69
70
=head2 CheckUniqueness
71
72
  my $ok = CheckUniqueness($code, $value[, $borrowernumber]);
73
74
Given an attribute type and value, verify if would violate
75
a unique_id restriction if added to the patron.  The
76
optional C<$borrowernumber> is the patron that the attribute
77
value would be added to, if known.
78
79
Returns false if the C<$code> is not valid or the
80
value would violate the uniqueness constraint.
81
82
=cut
83
84
sub CheckUniqueness {
85
    my $code = shift;
86
    my $value = shift;
87
    my $borrowernumber = @_ ? shift : undef;
88
89
    my $attr_type = C4::Members::AttributeTypes->fetch($code);
90
91
    return 0 unless defined $attr_type;
92
    return 1 unless $attr_type->unique_id();
93
94
    my $dbh = C4::Context->dbh;
95
    my $sth;
96
    if (defined($borrowernumber)) {
97
        $sth = $dbh->prepare("SELECT COUNT(*) 
98
                              FROM borrower_attributes 
99
                              WHERE code = ? 
100
                              AND attribute = ?
101
                              AND borrowernumber <> ?");
102
        $sth->execute($code, $value, $borrowernumber);
103
    } else {
104
        $sth = $dbh->prepare("SELECT COUNT(*) 
105
                              FROM borrower_attributes 
106
                              WHERE code = ? 
107
                              AND attribute = ?");
108
        $sth->execute($code, $value);
109
    }
110
    my ($count) = $sth->fetchrow_array;
111
    return ($count == 0);
112
}
113
114
=head2 extended_attributes_code_value_arrayref 
70
=head2 extended_attributes_code_value_arrayref 
115
71
116
   my $patron_attributes = "homeroom:1150605,grade:01,extradata:foobar";
72
   my $patron_attributes = "homeroom:1150605,grade:01,extradata:foobar";
(-)a/Koha/Patron/Attribute.pm (-5 / +10 lines)
Lines 47-53 sub store { Link Here
47
    my $self = shift;
47
    my $self = shift;
48
48
49
    $self->_check_repeatable;
49
    $self->_check_repeatable;
50
    $self->_check_unique_id;
50
    $self->check_unique_id;
51
51
52
    return $self->SUPER::store();
52
    return $self->SUPER::store();
53
}
53
}
Lines 139-159 sub _check_repeatable { Link Here
139
    return $self;
139
    return $self;
140
}
140
}
141
141
142
=head3 _check_unique_id
142
=head3 check_unique_id
143
143
144
_check_unique_id checks if the attribute type is marked as unique id and throws and exception
144
check_unique_id checks if the attribute type is marked as unique id and throws and exception
145
if the attribute type is a unique id and there's already an attribute with the same
145
if the attribute type is a unique id and there's already an attribute with the same
146
code and value on the database.
146
code and value on the database.
147
147
148
=cut
148
=cut
149
149
150
sub _check_unique_id {
150
sub check_unique_id {
151
151
152
    my $self = shift;
152
    my $self = shift;
153
153
154
    if ( $self->type->unique_id ) {
154
    if ( $self->type->unique_id ) {
155
        my $params = { code => $self->code, attribute => $self->attribute };
156
157
        $params->{borrowernumber} = { '!=' => $self->borrowernumber } if $self->borrowernumber;
158
        $params->{id}             = { '!=' => $self->id }             if $self->in_storage;
159
155
        my $unique_count = Koha::Patron::Attributes
160
        my $unique_count = Koha::Patron::Attributes
156
            ->search( { code => $self->code, attribute => $self->attribute } )
161
            ->search( $params )
157
            ->count;
162
            ->count;
158
        Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw()
163
        Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw()
159
            if $unique_count > 0;
164
            if $unique_count > 0;
(-)a/members/memberentry.pl (-31 / +32 lines)
Lines 312-318 if ( ( defined $newdata{'userid'} && $newdata{'userid'} eq '' ) || $check_Borrow Link Here
312
}
312
}
313
  
313
  
314
$debug and warn join "\t", map {"$_: $newdata{$_}"} qw(dateofbirth dateenrolled dateexpiry);
314
$debug and warn join "\t", map {"$_: $newdata{$_}"} qw(dateofbirth dateenrolled dateexpiry);
315
my $extended_patron_attributes = ();
315
my $extended_patron_attributes;
316
if ($op eq 'save' || $op eq 'insert'){
316
if ($op eq 'save' || $op eq 'insert'){
317
317
318
    output_and_exit( $input, $cookie, $template,  'wrong_csrf_token' )
318
    output_and_exit( $input, $cookie, $template,  'wrong_csrf_token' )
Lines 398-416 if ($op eq 'save' || $op eq 'insert'){ Link Here
398
      push (@errors, "ERROR_bad_email_alternative") if (!Email::Valid->address($emailalt));
398
      push (@errors, "ERROR_bad_email_alternative") if (!Email::Valid->address($emailalt));
399
  }
399
  }
400
400
401
  if (C4::Context->preference('ExtendedPatronAttributes')) {
401
  if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
402
    $extended_patron_attributes = parse_extended_patron_attributes($input);
402
      $extended_patron_attributes = parse_extended_patron_attributes($input);
403
    foreach my $attr (@$extended_patron_attributes) {
403
      for my $attr ( @$extended_patron_attributes ) {
404
        unless (C4::Members::Attributes::CheckUniqueness($attr->{code}, $attr->{value}, $borrowernumber)) {
404
          $attr->{borrowernumber} = $borrowernumber if $borrowernumber;
405
            my $attr_info = C4::Members::AttributeTypes->fetch($attr->{code});
405
          my $attribute = Koha::Patron::Attribute->new($attr);
406
            push @errors, "ERROR_extended_unique_id_failed";
406
          eval {$attribute->check_unique_id};
407
            $template->param(
407
          if ( $@ ) {
408
                ERROR_extended_unique_id_failed_code => $attr->{code},
408
              push @errors, "ERROR_extended_unique_id_failed";
409
                ERROR_extended_unique_id_failed_value => $attr->{value},
409
              my $attr_info = C4::Members::AttributeTypes->fetch($attr->{code});
410
                ERROR_extended_unique_id_failed_description => $attr_info->description()
410
              $template->param(
411
            );
411
                  ERROR_extended_unique_id_failed_code => $attr->{code},
412
        }
412
                  ERROR_extended_unique_id_failed_value => $attr->{attribute},
413
    }
413
                  ERROR_extended_unique_id_failed_description => $attr_info->description()
414
              );
415
          }
416
      }
414
  }
417
  }
415
}
418
}
416
419
Lines 479-488 if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ Link Here
479
            }
482
            }
480
        }
483
        }
481
484
482
        if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
483
            $patron->extended_attributes->filter_by_branch_limitations->delete;
484
            $patron->extended_attributes($extended_patron_attributes);
485
        }
486
        if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) {
485
        if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) {
487
            C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template, 1, $newdata{'categorycode'});
486
            C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template, 1, $newdata{'categorycode'});
488
        }
487
        }
Lines 550-564 if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ Link Here
550
        }
549
        }
551
550
552
        add_guarantors( $patron, $input );
551
        add_guarantors( $patron, $input );
553
        if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
554
            $patron->extended_attributes->filter_by_branch_limitations->delete;
555
            $patron->extended_attributes($extended_patron_attributes);
556
        }
557
        if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) {
552
        if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) {
558
            C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template);
553
            C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template);
559
        }
554
        }
560
	}
555
	}
561
556
557
    if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
558
        $patron->extended_attributes->filter_by_branch_limitations->delete;
559
        $patron->extended_attributes($extended_patron_attributes);
560
    }
561
562
    if ( $destination eq 'circ' and not C4::Auth::haspermission( C4::Context->userenv->{id}, { circulate => 'circulate_remaining_permissions' } ) ) {
562
    if ( $destination eq 'circ' and not C4::Auth::haspermission( C4::Context->userenv->{id}, { circulate => 'circulate_remaining_permissions' } ) ) {
563
        # If we want to redirect to circulation.pl and need to check if the logged in user has the necessary permission
563
        # If we want to redirect to circulation.pl and need to check if the logged in user has the necessary permission
564
        $destination = 'not_circ';
564
        $destination = 'not_circ';
Lines 832-838 if ( C4::Context->preference('TranslateNotices') ) { Link Here
832
832
833
output_html_with_http_headers $input, $cookie, $template->output;
833
output_html_with_http_headers $input, $cookie, $template->output;
834
834
835
sub  parse_extended_patron_attributes {
835
sub parse_extended_patron_attributes {
836
    my ($input) = @_;
836
    my ($input) = @_;
837
    my @patron_attr = grep { /^patron_attr_\d+$/ } $input->multi_param();
837
    my @patron_attr = grep { /^patron_attr_\d+$/ } $input->multi_param();
838
838
Lines 844-850 sub parse_extended_patron_attributes { Link Here
844
        my $code     = $input->param("${key}_code");
844
        my $code     = $input->param("${key}_code");
845
        next if exists $dups{$code}->{$value};
845
        next if exists $dups{$code}->{$value};
846
        $dups{$code}->{$value} = 1;
846
        $dups{$code}->{$value} = 1;
847
        push @attr, { code => $code, value => $value };
847
        push @attr, { code => $code, attribute => $value };
848
    }
848
    }
849
    return \@attr;
849
    return \@attr;
850
}
850
}
Lines 859-873 sub patron_attributes_form { Link Here
859
        $template->param(no_patron_attribute_types => 1);
859
        $template->param(no_patron_attribute_types => 1);
860
        return;
860
        return;
861
    }
861
    }
862
    my $patron = Koha::Patrons->find($borrowernumber); # Already fetched but outside of this sub
862
    my @attributes;
863
    my @attributes = $patron->extended_attributes->as_list; # FIXME Must be improved!
863
    if ( $borrowernumber ) {
864
    my @classes = uniq( map {$_->type->class} @attributes );
864
        my $patron = Koha::Patrons->find($borrowernumber); # Already fetched but outside of this sub
865
    @classes = sort @classes;
865
        @attributes = $patron->extended_attributes->as_list; # FIXME Must be improved!
866
    }
866
867
867
    # map patron's attributes into a more convenient structure
868
    # map patron's attributes into a more convenient structure
868
    my %attr_hash = ();
869
    my %attr_hash = ();
869
    foreach my $attr (@attributes) {
870
    foreach my $attr (@attributes) {
870
        push @{ $attr_hash{$attr->{code}} }, $attr;
871
        push @{ $attr_hash{$attr->code} }, $attr;
871
    }
872
    }
872
873
873
    my @attribute_loop = ();
874
    my @attribute_loop = ();
Lines 886-896 sub patron_attributes_form { Link Here
886
        if (exists $attr_hash{$attr_type->code()}) {
887
        if (exists $attr_hash{$attr_type->code()}) {
887
            foreach my $attr (@{ $attr_hash{$attr_type->code()} }) {
888
            foreach my $attr (@{ $attr_hash{$attr_type->code()} }) {
888
                my $newentry = { %$entry };
889
                my $newentry = { %$entry };
889
                $newentry->{value} = $attr->{value};
890
                $newentry->{value} = $attr->attribute;
890
                $newentry->{use_dropdown} = 0;
891
                $newentry->{use_dropdown} = 0;
891
                if ($attr_type->authorised_value_category()) {
892
                if ($attr_type->authorised_value_category()) {
892
                    $newentry->{use_dropdown} = 1;
893
                    $newentry->{use_dropdown} = 1;
893
                    $newentry->{auth_val_loop} = GetAuthorisedValues($attr_type->authorised_value_category(), $attr->{value});
894
                    $newentry->{auth_val_loop} = GetAuthorisedValues($attr_type->authorised_value_category(), $attr->attribute);
894
                }
895
                }
895
                $i++;
896
                $i++;
896
                undef $newentry->{value} if ($attr_type->unique_id() && $op eq 'duplicate');
897
                undef $newentry->{value} if ($attr_type->unique_id() && $op eq 'duplicate');
(-)a/opac/opac-memberentry.pl (-4 / +6 lines)
Lines 98-108 my $attributes = ParsePatronAttributes($borrowernumber,$cgi); Link Here
98
my $conflicting_attribute = 0;
98
my $conflicting_attribute = 0;
99
99
100
foreach my $attr (@$attributes) {
100
foreach my $attr (@$attributes) {
101
    unless ( C4::Members::Attributes::CheckUniqueness($attr->{code}, $attr->{value}, $borrowernumber) ) {
101
    my $attribute = Koha::Patron::Attribute->new($attr);
102
    eval {$attribute->check_unique_id};
103
    if ( $@ ) {
102
        my $attr_info = C4::Members::AttributeTypes->fetch($attr->{code});
104
        my $attr_info = C4::Members::AttributeTypes->fetch($attr->{code});
103
        $template->param(
105
        $template->param(
104
            extended_unique_id_failed_code => $attr->{code},
106
            extended_unique_id_failed_code => $attr->{code},
105
            extended_unique_id_failed_value => $attr->{value},
107
            extended_unique_id_failed_value => $attr->{attribute},
106
            extended_unique_id_failed_description => $attr_info->description()
108
            extended_unique_id_failed_description => $attr_info->description()
107
        );
109
        );
108
        $conflicting_attribute = 1;
110
        $conflicting_attribute = 1;
Lines 664-670 sub ParsePatronAttributes { Link Here
664
            }
666
            }
665
            else {
667
            else {
666
                # we've got a value
668
                # we've got a value
667
                push @attributes, { code => $code, value => $value };
669
                push @attributes, { code => $code, attribute => $value };
668
670
669
                # 'code' is no longer a delete candidate
671
                # 'code' is no longer a delete candidate
670
                delete $delete_candidates->{$code}
672
                delete $delete_candidates->{$code}
Lines 677-683 sub ParsePatronAttributes { Link Here
677
        if ( Koha::Patron::Attributes->search({
679
        if ( Koha::Patron::Attributes->search({
678
                borrowernumber => $borrowernumber, code => $code })->count > 0 )
680
                borrowernumber => $borrowernumber, code => $code })->count > 0 )
679
        {
681
        {
680
            push @attributes, { code => $code, value => '' }
682
            push @attributes, { code => $code, attribute => '' }
681
                unless any { $_->{code} eq $code } @attributes;
683
                unless any { $_->{code} eq $code } @attributes;
682
        }
684
        }
683
    }
685
    }
(-)a/t/db_dependent/Members/Attributes.t (-20 / +25 lines)
Lines 26-32 use Koha::Database; Link Here
26
use t::lib::TestBuilder;
26
use t::lib::TestBuilder;
27
use t::lib::Mocks;
27
use t::lib::Mocks;
28
28
29
use Test::More tests => 39;
29
use Test::Exception;
30
use Test::More tests => 33;
30
31
31
use_ok('C4::Members::Attributes');
32
use_ok('C4::Members::Attributes');
32
33
Lines 150-173 is( $attr_0->type->description, $attribute_type1->description(), 'delete then ad Link Here
150
is( $attr_0->attribute, $attribute->{attribute}, 'delete then add a new attribute updates the field value correctly' );
151
is( $attr_0->attribute, $attribute->{attribute}, 'delete then add a new attribute updates the field value correctly' );
151
152
152
153
153
my $check_uniqueness = C4::Members::Attributes::CheckUniqueness();
154
lives_ok { # Editing, new value, same patron
154
is( $check_uniqueness, 0, 'CheckUniqueness without arguments returns false' );
155
    Koha::Patron::Attribute->new(
155
$check_uniqueness = C4::Members::Attributes::CheckUniqueness($attribute->{code});
156
        {
156
is( $check_uniqueness, 1, 'CheckUniqueness with a valid argument code returns true' );
157
            code           => $attribute->{code},
157
$check_uniqueness = C4::Members::Attributes::CheckUniqueness(undef, $attribute->{attribute});
158
            attribute      => 'new value',
158
is( $check_uniqueness, 0, 'CheckUniqueness without the argument code returns false' );
159
            borrowernumber => $patron->borrowernumber
159
$check_uniqueness = C4::Members::Attributes::CheckUniqueness('my invalid code');
160
        }
160
is( $check_uniqueness, 0, 'CheckUniqueness with an invalid argument code returns false' );
161
    )->check_unique_id;
161
$check_uniqueness = C4::Members::Attributes::CheckUniqueness('my invalid code', $attribute->{attribute});
162
} 'no exception raised';
162
is( $check_uniqueness, 0, 'CheckUniqueness with an invalid argument code returns fale' );
163
lives_ok { # Editing, same value, same patron
163
$check_uniqueness = C4::Members::Attributes::CheckUniqueness($attribute->{code}, 'new value');
164
    Koha::Patron::Attribute->new(
164
is( $check_uniqueness, 1, 'CheckUniqueness with a new value returns true' );
165
        {
165
$check_uniqueness = C4::Members::Attributes::CheckUniqueness('my invalid code', 'new value');
166
            code           => $attributes->[1]->{code},
166
is( $check_uniqueness, 0, 'CheckUniqueness with an invalid argument code and a new value returns false' );
167
            attribute      => $attributes->[1]->{attribute},
167
$check_uniqueness = C4::Members::Attributes::CheckUniqueness($attributes->[1]->{code}, $attributes->[1]->{attribute});
168
            borrowernumber => $patron->borrowernumber
168
is( $check_uniqueness, 1, 'CheckUniqueness with an attribute unique_id=0 returns true' );
169
        }
169
$check_uniqueness = C4::Members::Attributes::CheckUniqueness($attribute->{code}, $attribute->{attribute});
170
    )->check_unique_id;
170
is( $check_uniqueness, '', 'CheckUniqueness returns false' );
171
} 'no exception raised';
172
throws_ok { # Creating a new one, but already exists!
173
    Koha::Patron::Attribute->new(
174
        { code => $attribute->{code}, attribute => $attribute->{attribute} } )
175
      ->check_unique_id;
176
} 'Koha::Exceptions::Patron::Attribute::UniqueIDConstraint';
171
177
172
178
173
my $borrower_numbers = C4::Members::Attributes::SearchIdMatchingAttribute('attribute1');
179
my $borrower_numbers = C4::Members::Attributes::SearchIdMatchingAttribute('attribute1');
174
- 

Return to bug 20443