Bug 39452

Summary: Log cardnumber changes as distinct action within borrower logs
Product: Koha Reporter: Andrew Fuerste-Henry <andrew>
Component: PatronsAssignee: Nick Clemens (kidclamp) <nick>
Status: Pushed to main --- QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, kyle.m.hall, m.de.rooy, martin.renvoize, nick
Version: MainKeywords: release-notes-needed
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27364
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:
Bug Depends on: 18626    
Bug Blocks:    
Attachments: Bug 39452: Add CardnumberLog to track changes to cardnumbers
Bug 39452: Add CardnumberLog to track changes to cardnumbers
Bug 39452: Add CardnumberLog to track changes to cardnumbers
Bug 39452: (follow-up) Use INSERT IGNORE for system preference
Bug 39452: Add CardnumberLog to track changes to cardnumbers
Bug 39452: (follow-up) Use INSERT IGNORE for system preference

Description Andrew Fuerste-Henry 2025-03-25 17:54:47 UTC
Libraries find it useful to retain previous patron cardnumbers. Long ago, bug 18626 started retaining those old cardnumbers in the action logs. But action logs contain a lot of other data a library may not wish to retain indefinitely. Cleanup_database can specify deletion/retention of logs down to the action level, so making cardnumber changes a distinct action would make is easy to retain cardnumber changes in the logs without retaining other data.
Comment 1 Nick Clemens (kidclamp) 2025-03-25 19:59:53 UTC Comment hidden (obsolete)
Comment 2 Philip Orr 2025-03-30 18:43:50 UTC
Created attachment 179904 [details] [review]
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>
Comment 3 Marcel de Rooy 2025-04-18 08:32:37 UTC
 FAIL   Koha/Patron.pm
   FAIL   tidiness
                File is not tidy, please run `perl misc/devel/tidy.pl Koha/Patron.pm`

 FAIL   installer/data/mysql/atomicupdate/bug_39452_add_cardnumberlog.pl
   FAIL   forbidden patterns
                forbidden pattern: Use INSERT IGNORE INTO on inserting a new syspref (see bug 9071) (line 13)
   FAIL   tidiness
                File is not tidy, please run `perl misc/devel/tidy.pl installer/data/mysql/atomicupdate/bug_39452_add_cardnumberlog.pl`

 OK     installer/data/mysql/mandatory/sysprefs.sql

 OK     koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt

 OK     koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt

 FAIL   t/db_dependent/Koha/Patrons.t
   FAIL   tidiness
                File is not tidy, please run `perl misc/devel/tidy.pl t/db_dependent/Koha/Patrons.t`
Comment 4 Owen Leonard 2025-04-18 14:10:19 UTC
I also got this error (from the pre-commit hook?) when trying to tidy:

Constant subroutine Koha::Patron::ADMINISTRATIVE_LOCKOUT redefined at /usr/lib/x86_64-linux-gnu/perl-base/constant.pm line 171.
Comment 5 Marcel de Rooy 2025-04-22 11:41:15 UTC
(In reply to Owen Leonard from comment #4)
> I also got this error (from the pre-commit hook?) when trying to tidy:
> 
> Constant subroutine Koha::Patron::ADMINISTRATIVE_LOCKOUT redefined at
> /usr/lib/x86_64-linux-gnu/perl-base/constant.pm line 171.

Probably you can ignore that one. We have lots of those due to some cyclic dependencies between modules.
Comment 6 Nick Clemens (kidclamp) 2025-05-07 14:16:25 UTC
Created attachment 182031 [details] [review]
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>
Comment 7 Nick Clemens (kidclamp) 2025-05-07 14:46:08 UTC
Created attachment 182032 [details] [review]
Bug 39452: (follow-up) Use INSERT IGNORE for system preference
Comment 8 Martin Renvoize (ashimema) 2025-05-08 12:26:12 UTC
Created attachment 182075 [details] [review]
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>
Comment 9 Martin Renvoize (ashimema) 2025-05-08 12:26:14 UTC
Created attachment 182076 [details] [review]
Bug 39452: (follow-up) Use INSERT IGNORE for system preference

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 10 Katrin Fischer 2025-05-09 06:55:28 UTC
Fixed before pushing:
Signed-off-by: PhilipOrr <philip.orr@lmscloud.de>

+        say $out "Added new system preference 'CardnumberLog'";
say_success

+    description => "Add CardnumberLog Preference",
Capitalization

+            ('CardnumberLog','1',NULL,'If ON, log edit actions on patron cardnumbers','YesNo')
Not turning this on by default, updated to 0 in the atomicupdate.
Comment 11 Katrin Fischer 2025-05-09 07:14:08 UTC
Pushed for 25.05!

Well done everyone, thank you!