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

(-)a/Koha/Patron/Attributes.pm (-4 / +5 lines)
Lines 96-103 $new_attributes is an arrayref of hashrefs Link Here
96
sub merge_and_replace_with {
96
sub merge_and_replace_with {
97
    my ( $self, $new_attributes ) = @_;
97
    my ( $self, $new_attributes ) = @_;
98
98
99
    my @merged = @{$self->unblessed};
99
    my @existing_attributes = @{$self->unblessed};
100
    my $attribute_types = { map { $_->code => $_->unblessed } Koha::Patron::Attribute::Types->search };
100
    my $attribute_types = { map { $_->code => $_->unblessed } Koha::Patron::Attribute::Types->search };
101
    my @new_attributes;
101
    for my $attr ( @$new_attributes ) {
102
    for my $attr ( @$new_attributes ) {
102
103
103
        my $attribute_type = $attribute_types->{$attr->{code}};
104
        my $attribute_type = $attribute_types->{$attr->{code}};
Lines 107-119 sub merge_and_replace_with { Link Here
107
108
108
        unless ( $attribute_type->{repeatable} ) {
109
        unless ( $attribute_type->{repeatable} ) {
109
            # filter out any existing attributes of the same code
110
            # filter out any existing attributes of the same code
110
            @merged = grep {$attr->{code} ne $_->{code}} @merged;
111
            @existing_attributes = grep {$attr->{code} ne $_->{code}} @existing_attributes;
111
        }
112
        }
112
113
113
        push @merged, $attr;
114
        push @new_attributes, $attr;
114
    }
115
    }
115
116
116
    @merged = map { { code => $_->{code}, attribute => $_->{attribute} } } @merged;
117
    my @merged = map { { code => $_->{code}, attribute => $_->{attribute} } } ( @existing_attributes, @new_attributes );
117
118
118
    # WARNING - we would like to return a set, but $new_attributes is not in storage yet
119
    # WARNING - we would like to return a set, but $new_attributes is not in storage yet
119
    # Maybe there is something obvious I (JD) am missing
120
    # Maybe there is something obvious I (JD) am missing
(-)a/Koha/Patrons/Import.pm (-71 / +85 lines)
Lines 241-246 sub import_patrons { Link Here
241
              if exists $borrower{$field} and $borrower{$field} eq "";
241
              if exists $borrower{$field} and $borrower{$field} eq "";
242
        }
242
        }
243
243
244
        my $success = 1;
244
        if ($borrowernumber) {
245
        if ($borrowernumber) {
245
246
246
            # borrower exists
247
            # borrower exists
Lines 270-278 sub import_patrons { Link Here
270
            }
271
            }
271
272
272
            my $patron = Koha::Patrons->find( $borrowernumber );
273
            my $patron = Koha::Patrons->find( $borrowernumber );
273
            eval { $patron->set(\%borrower)->store };
274
            try {
274
            if ( $@ ) {
275
                $schema->storage->txn_do(sub {
276
                    $patron->set(\%borrower)->store;
277
                    # Don't add a new restriction if the existing 'combined' restriction matches this one
278
                    if ( $borrower{debarred} && ( ( $borrower{debarred} ne $member->{debarred} ) || ( $borrower{debarredcomment} ne $member->{debarredcomment} ) ) ) {
279
280
                        # Check to see if this debarment already exists
281
                        my $debarrments = GetDebarments(
282
                            {
283
                                borrowernumber => $borrowernumber,
284
                                expiration     => $borrower{debarred},
285
                                comment        => $borrower{debarredcomment}
286
                            }
287
                        );
288
289
                        # If it doesn't, then add it!
290
                        unless (@$debarrments) {
291
                            AddDebarment(
292
                                {
293
                                    borrowernumber => $borrowernumber,
294
                                    expiration     => $borrower{debarred},
295
                                    comment        => $borrower{debarredcomment}
296
                                }
297
                            );
298
                        }
299
                    }
300
                    if ($patron->category->category_type ne 'S' && $overwrite_passwords && defined $borrower{password} && $borrower{password} ne ''){
301
                        try {
302
                            $patron->set_password({ password => $borrower{password} });
303
                        }
304
                        catch {
305
                            if ( $_->isa('Koha::Exceptions::Password::TooShort') ) {
306
                                push @errors, { passwd_too_short => 1, borrowernumber => $borrowernumber, length => $_->{length}, min_length => $_->{min_length} };
307
                            }
308
                            elsif ( $_->isa('Koha::Exceptions::Password::WhitespaceCharacters') ) {
309
                                push @errors, { passwd_whitespace => 1, borrowernumber => $borrowernumber } ;
310
                            }
311
                            elsif ( $_->isa('Koha::Exceptions::Password::TooWeak') ) {
312
                                push @errors, { passwd_too_weak => 1, borrowernumber => $borrowernumber } ;
313
                            }
314
                            elsif ( $_->isa('Koha::Exceptions::Password::Plugin') ) {
315
                                push @errors, { passwd_plugin_err => 1, borrowernumber => $borrowernumber } ;
316
                            }
317
                            else {
318
                                push @errors, { passwd_unknown_err => 1, borrowernumber => $borrowernumber } ;
319
                            }
320
                        }
321
                    }
322
                    if ($extended) {
323
                        if ($ext_preserve) {
324
                            $patron_attributes = $patron->extended_attributes->merge_and_replace_with( $patron_attributes );
325
                        }
326
                        # We do not want to filter by branch, maybe we should?
327
                        Koha::Patrons->find($borrowernumber)->extended_attributes->delete;
328
                        $patron->extended_attributes($patron_attributes);
329
                    }
330
                    $overwritten++;
331
                    push(
332
                        @feedback,
333
                        {
334
                            feedback => 1,
335
                            name     => 'lastoverwritten',
336
                            value    => $borrower{'surname'} . ' / ' . $borrowernumber
337
                        }
338
                    );
339
                });
340
            } catch {
275
                $invalid++;
341
                $invalid++;
342
                $success = 0;
343
344
                my $patron_id = defined $matchpoint ? $borrower{$matchpoint} : $matchpoint_attr_type;
345
                if ( $_->isa('Koha::Exceptions::Patron::Attribute::UniqueIDConstraint') ) {
346
                    push @errors, { patron_attribute_unique_id_constraint => 1, patron_id => $patron_id, attribute => $_->attribute };
347
                } elsif ( $_->isa('Koha::Exceptions::Patron::Attribute::InvalidType') ) {
348
                    push @errors, { patron_attribute_invalid_type => 1, patron_id => $patron_id, attribute_type_code => $_->type };
349
                } elsif ( $_->isa('Koha::Exceptions::Patron::Attribute::NonRepeatable') ) {
350
                    push @errors, { patron_attribute_non_repeatable => 1, patron_id => $patron_id, attribute => $_->attribute };
351
                } else {
352
                    use Data::Printer colored => 1; warn p $_;
353
                    push @errors, { unknown_error => 1 };
354
                }
276
355
277
                push(
356
                push(
278
                    @errors,
357
                    @errors,
Lines 282-357 sub import_patrons { Link Here
282
                        value => $borrower{'surname'} . ' / ' . $borrowernumber
361
                        value => $borrower{'surname'} . ' / ' . $borrowernumber
283
                    }
362
                    }
284
                );
363
                );
285
                next LINE;
286
            }
287
            # Don't add a new restriction if the existing 'combined' restriction matches this one
288
            if ( $borrower{debarred} && ( ( $borrower{debarred} ne $member->{debarred} ) || ( $borrower{debarredcomment} ne $member->{debarredcomment} ) ) ) {
289
290
                # Check to see if this debarment already exists
291
                my $debarrments = GetDebarments(
292
                    {
293
                        borrowernumber => $borrowernumber,
294
                        expiration     => $borrower{debarred},
295
                        comment        => $borrower{debarredcomment}
296
                    }
297
                );
298
299
                # If it doesn't, then add it!
300
                unless (@$debarrments) {
301
                    AddDebarment(
302
                        {
303
                            borrowernumber => $borrowernumber,
304
                            expiration     => $borrower{debarred},
305
                            comment        => $borrower{debarredcomment}
306
                        }
307
                    );
308
                }
309
            }
364
            }
310
            if ($patron->category->category_type ne 'S' && $overwrite_passwords && defined $borrower{password} && $borrower{password} ne ''){
311
                try {
312
                    $patron->set_password({ password => $borrower{password} });
313
                }
314
                catch {
315
                    if ( $_->isa('Koha::Exceptions::Password::TooShort') ) {
316
                        push @errors, { passwd_too_short => 1, borrowernumber => $borrowernumber, length => $_->{length}, min_length => $_->{min_length} };
317
                    }
318
                    elsif ( $_->isa('Koha::Exceptions::Password::WhitespaceCharacters') ) {
319
                        push @errors, { passwd_whitespace => 1, borrowernumber => $borrowernumber } ;
320
                    }
321
                    elsif ( $_->isa('Koha::Exceptions::Password::TooWeak') ) {
322
                        push @errors, { passwd_too_weak => 1, borrowernumber => $borrowernumber } ;
323
                    }
324
                    elsif ( $_->isa('Koha::Exceptions::Password::Plugin') ) {
325
                        push @errors, { passwd_plugin_err => 1, borrowernumber => $borrowernumber } ;
326
                    }
327
                    else {
328
                        push @errors, { passwd_unknown_err => 1, borrowernumber => $borrowernumber } ;
329
                    }
330
                }
331
            }
332
            if ($extended) {
333
                if ($ext_preserve) {
334
                    $patron_attributes = $patron->extended_attributes->merge_and_replace_with( $patron_attributes );
335
                }
336
                eval {
337
                    # We do not want to filter by branch, maybe we should?
338
                    Koha::Patrons->find($borrowernumber)->extended_attributes->delete;
339
                    $patron->extended_attributes($patron_attributes);
340
                };
341
                if ($@) {
342
                    # FIXME This is not an unknown error, we can do better here
343
                    push @errors, { unknown_error => 1 };
344
                }
345
            }
346
            $overwritten++;
347
            push(
348
                @feedback,
349
                {
350
                    feedback => 1,
351
                    name     => 'lastoverwritten',
352
                    value    => $borrower{'surname'} . ' / ' . $borrowernumber
353
                }
354
            );
355
        }
365
        }
