From 847e3c9cd76aba19340e3d767bfcd536741f8fb4 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.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
---
 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 9968d36350..9589c1740a 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 );
@@ -645,7 +646,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 e7c49fc14e..0fc7b23b5b 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 );
@@ -1688,7 +1687,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' } );
@@ -1775,7 +1774,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 =
@@ -1792,11 +1791,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.30.2