From aab66dd8818efc76c1a384e26904a0bcfe77f5e0 Mon Sep 17 00:00:00 2001 From: Stefan Berndtsson Date: Fri, 22 Sep 2023 13:14:03 +0200 Subject: [PATCH] Bug 26744: Log changes to extended patron attributes Log non-repeatable attributes and only log changes and creations, not deletions. When changed, attributes are first all deleted, then the relevant ones are recreated. This patch catches all deletions, and checks against them in the creation phase, logging only the attributes that have changed (with before/after content). This makes it impossible to log actual deletions. To test: 1) Ensure tests in t/db_dependent/Koha/Patron/Attribute.t passes Sponsored-by: Gothenburg University Library --- Koha/Patron/Attribute.pm | 51 +++++++++++++++ t/db_dependent/Koha/Patron/Attribute.t | 90 +++++++++++++++++++++++++- 2 files changed, 140 insertions(+), 1 deletion(-) diff --git a/Koha/Patron/Attribute.pm b/Koha/Patron/Attribute.pm index aaaefbcbb80..6c6fd90ce4d 100644 --- a/Koha/Patron/Attribute.pm +++ b/Koha/Patron/Attribute.pm @@ -21,9 +21,14 @@ use Koha::Database; use Koha::Exceptions::Patron::Attribute; use Koha::Patron::Attribute::Types; use Koha::AuthorisedValues; +use Koha::Cache::Memory::Lite; +use C4::Log qw( logaction ); +use JSON qw( to_json ); use base qw(Koha::Object); +our %deleted_attributes; + =head1 NAME Koha::Patron::Attribute - Koha Patron Attribute Object class @@ -55,6 +60,33 @@ sub store { Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw( attribute => $self ) unless $self->unique_ok(); + # Only log for non-repeatable attributes + if (!$self->type->repeatable) { + my $change; + if ( exists $deleted_attributes{$self->_deleted_attribute_key} ) { + my $previous_attribute = $deleted_attributes{$self->_deleted_attribute_key}; + if ( $previous_attribute->attribute ne $self->attribute ) { + $change = { + before => $previous_attribute->attribute, + after => $self->attribute + }; + } + delete $deleted_attributes{$self->_deleted_attribute_key}; + } else { + $change = { + before => "", + after => $self->attribute + }; + } + if (defined $change) { + logaction( + "MEMBERS", + "MODIFY", + $self->borrowernumber, + "Patron attribute " . $self->code . ": " . to_json($change, { pretty => 1, canonical => 1 }) + ); + } + } return $self->SUPER::store(); } @@ -189,6 +221,20 @@ sub unique_ok { return $ok; } +=head3 delete + +Overloaded I method to temporarily stash deleted attribute for action log. + +=cut + +sub delete { + my ($self) = @_; + if (!$self->type->repeatable) { + $deleted_attributes{$self->_deleted_attribute_key} = $self; + } + $self->SUPER::delete(); +} + =head2 Internal methods =head3 _type @@ -199,4 +245,9 @@ sub _type { return 'BorrowerAttribute'; } +sub _deleted_attribute_key { + my ($self) = @_; + return join(':', $self->borrowernumber, $self->code); +} + 1; diff --git a/t/db_dependent/Koha/Patron/Attribute.t b/t/db_dependent/Koha/Patron/Attribute.t index a24de0fb2d2..b946b63f112 100755 --- a/t/db_dependent/Koha/Patron/Attribute.t +++ b/t/db_dependent/Koha/Patron/Attribute.t @@ -18,8 +18,9 @@ # along with Koha; if not, see . use Modern::Perl; +use JSON qw( to_json ); -use Test::More tests => 3; +use Test::More tests => 4; use t::lib::TestBuilder; use Test::Exception; @@ -27,6 +28,7 @@ use Test::Exception; use Koha::Database; use Koha::Patron::Attribute; use Koha::Patron::Attributes; +use Koha::ActionLogs; my $schema = Koha::Database->new->schema; my $builder = t::lib::TestBuilder->new; @@ -404,3 +406,89 @@ subtest 'merge_and_replace_with' => sub { $schema->storage->txn_rollback; }; + +subtest 'action log tests' => sub { + plan tests => 3; + my $schema = Koha::Database->new->schema; + $schema->storage->txn_begin; + + my $patron = $builder->build_object({ class=> 'Koha::Patrons' }); + my $attribute_type = $builder->build_object( + { + class => 'Koha::Patron::Attribute::Types', + value => { repeatable => 0 } + } + ); + + my $attributes = [ + { + attribute => 'Foo', + code => $attribute_type->code, + } + ]; + $patron->extended_attributes($attributes); + + my $change = { + before => "", + after => 'Foo', + }; + my $info = "Patron attribute " . $attribute_type->code . ": " . to_json($change, { pretty => 1, canonical => 1 }); + my $action_logs = Koha::ActionLogs->search( + { + module => "MEMBERS", + action => "MODIFY", + object => $patron->borrowernumber, + info => $info + } + ); + is( + $action_logs->count, + 1, + 'An action log entry has been created when adding patron attribute' + ); + + $patron->extended_attributes($attributes); + $action_logs = Koha::ActionLogs->search( + { + module => "MEMBERS", + action => "MODIFY", + object => $patron->borrowernumber, + info => $info + } + ); + is( + $action_logs->count, + 1, + 'No additional action log entry has been created when updating patron attribute with same value' + ); + + $attributes = [ + { + attribute => 'Bar', + code => $attribute_type->code, + } + ]; + $patron->extended_attributes($attributes); + + $change = { + before => 'Foo', + after => 'Bar' + }; + $info = "Patron attribute " . $attribute_type->code . ": " . to_json($change, { pretty => 1, canonical => 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 attribute with different value' + ); + + $schema->storage->txn_rollback; +}; + -- 2.43.0