Attachment 125070 Details for Bug 29059 – Bug 29059: Keep non-repeatable attribute from patron to preserve when merging

[patch] Bug 29059: Keep non-repeatable attribute from patron to preserve when merging

Bug-29059-Keep-non-repeatable-attribute-from-patro.patch (text/plain), 4.17 KB, created by Jonathan Druart on 2021-09-20 15:01:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2021-09-20 15:01:33 UTC

Size: 4.17 KB

patch

obsolete

>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

Actions: View | Diff | Splinter Review

Attachments on bug 29059: 125070 | 131126 | 132485