From 7db0b4ef519f75404de3fb000b675909e230dd78 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 20 Sep 2021 16:59:37 +0200 Subject: [PATCH] Bug 29059: Keep non-repeatable attribute from patron to preserve when merging See bug 21648 comment 17. Suggestion is to keep the non-repeatable patron's attribute from the patron we selected instead of raising a blocking error. A side-effect will be that when several patrons are merged, the non-repeatable attribute from the first one will be kept, which can result in unexpected result if the original patron does not have the attribute defined. --- Koha/Patron.pm | 10 +++++++++- t/db_dependent/Koha/Patrons.t | 29 +++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 941dace0df8..7eb7ff39bdb 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -23,6 +23,7 @@ use Modern::Perl; use List::MoreUtils qw( any uniq ); use JSON qw( to_json ); use Unicode::Normalize qw( NFKD ); +use Try::Tiny; use C4::Context; use C4::Log qw( logaction ); @@ -634,7 +635,14 @@ sub merge_with { ]; $attributes->delete; # We need to delete before trying to merge them to prevent exception on unique and repeatable for my $attribute ( @$new_attributes ) { - $self->add_extended_attribute($attribute); + try { + $self->add_extended_attribute($attribute); + } catch { + # Don't block the merge if there is a non-repeatable attribute that cannot be added to the current patron. + unless ( $_->isa('Koha::Exceptions::Patron::Attribute::NonRepeatable') ) { + $_->rethrow; + } + }; } while (my ($r, $field) = each(%$RESULTSET_PATRON_ID_MAPPING)) { diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 152b80c35b9..68704147dae 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -26,7 +26,6 @@ use Test::MockModule; use Time::Fake; use DateTime; use JSON; -use Data::Dumper; use utf8; use C4::Circulation qw( AddIssue AddReturn ); @@ -1658,7 +1657,7 @@ subtest 'Test Koha::Patrons::merge' => sub { is( Koha::Patrons->search( { borrowernumber => $keeper->id } )->count, 1, "Patron from attempted merge with AnonymousPatron still exists" ); subtest 'extended attributes' => sub { - plan tests => 5; + plan tests => 8; my $keep_patron = $builder->build_object( { class => 'Koha::Patrons' } ); @@ -1745,7 +1744,7 @@ subtest 'Test Koha::Patrons::merge' => sub { $keep_patron->delete; $merge_patron->delete; - # Recreate but expect an exception because 2 "normal" attributes will be in the resulting patron + # Recreate but don't expect an exception if 2 non-repeatable attributes exist, pick the one from the patron we keep $keep_patron = $builder->build_object( { class => 'Koha::Patrons' } ); $merge_patron = @@ -1762,11 +1761,25 @@ subtest 'Test Koha::Patrons::merge' => sub { ] ); - throws_ok { - $keep_patron->merge_with( [ $merge_patron->borrowernumber ] ); - } - 'Koha::Exceptions::Patron::Attribute::NonRepeatable', - 'Exception thrown trying to merge several non-repeatable attributes'; + $keep_patron->merge_with( [ $merge_patron->borrowernumber ] ); + $merged_attributes = $keep_patron->extended_attributes; + is( $merged_attributes->count, 4 ); + compare_attributes( + $merged_attributes, + ['from attr 1'], + $attribute_type_normal_1->code + ); + compare_attributes( + $merged_attributes, + ['to attr 2'], + $attribute_type_normal_2->code + ); + compare_attributes( + $merged_attributes, + [ 'from attr repeatable', 'to attr repeatable' ], + $attribute_type_repeatable->code + ); + }; t::lib::Mocks::mock_preference( 'AnonymousPatron', '' ); -- 2.25.1