From c2880f6ffd418cd91c404a38a512da8448e5325e 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 | 55 ++++++++++++++++ t/db_dependent/Koha/Patron/Attribute.t | 88 +++++++++++++++++++++++++- 2 files changed, 142 insertions(+), 1 deletion(-) diff --git a/Koha/Patron/Attribute.pm b/Koha/Patron/Attribute.pm index aaaefbcbb80..34eaf4c00cf 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,13 @@ sub _type { return 'BorrowerAttribute'; } +=head3 _deleted_attribute_key + +=cut + +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..c97a7e79bc4 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,87 @@ 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