From 9ce0dd6b9225328e49098b69201767f240b47270 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Fri, 20 Mar 2020 10:58:54 +0100
Subject: [PATCH] Bug 20443: Fix merge_with behavior

merge_with were returning Koha::Patron::Attribute for existing attribute
and hashref for new attribute.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 Koha/Patron/Attributes.pm     |  4 +--
 t/db_dependent/Koha/Patrons.t | 61 ++++++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/Koha/Patron/Attributes.pm b/Koha/Patron/Attributes.pm
index 0e03c32972..8acef02fac 100644
--- a/Koha/Patron/Attributes.pm
+++ b/Koha/Patron/Attributes.pm
@@ -96,7 +96,7 @@ $new_attributes is an arrayref of hashrefs
 sub merge_with {
     my ( $self, $new_attributes ) = @_;
 
-    my @merged = $self->as_list;
+    my @merged = @{$self->unblessed};
     my $attribute_types = { map { $_->code => $_->unblessed } Koha::Patron::Attribute::Types->search };
     for my $attr ( @$new_attributes ) {
         unless ( $attr->{code} ) {
@@ -112,7 +112,7 @@ sub merge_with {
         }
         unless ( $attribute_type->{repeatable} ) {
             # filter out any existing attributes of the same code
-            @merged = grep {$attr->{code} ne $_->code} @merged;
+            @merged = grep {$attr->{code} ne $_->{code}} @merged;
         }
 
         push @merged, $attr;
diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t
index a6ab9dc680..0c5bb31246 100644
--- a/t/db_dependent/Koha/Patrons.t
+++ b/t/db_dependent/Koha/Patrons.t
@@ -2021,7 +2021,7 @@ subtest 'anonymize' => sub {
 $schema->storage->txn_rollback;
 
 subtest 'extended_attributes' => sub {
-    plan tests => 11;
+    plan tests => 14;
     my $schema = Koha::Database->new->schema;
     $schema->storage->txn_begin;
 
@@ -2046,6 +2046,8 @@ subtest 'extended_attributes' => sub {
         }
     )->store;
 
+    my $attribute_type3 = $builder->build_object({ class => 'Koha::Patron::Attribute::Types' });
+
     my $deleted_attribute_type = $builder->build_object({ class => 'Koha::Patron::Attribute::Types' });
     my $deleted_attribute_type_code = $deleted_attribute_type->code;
     $deleted_attribute_type->delete;
@@ -2112,6 +2114,63 @@ subtest 'extended_attributes' => sub {
     $attribute_12 = $patron_2->get_extended_attribute( $attribute_type1->code );
     is( $attribute_12->attribute, 'my attribute12', 'Koha::Patron->get_extended_attribute should return the correct attribute value' );
 
+    warning_is {
+        $extended_attributes_for_2 = $patron_2->extended_attributes->merge_with(
+            [
+                {
+                    attribute => 'my attribute12 XXX',
+                    code      => $attribute_type1->code(),
+                },
+                {
+                    attribute => 'my nonexistent attribute 2',
+                    code      => $deleted_attribute_type_code,
+                },
+                {
+                    attribute => 'my attribute 3', # Adding a new attribute using merge_with
+                    code      => $attribute_type3->code,
+                },
+            ]
+        );
+    }
+    "Cannot merge element: unrecognized code = '$deleted_attribute_type_code'",
+    "Trying to merge_with using a nonexistent attribute code should display a warning";
+
+    is( @$extended_attributes_for_2, 3, 'There should be 3 attributes now for patron 3');
+    my $expected_attributes_for_2 = [
+        {
+            code      => $attribute_type1->code(),
+            attribute => 'my attribute12 XXX',
+        },
+        {
+            code      => $attribute_type_limited->code(),
+            attribute => 'my attribute limited 2',
+        },
+        {
+            attribute => 'my attribute 3',
+            code      => $attribute_type3->code,
+        },
+    ];
+    # Sorting them by code
+    $expected_attributes_for_2 = [ sort { $a->{code} cmp $b->{code} } @$expected_attributes_for_2 ];
+
+    is_deeply(
+        [
+            {
+                code      => $extended_attributes_for_2->[0]->{code},
+                attribute => $extended_attributes_for_2->[0]->{attribute}
+            },
+            {
+                code      => $extended_attributes_for_2->[1]->{code},
+                attribute => $extended_attributes_for_2->[1]->{attribute}
+            },
+            {
+                code      => $extended_attributes_for_2->[2]->{code},
+                attribute => $extended_attributes_for_2->[2]->{attribute}
+            },
+        ],
+        $expected_attributes_for_2
+    );
+
     # TODO - What about multiple? POD explains the problem
     my $non_existent = $patron_2->get_extended_attribute( 'not_exist' );
     is( $non_existent, undef, 'Koha::Patron->get_extended_attribute must return undef if the attribute does not exist' );
-- 
2.39.5