Bugzilla – Attachment 182075 Details for
Bug 39452
Log cardnumber changes as distinct action within borrower logs
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 39452: Add CardnumberLog to track changes to cardnumbers
Bug-39452-Add-CardnumberLog-to-track-changes-to-ca.patch (text/plain), 12.97 KB, created by
Martin Renvoize (ashimema)
on 2025-05-08 12:26:12 UTC
(
hide
)
Description:
Bug 39452: Add CardnumberLog to track changes to cardnumbers
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-05-08 12:26:12 UTC
Size:
12.97 KB
patch
obsolete
>From 9fbf1027174904250ee6e0bb03dc3165ff43b734 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Wed, 7 May 2025 13:59:54 +0000 >Subject: [PATCH] Bug 39452: Add CardnumberLog to track changes to cardnumbers > >This patch adds a new preference 'CardnumberLog' which allows libraries >to track cardnumebr changes for patrons. > >While these changes are currently captured in the BorrowersLog, some libraries >would like to keep cardnumber chanegs specifically, and for a longer period than >other borrower modifications. > >To test: >1 - Apply patches, update database >2 - Ensure BorrowersLog and CardnumberLog are enabled >3 - Edit a borrower, not thier cardnuber >4 - Confirm in Modification Log only one entry is recorded >5 - Edit again, changing cardnumber only >6 - Confirm 2 modifications are recorded, a MODIFY and a MODIFY_CARDNUMBER >7 - Disable BorrowersLog >8 - Edit patron changing something other than cardnumber >9 - Confirm no log generated >10 - Edit borrowers cardnumber >11 - Confirm one MODIFY_CARDNUMBER recorded >12 - Disable CardnumberLog >13 - Edit patrons cardnumber and other fields >14 - Confirm no logs generated > >Signed-off-by: PhilipOrr <philip.orr@lmscloud.de> >Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> >--- > Koha/Patron.pm | 17 +++++- > installer/data/mysql/mandatory/sysprefs.sql | 1 + > .../prog/en/includes/action-logs.inc | 2 + > .../en/modules/admin/preferences/logs.pref | 6 ++ > .../prog/en/modules/tools/viewlog.tt | 2 +- > .../bootstrap/en/modules/opac-detail.tt | 2 +- > t/db_dependent/Koha/Patrons.t | 56 +++++++++++++++++-- > 7 files changed, 77 insertions(+), 9 deletions(-) > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 55e06599574..ce9538b7d4a 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -401,7 +401,7 @@ sub store { > } > > # Actionlogs >- if ( C4::Context->preference("BorrowersLog") ) { >+ if ( C4::Context->preference("BorrowersLog") || C4::Context->preference("CardnumberLog") ) { > my $info; > my $from_storage = $self_from_storage->unblessed; > my $from_object = $self->unblessed; >@@ -413,7 +413,8 @@ sub store { > if $from_object->{dateexpiry}; > > my @skip_fields = (qw/lastseen updated_on/); >- for my $key ( keys %{$from_storage} ) { >+ my @keys = C4::Context->preference("BorrowersLog") ? keys %{$from_storage} : ('cardnumber'); >+ for my $key (@keys) { > next if any { /$key/ } @skip_fields; > my $storage_value = $from_storage->{$key} // q{}; > my $object_value = $from_object->{$key} // q{}; >@@ -436,7 +437,17 @@ sub store { > $info, > { utf8 => 1, pretty => 1, canonical => 1 } > ) >- ); >+ ) if C4::Context->preference("BorrowersLog"); >+ logaction( >+ "MEMBERS", >+ "MODIFY_CARDNUMBER", >+ $self->borrowernumber, >+ to_json( >+ $info->{cardnumber}, >+ { utf8 => 1, pretty => 1, canonical => 1 } >+ ) >+ ) if defined $info->{cardnumber} && C4::Context->preference("CardnumberLog"); >+ > } > } > >diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql >index 0083b925c40..9fea00c13b7 100644 >--- a/installer/data/mysql/mandatory/sysprefs.sql >+++ b/installer/data/mysql/mandatory/sysprefs.sql >@@ -144,6 +144,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` > ('CanMarkHoldsToPullAsLost','do_not_allow','do_not_allow|allow|allow_and_notify','Add a button to the "Holds to pull" screen to mark an item as lost and notify the patron.','Choice'), > ('canreservefromotherbranches','1','','With Independent branches on, can a user from one library place a hold on an item from another library','YesNo'), > ('CardnumberLength', '', '', 'Set a length for card numbers with a maximum of 32 characters.', 'Free'), >+('CardnumberLog','1',NULL,'If ON, log edit actions on patron cardnumbers','YesNo'), > ('casAuthentication','0','','Enable or disable CAS authentication','YesNo'), > ('casLogout','0','','Does a logout from Koha should also log the user out of CAS?','YesNo'), > ('casServerUrl','https://localhost:8443/cas','','URL of the cas server','Free'), >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc >index 329a721c9fa..dae105f20e4 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc >@@ -147,6 +147,8 @@ > <span>Overdue</span> > [% CASE 'EXPIRE' %] > <span>Expire</span> >+ [% CASE 'MODIFY_CARDNUMBER' %] >+ <span>Modify cardnumber</span> > [% CASE %] > [% action | html %] > [% END %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref >index 46100bb836a..c2598d19a45 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref >@@ -12,6 +12,12 @@ Logging: > 1: Log > 0: "Don't log" > - changes to patron records and patron restrictions. >+ - >+ - pref: CardnumberLog >+ choices: >+ 1: Log >+ 0: "Don't log" >+ - changes to patron cardnumbers. > - > - pref: CataloguingLog > choices: >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt >index cd6f64b2a3e..672b2db4133 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt >@@ -147,7 +147,7 @@ > <label for="actionALL" class="viewlog"><input type="checkbox" id="actionALL" name="actions" value="" /> All</label> > [% END %] > >- [% FOREACH actx IN [ 'ADD' 'DELETE' 'MODIFY' 'ISSUE' 'RETURN' 'RENEW' 'CREATE' 'CANCEL' 'FILL' 'SUSPEND' 'RESUME' 'ADDCIRCMESSAGE' 'MODCIRCMESSAGE' 'DELCIRCMESSAGE' 'STATUS_CHANGE' 'PATRON_NOTICE' 'CHANGE PASS' 'RESET PASS' 'Run' 'End' 'EDIT_MAPPINGS' 'RESET_MAPPINGS' 'ADD_BASKET' 'MODIFY_BASKET' 'MODIFY_BASKET_HEADER' 'MODIFY_BASKET_USERS' 'CLOSE_BASKET' 'APPROVE_BASKET' 'REOPEN_BASKET' 'CANCEL_ORDER' 'CREATE_ORDER' 'MODIFY_ORDER' 'CREATE_INVOICE_ADJUSTMENT' 'UPDATE_INVOICE_ADJUSTMENT' 'DELETE_INVOICE_ADJUSTMENT' 'RECEIVE_ORDER' 'MODIFY_BUDGET' 'MODIFY_FUND' 'CREATE_FUND' 'DELETE_FUND' 'ACQUISITION CLAIM' 'ACQUISITION ORDER' 'OVERDUE' 'EXPIRE' 'CREATE_RESTRICTION' 'MODIFY_RESTRICTION' 'DELETE_RESTRICTION' ] %] >+ [% FOREACH actx IN [ 'ADD' 'DELETE' 'MODIFY' 'ISSUE' 'RETURN' 'RENEW' 'CREATE' 'CANCEL' 'FILL' 'SUSPEND' 'RESUME' 'ADDCIRCMESSAGE' 'MODCIRCMESSAGE' 'DELCIRCMESSAGE' 'STATUS_CHANGE' 'PATRON_NOTICE' 'CHANGE PASS' 'RESET PASS' 'Run' 'End' 'EDIT_MAPPINGS' 'RESET_MAPPINGS' 'ADD_BASKET' 'MODIFY_BASKET' 'MODIFY_BASKET_HEADER' 'MODIFY_BASKET_USERS' 'CLOSE_BASKET' 'APPROVE_BASKET' 'REOPEN_BASKET' 'CANCEL_ORDER' 'CREATE_ORDER' 'MODIFY_ORDER' 'CREATE_INVOICE_ADJUSTMENT' 'UPDATE_INVOICE_ADJUSTMENT' 'DELETE_INVOICE_ADJUSTMENT' 'RECEIVE_ORDER' 'MODIFY_BUDGET' 'MODIFY_FUND' 'CREATE_FUND' 'DELETE_FUND' 'ACQUISITION CLAIM' 'ACQUISITION ORDER' 'OVERDUE' 'EXPIRE' 'CREATE_RESTRICTION' 'MODIFY_RESTRICTION' 'DELETE_RESTRICTION' 'MODIFY_CARDNUMBER' ] %] > [% IF actions.grep(actx).size %] > <label for="action[% actx| replace('\s+', '_') | html %]" class="viewlog" > ><input type="checkbox" id="action[% actx | replace('\s+', '_') | html %]" name="actions" value="[% actx | html %]" checked="checked" /> [% PROCESS translate_log_action action=actx %]</label >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt >index 8792eb39122..10dd73ebd42 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt >@@ -1514,7 +1514,7 @@ > <td class="barcode" property="serialNumber">[% ITEM_RESULT.barcode | html %]</td> > [% END %] > >- [% IF item_level_holds > 0 %] >+ [% IF item_level_holds > 0 || show_priority %] > <td class="holds_count"> > [% IF holds_count.defined %][% ITEM_RESULT.holds_count | html %][% END %] > [% IF ITEM_RESULT.priority %] >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index 3fee9fc2dfd..ed7ad7a3516 100755 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -2019,10 +2019,11 @@ $nb_of_patrons = Koha::Patrons->search->count; > $retrieved_patron_1->delete; > is( Koha::Patrons->search->count, $nb_of_patrons - 1, 'Delete should have deleted the patron' ); > >-subtest 'BorrowersLog tests' => sub { >- plan tests => 5; >+subtest 'BorrowersLog and CardnumberLog tests' => sub { >+ plan tests => 13; > >- t::lib::Mocks::mock_preference( 'BorrowersLog', 1 ); >+ t::lib::Mocks::mock_preference( 'BorrowersLog', 1 ); >+ t::lib::Mocks::mock_preference( 'CardnumberLog', 0 ); > my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); > > my $cardnumber = $patron->cardnumber; >@@ -2042,7 +2043,8 @@ subtest 'BorrowersLog tests' => sub { > ->search( { module => 'MEMBERS', action => 'MODIFY', object => $patron->borrowernumber } ); > is( scalar @logs, 1, 'With BorrowerLogs and TrackLastPatronActivityTriggers we should not spam the logs' ); > >- Koha::ActionLogs->search()->delete(); >+ # Clear the logs for this patron >+ Koha::ActionLogs->search( { object => $patron->borrowernumber } )->delete(); > $patron->get_from_storage(); > $patron->set( { debarred => "" } ); > $patron->store; >@@ -2057,6 +2059,52 @@ subtest 'BorrowersLog tests' => sub { > defined $log, > "No action log generated where incoming changed column is empty string and value in storage is NULL" > ); >+ >+ t::lib::Mocks::mock_preference( 'CardnumberLog', 1 ); >+ $patron->set( { cardnumber => 'TESTCARDNUMBER_1' } )->store; >+ @logs = $schema->resultset('ActionLog') >+ ->search( { module => 'MEMBERS', action => 'MODIFY', object => $patron->borrowernumber } ); >+ is( scalar @logs, 1, 'With BorrowersLog, one detailed MODIFY action should be logged for the modification.' ); >+ @logs = $schema->resultset('ActionLog') >+ ->search( { module => 'MEMBERS', action => 'MODIFY_CARDNUMBER', object => $patron->borrowernumber } ); >+ is( >+ scalar @logs, 1, >+ 'With CardnumberLog, one detailed MODIFY_CARDNUMBER action should be logged for the modification.' >+ ); >+ >+ t::lib::Mocks::mock_preference( 'BorrowersLog', 0 ); >+ $patron->set( { cardnumber => 'TESTCARDNUMBER' } )->store; >+ @logs = $schema->resultset('ActionLog') >+ ->search( { module => 'MEMBERS', action => 'MODIFY', object => $patron->borrowernumber } ); >+ is( >+ scalar @logs, 1, >+ 'With Cardnumberlog and not BorrowersLog, no more detailed MODIFY action should be logged for the modification.' >+ ); >+ @logs = $schema->resultset('ActionLog') >+ ->search( { module => 'MEMBERS', action => 'MODIFY_CARDNUMBER', object => $patron->borrowernumber } ); >+ is( >+ scalar @logs, 2, >+ 'With CardnumberLogs, one more detailed MODIFY_CARDNUMBER action should be logged for the modification.' >+ ); >+ $log_info = from_json( $logs[1]->info ); >+ is( $log_info->{after}, 'TESTCARDNUMBER', 'Got correct new cardnumber' ); >+ is( $log_info->{before}, 'TESTCARDNUMBER_1', 'Got correct old cardnumber' ); >+ >+ t::lib::Mocks::mock_preference( 'CardnumberLog', 0 ); >+ $patron->set( { cardnumber => 'TESTCARDNUMBER_1' } )->store; >+ @logs = $schema->resultset('ActionLog') >+ ->search( { module => 'MEMBERS', action => 'MODIFY', object => $patron->borrowernumber } ); >+ is( >+ scalar @logs, 1, >+ 'With neither Cardnumberlog nor BorrowersLog, no more detailed MODIFY action should be logged for the modification.' >+ ); >+ @logs = $schema->resultset('ActionLog') >+ ->search( { module => 'MEMBERS', action => 'MODIFY_CARDNUMBER', object => $patron->borrowernumber } ); >+ is( >+ scalar @logs, 2, >+ 'With neither CardnumberLog nor BorrowersLog, one detailed MODIFY_CARDNUMBER action should be logged for the modification.' >+ ); >+ > }; > $schema->storage->txn_rollback; > >-- >2.49.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 39452
:
179703
|
179904
|
182031
|
182032
| 182075 |
182076