356
        else {
366
        else {
357
            try {
367
            try {
Lines 397-402 sub import_patrons { Link Here
397
                });
407
                });
398
            } catch {
408
            } catch {
399
                $invalid++;
409
                $invalid++;
410
                $success = 0;
400
                my $patron_id = defined $matchpoint ? $borrower{$matchpoint} : $matchpoint_attr_type;
411
                my $patron_id = defined $matchpoint ? $borrower{$matchpoint} : $matchpoint_attr_type;
401
                if ( $_->isa('Koha::Exceptions::Patron::Attribute::UniqueIDConstraint') ) {
412
                if ( $_->isa('Koha::Exceptions::Patron::Attribute::UniqueIDConstraint') ) {
402
                    push @errors, { patron_attribute_unique_id_constraint => 1, patron_id => $patron_id, attribute => $_->attribute };
413
                    push @errors, { patron_attribute_unique_id_constraint => 1, patron_id => $patron_id, attribute => $_->attribute };
Lines 406-411 sub import_patrons { Link Here
406
                    push @errors, { patron_attribute_non_repeatable => 1, patron_id => $patron_id, attribute => $_->attribute };
417
                    push @errors, { patron_attribute_non_repeatable => 1, patron_id => $patron_id, attribute => $_->attribute };
407
418
408
                } else {
419
                } else {
420
                    use Data::Printer colored => 1; warn p $_;
409
                    push @errors, { unknown_error => 1 };
421
                    push @errors, { unknown_error => 1 };
410
                }
422
                }
411
                push(
423
                push(
Lines 418-423 sub import_patrons { Link Here
418
            };
430
            };
419
        }
431
        }
420
432
433
        next LINE unless $success;
434
421
        # Add a guarantor if we are given a relationship
435
        # Add a guarantor if we are given a relationship
422
        if ( $guarantor_id ) {
436
        if ( $guarantor_id ) {
423
            my $relationship = Koha::Patron::Relationships->find(
437
            my $relationship = Koha::Patron::Relationships->find(
(-)a/t/db_dependent/Koha/Patrons/Import.t (-4 / +68 lines)
Lines 969-975 subtest 'patron_attributes' => sub { Link Here
969
    }
969
    }
970
970
971
    subtest 'update existing patron' => sub {
971
    subtest 'update existing patron' => sub {
972
        plan tests => 6;
972
        plan tests => 19;
973
973
974
        my $patron = $builder->build_object(
974
        my $patron = $builder->build_object(
975
            {
975
            {
Lines 1016-1023 subtest 'patron_attributes' => sub { Link Here
1016
        # The normal_attribute_type has been replaced with 'my normal attribute 2'
1016
        # The normal_attribute_type has been replaced with 'my normal attribute 2'
1017
        compare_patron_attributes($patron->extended_attributes->unblessed, { %$attributes, %$new_attributes } );
1017
        compare_patron_attributes($patron->extended_attributes->unblessed, { %$attributes, %$new_attributes } );
1018
1018
1019
1019
        # UniqueIDConstraint
1020
        # uniqueness
1021
        $patron->extended_attributes->delete; # reset
1020
        $patron->extended_attributes->delete; # reset
1022
        $builder->build_object(
1021
        $builder->build_object(
1023
            {
1022
            {
Lines 1041-1046 subtest 'patron_attributes' => sub { Link Here
1041
        );
1040
        );
1042
1041
1043
        is( $result->{overwritten}, 0 );
1042
        is( $result->{overwritten}, 0 );
1043
        my $error = $result->{errors}->[0];
1044
        is( $error->{patron_attribute_unique_id_constraint}, 1 );
1045
        is( $error->{patron_id}, $cardnumber );
1046
        is( $error->{attribute}->code, $unique_attribute_type->code );
1047
1048
        compare_patron_attributes($patron->extended_attributes->unblessed, {},  );
1049
1050
1051
        #InvalidType
1052
        $attributes = {
1053
            $non_existent_attribute_type_code => ['my non-existent attribute'],
1054
            $normal_attribute_type->code      => ['my attribute 1'],
1055
        };
1056
        $fh = build_csv({ %$attributes });
1057
1058
        $result = $patrons_import->import_patrons(
1059
            {
1060
                file                         => $fh,
1061
                matchpoint                   => 'cardnumber',
1062
                overwrite_cardnumber         => 1,
1063
                preserve_extended_attributes => 1
1064
            }
1065
        );
1066
        is( $result->{overwritten}, 0 );
1067
1068
        $error = $result->{errors}->[0];
1069
        is( $error->{patron_attribute_invalid_type}, 1 );
1070
        is( $error->{patron_id}, $cardnumber );
1071
        is( $error->{attribute_type_code}, $non_existent_attribute_type_code );
1072
1073
        # NonRepeatable
1074
        $attributes = {
1075
                $repeatable_attribute_type->code => ['my repeatable attribute 1', 'my repeatable attribute 2'],
1076
                $normal_attribute_type->code     => ['my normal attribute 1', 'my normal attribute 2'],
1077
            };
1078
        $fh = build_csv({ %$attributes });
1079
        $result = $patrons_import->import_patrons(
1080
            {
1081
                file                         => $fh,
1082
                matchpoint                   => 'cardnumber',
1083
                overwrite_cardnumber         => 1,
1084
                preserve_extended_attributes => 1
1085
            }
1086
        );
1087
        is( $result->{overwritten}, 0 );
1088
1089
        $error = $result->{errors}->[0];
1090
        is( $error->{patron_attribute_non_repeatable}, 1 );
1091
        is( $error->{patron_id}, $cardnumber );
1092
        is( $error->{attribute}->code, $normal_attribute_type->code );
1093
1094
        # Don't preserve existing attributes
1095
        $attributes = {
1096
                $repeatable_attribute_type->code => ['my repeatable attribute 3', 'my repeatable attribute 4'],
1097
                $normal_attribute_type->code     => ['my normal attribute 1'],
1098
            };
1099
        $fh = build_csv({ %$attributes });
1100
        $result = $patrons_import->import_patrons(
1101
            {
1102
                file                         => $fh,
1103
                matchpoint                   => 'cardnumber',
1104
                overwrite_cardnumber         => 1,
1105
                preserve_extended_attributes => 1
1106
            }
1107
        );
1108
        is( $result->{overwritten}, 1 );
1044
1109
1045
        compare_patron_attributes($patron->extended_attributes->unblessed, { %$attributes } );
1110
        compare_patron_attributes($patron->extended_attributes->unblessed, { %$attributes } );
1046
1111
1047
- 

Return to bug 28220