https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |20956, 36698 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 [Bug 20956] BorrowersLog is not logging permission changes https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36698 [Bug 36698] Display 'diff' nicely in action logs -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |37940 CC| |martin.renvoize@openfifth.c | |o.uk Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37940 [Bug 37940] [OMNIBUS] All modification logged via action logs should include original to create JSON diff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Sponsorship status|--- |Unsponsored Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 --- Comment #3 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 195496 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195496&action=edit Bug 42085: Log patron permissions in MEMBERS/DELETE action log When BorrowersLog is enabled, Koha::Patron::delete() now captures the patron's granted permissions before deletion and includes them in the MEMBERS/DELETE log entry. Patrons with no granted permissions produce no permissions key in the log. Patrons with permissions will show them as removed in the diff viewer, consistent with all other fields in the DELETE diff. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Little bit of refactoring added in here so we're better covered for unit tests and any future API additions can easily go through the same code. Basically.. check you can still add and remove permissions as you could before and then also check the logging of those changes in action logs depending on the BorrowersLog preference. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 --- Comment #7 from Lisette Scheer <lisette@bywatersolutions.com> --- (In reply to Lisette Scheer from comment #6)

I'm not sure if tying permissions logging specifically to the borrowers log is desired behavior. Some libraries might want to log changes to permissions for security reasons but not log all borrower changes.

So a PermissionLog option would be nice. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195495|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195497|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 --- Comment #9 from David Nind <david@davidnind.com> --- Created attachment 195501 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195501&action=edit Bug 42085: Log patron permissions in MEMBERS/DELETE action log When BorrowersLog is enabled, Koha::Patron::delete() now captures the patron's granted permissions before deletion and includes them in the MEMBERS/DELETE log entry. Patrons with no granted permissions produce no permissions key in the log. Patrons with permissions will show them as removed in the diff viewer, consistent with all other fields in the DELETE diff. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |The log viewer now (with release notes| |dependent changes) shows | |the details for a deleted | |borrower in the info and | |diff columns - including | |the permissions they had | |before the patron account | |was deleted. Previously, no | |details were recorded - | |just that a patron with | |their borrower number was | |deleted. | | | |This may be helpful for | |auditing purposes (for | |example, a previous | |employee's account was | |decommissioned, and they | |had XYZ permissions at the | |time). CC| |david@davidnind.com --- Comment #11 from David Nind <david@davidnind.com> --- Not sure that I got the release note right (with all the dependent patches). Testing notes (using KTD): 1. For a staff patron (I used Henry): 1.1 Add a permission (I added: Staff access, allows viewing of catalogue in staff interface (catalogue)) 1.2 Change details for their account (for example, add a middle name) 1.3 Delete the patron account 2. In the log viewer, show the Diff column (Columns > Diff). 3. In the log viewer, note that: [1] 3.1 The patron account details change in 1.2 is logged 3.2 The patron account deletion is logged, but no details are recorded 3.3 The permission changes are not logged. 3.4 As there are no details recorded when the patron is deleted, there is no record of the permissions they had. 4. Apply the patches and restart everything (restart_all). 5. Change a patron so that they are a staff patron (I used Mary) 6. For the staff patron, repeat the steps in step 1 7. In the log viewer, note that there are now entries in the Diff column for all the changes made (there are also additional details logged in the info column) 8. The info and diff columns for the patron account deletion now shows the permissions they had at the time the account was deleted 8.1 Info column: ..."permissions" : { "catalogue" : 1 }... 8.2 Diff column: ..."permissions" : { "R" : { "catalogue" : 1 } }... 9. The tests pass: prove t/db_dependent/Koha/Patrons.t [1] Log viewer entries before the patch: Patrons Delete 19 Staff interface Patrons Modify 19 { "middle_name" : { "after" : "James", "before" : null } } Staff interface -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com QA Contact|testopia@bugs.koha-communit |lisette@bywatersolutions.co |y.org |m -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 --- Comment #13 from Lisette Scheer <lisette@bywatersolutions.com> --- I filed a followup for different modules. Tests failed, otherwise worked as expected Running tests (1) * Proving /kohadevbox/koha/t/db_dependent/Koha/Patrons.t FAIL # Looks like you planned 6 tests but ran 2. # Failed test 'holds and old_holds' # at /kohadevbox/koha/t/db_dependent/Koha/Patrons.t line 1308. encountered object '2026-03-28T13:52:52', but neither allow_blessed, convert_blessed nor allow_tags settings are enabled (or TO_JSON/FREEZE method missing) at /usr/share/perl5/JSON.pm line 173. # Looks like your test exited with 11 just after 23. /kohadevbox/koha/t/db_dependent/Koha/Patrons.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 29/51 subtests Test Summary Report ------------------- /kohadevbox/koha/t/db_dependent/Koha/Patrons.t (Wstat: 2816 (exited 11) Tests: 23 Failed: 1) Failed test: 22 Non-zero exit status: 11 Parse errors: Bad plan. You planned 51 tests but ran 23. Files=1, Test -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #14 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- I'm unable to replicate your test failure I'm afraid Lisette.. all I've done is apply the patches and rebase against main. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 --- Comment #15 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197320 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197320&action=edit Bug 42085: Add Koha::Patron::set_permissions method This new method moves the permissions write logic out of the CGI controller and into Koha::Patron, making it testable and reusable. Behaviour: - Accepts a hashref of granted permissions only (matches the non-zero subset of permissions() return value) - Computes the borrowers.flags bitmask from the userflags table - Replaces user_permissions rows atomically inside a txn_do block - Applies a session-aware superlibrarian guard when ProtectSuperlibrarianPrivileges is enabled - Logs a MEMBERS/MODIFY action when permissions change and BorrowersLog is enabled, diffing before/after states Tests added to t/db_dependent/Koha/Patrons.t. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 --- Comment #17 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 197322 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197322&action=edit Bug 42085: Slim down members/member-flags.pl cud-newflags block Replace the raw-DBI permission write block (~62 lines) with a call to $patron->set_permissions(\%granted). Logging and the superlibrarian guard are now handled inside the object method. The CGI layer now only parses the flag params into a granted hashref and delegates — keeping the controller thin and the logic testable. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197321|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 --- Comment #19 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 197528 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197528&action=edit Bug 42085: Add Koha::Patron::set_permissions method This new method moves the permissions write logic out of the CGI controller and into Koha::Patron, making it testable and reusable. Behaviour: - Accepts a hashref of granted permissions only (matches the non-zero subset of permissions() return value) - Computes the borrowers.flags bitmask from the userflags table - Replaces user_permissions rows atomically inside a txn_do block - Applies a session-aware superlibrarian guard when ProtectSuperlibrarianPrivileges is enabled - Logs a MEMBERS/MODIFY action when permissions change and BorrowersLog is enabled, diffing before/after states Tests added to t/db_dependent/Koha/Patrons.t. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 --- Comment #21 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 197530 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197530&action=edit Bug 42085: Slim down members/member-flags.pl cud-newflags block Replace the raw-DBI permission write block (~62 lines) with a call to $patron->set_permissions(\%granted). Logging and the superlibrarian guard are now handled inside the object method. The CGI layer now only parses the flag params into a granted hashref and delegates — keeping the controller thin and the logic testable. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting --- Comment #23 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- This will not be backported to 25.11.x due to it being deemed an enhancement or a dependancy not being met -- You are receiving this mail because: You are watching all bug changes.