From ac592cde9c7cfb1cf42bfa70cdedc0aafd7f1da4 Mon Sep 17 00:00:00 2001
From: David Gustafsson <david.gustafsson@ub.gu.se>
Date: Thu, 27 Feb 2025 17:01:25 +0100
Subject: [PATCH] Bug 26744: Log changes also in modborrowers.pl and when
 calling add_extended_attribute

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
---
 Koha/Patron.pm                         | 25 +++++++++++++++++--
 t/db_dependent/Koha/Patron/Attribute.t | 28 +++++++++++++++++++++-
 tools/modborrowers.pl                  | 33 ++++++++++++++------------
 3 files changed, 68 insertions(+), 18 deletions(-)

diff --git a/Koha/Patron.pm b/Koha/Patron.pm
index ee93d9b355..4ae67541a2 100644
--- a/Koha/Patron.pm
+++ b/Koha/Patron.pm
@@ -2237,13 +2237,34 @@ sub _generate_userid_internal {    # as we always did
 sub add_extended_attribute {
     my ( $self, $attribute ) = @_;
 
-    return Koha::Patron::Attribute->new(
+    my $change;
+    if (C4::Context->preference("BorrowersLog")) {
+        my @attribute_values_before = map { $_->attribute }
+            $self->extended_attributes->search({ 'me.code' => $attribute->{code} })->as_list;
+        my @attribute_values_after = sort ($attribute->{attribute}, @attribute_values_before);
+        $change = {
+            before => \@attribute_values_before,
+            after => \@attribute_values_after
+        }
+    }
+
+    my $added_attribute =  Koha::Patron::Attribute->new(
         {
-            %$attribute,
+            %{$attribute},
             ( borrowernumber => $self->borrowernumber ),
         }
     )->store;
 
+    if (C4::Context->preference("BorrowersLog")) {
+        logaction(
+            "MEMBERS",
+            "MODIFY",
+            $self->borrowernumber,
+            "Patron attribute " . $attribute->{code} . ": " . to_json($change, { pretty => 1, canonical => 1 })
+        );
+    }
+
+    return $added_attribute;
 }
 
 =head3 extended_attributes
diff --git a/t/db_dependent/Koha/Patron/Attribute.t b/t/db_dependent/Koha/Patron/Attribute.t
index 3817f0877f..487dfce9fe 100755
--- a/t/db_dependent/Koha/Patron/Attribute.t
+++ b/t/db_dependent/Koha/Patron/Attribute.t
@@ -524,7 +524,7 @@ subtest 'merge_and_replace_with' => sub {
 };
 
 subtest 'action log tests' => sub {
-    plan tests => 11;
+    plan tests => 12;
     my $schema = Koha::Database->new->schema;
     $schema->storage->txn_begin;
 
@@ -741,5 +741,31 @@ subtest 'action log tests' => sub {
         "New action log entry has been created when updating repeatable patron attribute with existing multiple values with multiple values"
     );
 
+    my $attribute = {
+        attribute => 'Qux',
+        code => $attribute_type->code,
+    };
+    $patron->add_extended_attribute($attribute);
+
+    $info = $get_info->(
+        ['Foo', 'Bar', 'Baz'],
+        ['Foo', 'Bar', 'Baz', 'Qux'],
+        $attribute_type->code,
+        1
+    );
+    $action_logs = Koha::ActionLogs->search(
+        {
+            module => "MEMBERS",
+            action => "MODIFY",
+            object => $patron->borrowernumber,
+            info => $info
+        }
+    );
+    is(
+        $action_logs->count,
+        1,
+        "New action log entry has been created when updating patron attributes using add_extended_attribute"
+    );
+
     $schema->storage->txn_rollback;
 };
diff --git a/tools/modborrowers.pl b/tools/modborrowers.pl
index 9e1830ddb2..e6608a15bc 100755
--- a/tools/modborrowers.pl
+++ b/tools/modborrowers.pl
@@ -40,6 +40,7 @@ use Koha::Patron::Debarments qw( AddDebarment DelDebarment );
 use Koha::Patrons;
 use List::MoreUtils qw(uniq);
 use Koha::Patron::Messages;
+use Try::Tiny;
 
 my $input = CGI->new;
 my $op    = $input->param('op') || 'show_form';
@@ -428,6 +429,9 @@ if ( $op eq 'cud-do' ) {
             $attributes->{$code}->{disabled} = grep { $_ eq sprintf( "attr%s_value", $i++ ) } @disabled;
         }
 
+        my @extended_attributes = map { { code => $_->code, attribute => $_->attribute } }
+            $patron->extended_attributes->filter_by_branch_limitations->as_list;
+
         for my $code ( keys %$attributes ) {
             my $attr_type = Koha::Patron::Attribute::Types->find($code);
 
@@ -436,25 +440,24 @@ if ( $op eq 'cud-do' ) {
             # If this borrower is not in the category of this attribute, we don't want to modify this attribute
             next if $attr_type->category_code and $attr_type->category_code ne $patron->categorycode;
 
-            if ( $attributes->{$code}->{disabled} ) {
+            # Remove any current values of same type
+            @extended_attributes = grep { $_->{code} ne $code } @extended_attributes;
 
-                # The attribute is disabled, we remove it for this borrower !
-                eval {
-                    $patron->extended_attributes->search( { 'me.code' => $code } )
-                        ->filter_by_branch_limitations->delete;
-                };
-                push @errors, { error => $@ } if $@;
-            } else {
-                eval {
-                    $patron->extended_attributes->search( { 'me.code' => $code } )
-                        ->filter_by_branch_limitations->delete;
-                    $patron->add_extended_attribute( { code => $code, attribute => $_ } )
-                        for @{ $attributes->{$code}->{values} };
-                };
-                push @errors, { error => $@ } if $@;
+            # The attribute is disabled, we remove it for this borrower !
+            if ( !$attributes->{$code}->{disabled} ) {
+                push @extended_attributes, { code => $code, attribute => $_}
+                    for @{ $attributes->{$code}->{values} };
             }
+
         }
 
+        try {
+            $patron->extended_attributes(\@extended_attributes);
+        } catch {
+            my $message =  blessed $_ ? $_->full_message() : $_;
+            push @errors, { error => $message };
+        };
+
         # Handle patron messages
         my $message      = $input->param('message');
         my $branchcode   = C4::Context::mybranch;
-- 
2.39.5