From a200d5fa9aa9a66d1fed539ac86248d6ca893fa3 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 --- 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 7f8783c303f..daf3650c8d3 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 b6ab6fed6d7..642b9cc8da0 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; @@ -742,5 +742,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 9e1830ddb2f..e6608a15bc4 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.34.1