Bugzilla – Attachment 164098 Details for
Bug 26744
Log changes to extended patron attributes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26744: Add support for repeatable and deleted attributes
Bug-26744-Add-support-for-repeatable-and-deleted-a.patch (text/plain), 12.53 KB, created by
David Gustafsson
on 2024-03-28 18:20:44 UTC
(
hide
)
Description:
Bug 26744: Add support for repeatable and deleted attributes
Filename:
MIME Type:
Creator:
David Gustafsson
Created:
2024-03-28 18:20:44 UTC
Size:
12.53 KB
patch
obsolete
>From 76d2ae7e8ad4c2ef541756e2590d39cf64c0536a Mon Sep 17 00:00:00 2001 >From: David Gustafsson <david.gustafsson@ub.gu.se> >Date: Thu, 28 Mar 2024 19:19:45 +0100 >Subject: [PATCH] Bug 26744: Add support for repeatable and deleted attributes > >--- > Koha/Patron.pm | 52 +++++++- > Koha/Patron/Attribute.pm | 55 -------- > t/db_dependent/Koha/Patron/Attribute.t | 171 ++++++++++++++++--------- > 3 files changed, 162 insertions(+), 116 deletions(-) > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 5b0bd9ea7f5..59330fe07dd 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -20,11 +20,12 @@ package Koha::Patron; > > use Modern::Perl; > >-use List::MoreUtils qw( any uniq ); >+use List::MoreUtils qw( any uniq notall zip6); > use JSON qw( to_json ); > use Unicode::Normalize qw( NFKD ); > use Try::Tiny; > use DateTime (); >+use C4::Log qw( logaction ); > > use C4::Auth qw( checkpw_hash ); > use C4::Context; >@@ -2124,6 +2125,21 @@ Or setter FIXME > sub extended_attributes { > my ( $self, $attributes ) = @_; > if ($attributes) { # setter >+ my %attribute_changes; >+ my $attribute_key = sub { >+ my ($attribute) = @_; >+ return join(':', $attribute->code, $self->borrowernumber); >+ }; >+ if (C4::Context->preference("BorrowersLog")) { >+ for my $attribute ($self->extended_attributes->as_list) { >+ my $repeatable = $attribute->type->repeatable ? 1 : 0; >+ $attribute_changes{$repeatable}->{$attribute_key->($attribute)}->{before} = []; >+ push ( >+ @{$attribute_changes{$repeatable}->{$attribute_key->($attribute)}->{before}}, >+ $attribute->attribute >+ ); >+ } >+ } > my $schema = $self->_result->result_source->schema; > $schema->txn_do( > sub { >@@ -2155,6 +2171,40 @@ sub extended_attributes { > } > } > ); >+ if (C4::Context->preference("BorrowersLog")) { >+ for my $attribute ($self->extended_attributes->as_list) { >+ my $repeatable = $attribute->type->repeatable ? 1 : 0; >+ $attribute_changes{$repeatable}->{$attribute_key->($attribute)}->{after} = []; >+ push ( >+ @{$attribute_changes{$repeatable}->{$attribute_key->($attribute)}->{after}}, >+ $attribute->attribute >+ ); >+ } >+ my $is_different = sub { >+ my ($a, $b) = map { [ sort @{$_} ] } @_; >+ return @{$a} != @{$b} || notall { $_->[0] eq $_->[1] } zip6 @{$a}, @{$b}; >+ }; >+ while (my ($repeatable, $changes) = each %attribute_changes) { >+ while (my ($attribute_key, $change) = each %{$changes}) { >+ my ($code) = split(':', $attribute_key); >+ $change->{before} //= []; >+ $change->{after} //= []; >+ >+ if ($is_different->($change->{before}, $change->{after})) { >+ unless ($repeatable) { >+ $change->{before} = @{$change->{before}} ? $change->{before}->[0] : ''; >+ $change->{after} = @{$change->{after}} ? $change->{after}->[0] : ''; >+ } >+ logaction( >+ "MEMBERS", >+ "MODIFY", >+ $self->borrowernumber, >+ "Patron attribute " . $code . ": " . to_json($change, { pretty => 1, canonical => 1 }) >+ ); >+ } >+ } >+ } >+ } > } > > my $rs = $self->_result->borrower_attributes; >diff --git a/Koha/Patron/Attribute.pm b/Koha/Patron/Attribute.pm >index cce32893e7f..aaaefbcbb80 100644 >--- a/Koha/Patron/Attribute.pm >+++ b/Koha/Patron/Attribute.pm >@@ -21,14 +21,9 @@ 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 >@@ -60,33 +55,6 @@ sub store { > Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw( attribute => $self ) > unless $self->unique_ok(); > >- # Only log for non-repeatable attributes >- if (!$self->type->repeatable && C4::Context->preference("BorrowersLog")) { >- 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(); > } > >@@ -221,20 +189,6 @@ sub unique_ok { > return $ok; > } > >-=head3 delete >- >-Overloaded I<store> method to temporarily stash deleted attribute for action log. >- >-=cut >- >-sub delete { >- my ($self) = @_; >- if (!$self->type->repeatable && C4::Context->preference("BorrowersLog")) { >- $deleted_attributes{$self->_deleted_attribute_key} = $self; >- } >- $self->SUPER::delete(); >-} >- > =head2 Internal methods > > =head3 _type >@@ -245,13 +199,4 @@ 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 d4307f3e496..705acee8101 100755 >--- a/t/db_dependent/Koha/Patron/Attribute.t >+++ b/t/db_dependent/Koha/Patron/Attribute.t >@@ -409,10 +409,24 @@ subtest 'merge_and_replace_with' => sub { > }; > > subtest 'action log tests' => sub { >- plan tests => 4; >+ plan tests => 9; > my $schema = Koha::Database->new->schema; > $schema->storage->txn_begin; > >+ my $get_info = sub { >+ my ($before, $after, $code, $repeatable) = @_; >+ my $change = { >+ before => $before, >+ after => $after >+ }; >+ if ($repeatable) { >+ while (my ($k, $v) = each %{$change}) { >+ $change->{$k} = $v ? [$v] : []; >+ } >+ } >+ return "Patron attribute " . $code . ": " . to_json($change, { pretty => 1, canonical => 1 }); >+ }; >+ > my $patron = $builder->build_object({ class=> 'Koha::Patrons' }); > my $attribute_type = $builder->build_object( > { >@@ -429,11 +443,8 @@ subtest 'action log tests' => sub { > } > ]; > $patron->extended_attributes($attributes); >- my $change = { >- before => "", >- after => 'Foo', >- }; >- my $info = "Patron attribute " . $attribute_type->code . ": " . to_json($change, { pretty => 1, canonical => 1 }); >+ >+ my $info = $get_info->('', 'Foo', $attribute_type->code); > my $action_logs = Koha::ActionLogs->search( > { > module => "MEMBERS", >@@ -447,64 +458,104 @@ subtest 'action log tests' => sub { > 0, > 'No action log entry has been created when adding patron attribute if BorrowersLog syspref disabled' > ); >- $patron->extended_attributes([]); > > t::lib::Mocks::mock_preference( 'BorrowersLog', 1 ); >- $patron->extended_attributes($attributes); >- $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' >- ); >+ my $current_action_logs_count; >+ my $repeatable_text; >+ for my $repeatable (0, 1) { >+ $repeatable_text = $repeatable ? ' repeatable' : ''; > >- $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' >- ); >+ $patron->extended_attributes([]); > >- $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' >- ); >+ $attribute_type = $builder->build_object( >+ { >+ class => 'Koha::Patron::Attribute::Types', >+ value => { repeatable => $repeatable } >+ } >+ ); >+ $attributes = [ >+ { >+ attribute => 'Foo', >+ code => $attribute_type->code, >+ } >+ ]; >+ >+ $patron->extended_attributes($attributes); >+ $info = $get_info->('', 'Foo', $attribute_type->code, $repeatable); >+ $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$repeatable_text patron attribute" >+ ); >+ >+ $current_action_logs_count = Koha::ActionLogs->search( >+ { >+ module => "MEMBERS", >+ action => "MODIFY", >+ object => $patron->borrowernumber >+ } >+ )->count; >+ >+ $patron->extended_attributes($attributes); >+ $action_logs = Koha::ActionLogs->search( >+ { >+ module => "MEMBERS", >+ action => "MODIFY", >+ object => $patron->borrowernumber >+ } >+ ); >+ is( >+ $action_logs->count, >+ $current_action_logs_count, >+ "No additional action log entry has been created when updating$repeatable_text patron attribute with same value" >+ ); >+ >+ $attributes = [ >+ { >+ attribute => 'Bar', >+ code => $attribute_type->code, >+ } >+ ]; >+ $patron->extended_attributes($attributes); >+ $info = $get_info->('Foo', 'Bar', $attribute_type->code, $repeatable); >+ $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$repeatable_text patron attribute with different value" >+ ); >+ >+ $patron->extended_attributes([]); >+ $info = $get_info->('Bar', '', $attribute_type->code, $repeatable); >+ $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 deleting$repeatable_text patron attribute value" >+ ); >+ } > > $schema->storage->txn_rollback; > }; >-- >2.43.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 26744
:
163957
|
163958
|
163959
|
164034
|
164035
|
164098
|
165212
|
165213
|
165301
|
165302
|
165303
|
165304
|
165305