From 0a24276fecd2557aad6e1f1f42919f92f9fb2c4d 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 0c16079fa9..9f2e1665b9 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 );
@@ -646,7 +647,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 1fb71cd1db..c8b4d4f3d6 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 );
@@ -1676,7 +1675,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' } );
@@ -1763,7 +1762,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 =
@@ -1780,11 +1779,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