From 7db0b4ef519f75404de3fb000b675909e230dd78 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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