[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Caroline Cyr La Rose changed: What|Removed |Added Status|Needs documenting |RESOLVED Resolution|--- |FIXED Documentation||https://gitlab.com/koha-com submission||munity/koha-manual/-/merge_ ||requests/1224 Documentation||Caroline Cyr La Rose contact|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list -- [email protected] To unsubscribe send an email to [email protected] website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Jacob O'Mara changed: What|Removed |Added Status|Pushed to main |Needs documenting --- Comment #125 from Jacob O'Mara --- 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. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Text to go in the|This patch adds permission |This enhancement adds release notes|changes to the members log. |logging of patron |Permission changes are |permission changes when |always logged, similar to |BorrowersLog is enabled. |the System Administration |Detailed information about |logging.|the changes is now included ||in the log viewer tool info ||and diff columns. Before ||this change, permission ||changes were not logged. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #124 from Martin Renvoize (ashimema) --- Thanks for all the hard work! Pushed to main for the next 26.05.00 release as RM Assistant -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Martin Renvoize (ashimema) changed: What|Removed |Added Status|Passed QA |Pushed to main Version(s)||26.05.00 released in|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Martin Renvoize (ashimema) changed: What|Removed |Added Blocks|42085 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 [Bug 42085] Permissions should be logged when a patron is deleted -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Lisette Scheer changed: What|Removed |Added Blocks||42085 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42085 [Bug 42085] Permissions should be logged when a patron is deleted -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Lisette Scheer changed: What|Removed |Added Text to go in the||This patch adds permission release notes||changes to the members log. ||Permission changes are ||always logged, similar to ||the System Administration ||logging. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #123 from Lisette Scheer --- Created attachment 194899 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194899&action=edit Bug 20956: (QA follow-up) Restore info pretty I restored the pretty code for the info file because the page needs to be able to wrap, like it does for other logs. Signed-off-by: David Nind Signed-off-by: Lisette Scheer -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #122 from Lisette Scheer --- Created attachment 194898 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194898&action=edit Bug 20956: (follow-up) Nest permissions data under 'permissions' key in MEMBERS/MODIFY log Changes to patron permissions via member-flags.pl are logged as MEMBERS/MODIFY, indistinguishable from regular patron field edits. This patch wraps the before/after permission hashes under a 'permissions' key so that the diff renderer displays changes as 'permissions.cataloguing' etc., making it clear the diff relates to permissions rather than first-level patron fields. Signed-off-by: David Nind Signed-off-by: Lisette Scheer -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #121 from Lisette Scheer --- Created attachment 194897 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194897&action=edit Bug 20956: Fix test failures Signed-off-by: David Nind Signed-off-by: Lisette Scheer -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956
--- Comment #120 from Lisette Scheer ---
Created attachment 194896
-->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194896&action=edit
Bug 20956: (QA follow-up) Match getuserflags() return structure
This patch changes Koha::Patron->permissions() to return the same
structure as C4::Auth::getuserflags() for architectural consistency.
Previously, permissions() only returned granted permissions:
{
catalogue => 1,
circulate => { override_renewals => 1 }
}
Now it returns ALL flags (matching getuserflags()):
{
superlibrarian => 0,
catalogue => 1,
circulate => { override_renewals => 1 },
borrowers => 0,
...
}
This provides several benefits:
1. Consistency - Matches established C4::Auth::getuserflags() pattern
2. Simpler checks - Can use "if ($flags->{module})" instead of
"if (exists $flags->{module})"
3. Future compatibility - Enables eventual deprecation of getuserflags()
in favor of the ORM-based $patron->permissions()
4. Helper compatibility - Works with existing _dispatch() logic
The logging functionality still works correctly - the diff will now
show explicit changes from 0 to 1 rather than absence to presence.
Test plan:
1. prove t/db_dependent/Koha/Patron.t
2. prove t/db_dependent/Log.t
3. Verify all tests pass
4. Confirm permissions() now returns all flags with 0/1 values
Signed-off-by: David Nind
Signed-off-by: Lisette Scheer
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #119 from Lisette Scheer --- Created attachment 194895 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194895&action=edit Bug 20956: (QA follow-up) Fix tests and refactor to use ORM This patch addresses several QA concerns: 1. Fixed duplicate subtest name - renamed second "is_superlibrarian() tests" to "permissions() tests" which accurately reflects what it tests 2. Fixed incorrect hash assignment - permissions() returns a hashref, not a hash. Changed `my %permissions` to `my $permissions` 3. Improved test coverage - expanded from 2 to 8 assertions covering: - Hashref return type verification - Single flag permission - No permissions - Multiple flags - Granular subpermissions from user_permissions table 4. Refactored permissions() method to use DBIx::Class ORM instead of raw SQL queries, following Koha::Object architecture principles All tests now pass without warnings. Test plan: 1. prove t/db_dependent/Koha/Patron.t 2. prove t/db_dependent/Log.t 3. Verify all tests pass 4. Confirm the test now properly validates the intended behavior Signed-off-by: David Nind Signed-off-by: Lisette Scheer -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #118 from Lisette Scheer --- Created attachment 194894 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194894&action=edit Bug 20956: (follow-up) Tidyness and fix no pod coverage in Koha/Patron.pm Signed-off-by: Hammat Wele Signed-off-by: Martin Renvoize Signed-off-by: David Nind Signed-off-by: Lisette Scheer -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #117 from Lisette Scheer --- Created attachment 194893 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194893&action=edit Bug 20956: (QA follow-up) Move subs to Koha::Patron, add unit tests Subroutines in members/member-flags.pl are combined and moved to Koha::Patron Removed the html-based english description of each permission from the logging Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize Signed-off-by: David Nind Signed-off-by: Lisette Scheer -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #116 from Lisette Scheer --- Created attachment 194892 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194892&action=edit Bug 20956: BorrowersLog is not logging permission changes Changes to permissions are not logged in the Koha action_logs, making it impossible to trace changes to staff permissions. This patch ensures that all changes to permissions are logged. Test plan: 1. Go to a staff interface 2. Search for a patron and change their permissions (note their borrowernumber) 3. Go to tools/Log viewer 4. Filter by Module = Patrons, Object = . 5. Click on submit ==> No permission changes are logged. 6. Apply the patch 7. Repeat step 1, 2, 3, 4, 5 ==> Permission changes are now logged 8. Check that the diff column contains the change: select diff from action_logs where object= order by action_id desc limit 1; Signed-off-by: David Nind Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize Signed-off-by: David Nind Signed-off-by: Lisette Scheer -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Lisette Scheer changed: What|Removed |Added Attachment #194890|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Lisette Scheer changed: What|Removed |Added Attachment #194889|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Lisette Scheer changed: What|Removed |Added Attachment #194888|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Lisette Scheer changed: What|Removed |Added Attachment #194887|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Lisette Scheer changed: What|Removed |Added Attachment #194886|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Lisette Scheer changed: What|Removed |Added Attachment #194885|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Lisette Scheer changed: What|Removed |Added Attachment #194884|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Lisette Scheer changed: What|Removed |Added Attachment #194883|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Lisette Scheer changed: What|Removed |Added Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #194882|0 |1 is obsolete|| --- Comment #115 from David Nind --- Created attachment 194890 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194890&action=edit Bug 20956: (QA follow-up) Restore info pretty I restored the pretty code for the info file because the page needs to be able to wrap, like it does for other logs. Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #194871|0 |1 is obsolete|| --- Comment #114 from David Nind --- Created attachment 194889 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194889&action=edit Bug 20956: (follow-up) Nest permissions data under 'permissions' key in MEMBERS/MODIFY log Changes to patron permissions via member-flags.pl are logged as MEMBERS/MODIFY, indistinguishable from regular patron field edits. This patch wraps the before/after permission hashes under a 'permissions' key so that the diff renderer displays changes as 'permissions.cataloguing' etc., making it clear the diff relates to permissions rather than first-level patron fields. Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #194870|0 |1 is obsolete|| --- Comment #113 from David Nind --- Created attachment 194888 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194888&action=edit Bug 20956: Fix test failures Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956
David Nind changed:
What|Removed |Added
Attachment #194869|0 |1
is obsolete||
--- Comment #112 from David Nind ---
Created attachment 194887
-->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194887&action=edit
Bug 20956: (QA follow-up) Match getuserflags() return structure
This patch changes Koha::Patron->permissions() to return the same
structure as C4::Auth::getuserflags() for architectural consistency.
Previously, permissions() only returned granted permissions:
{
catalogue => 1,
circulate => { override_renewals => 1 }
}
Now it returns ALL flags (matching getuserflags()):
{
superlibrarian => 0,
catalogue => 1,
circulate => { override_renewals => 1 },
borrowers => 0,
...
}
This provides several benefits:
1. Consistency - Matches established C4::Auth::getuserflags() pattern
2. Simpler checks - Can use "if ($flags->{module})" instead of
"if (exists $flags->{module})"
3. Future compatibility - Enables eventual deprecation of getuserflags()
in favor of the ORM-based $patron->permissions()
4. Helper compatibility - Works with existing _dispatch() logic
The logging functionality still works correctly - the diff will now
show explicit changes from 0 to 1 rather than absence to presence.
Test plan:
1. prove t/db_dependent/Koha/Patron.t
2. prove t/db_dependent/Log.t
3. Verify all tests pass
4. Confirm permissions() now returns all flags with 0/1 values
Signed-off-by: David Nind
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #194868|0 |1 is obsolete|| --- Comment #111 from David Nind --- Created attachment 194886 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194886&action=edit Bug 20956: (QA follow-up) Fix tests and refactor to use ORM This patch addresses several QA concerns: 1. Fixed duplicate subtest name - renamed second "is_superlibrarian() tests" to "permissions() tests" which accurately reflects what it tests 2. Fixed incorrect hash assignment - permissions() returns a hashref, not a hash. Changed `my %permissions` to `my $permissions` 3. Improved test coverage - expanded from 2 to 8 assertions covering: - Hashref return type verification - Single flag permission - No permissions - Multiple flags - Granular subpermissions from user_permissions table 4. Refactored permissions() method to use DBIx::Class ORM instead of raw SQL queries, following Koha::Object architecture principles All tests now pass without warnings. Test plan: 1. prove t/db_dependent/Koha/Patron.t 2. prove t/db_dependent/Log.t 3. Verify all tests pass 4. Confirm the test now properly validates the intended behavior Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #194867|0 |1 is obsolete|| --- Comment #110 from David Nind --- Created attachment 194885 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194885&action=edit Bug 20956: (follow-up) Tidyness and fix no pod coverage in Koha/Patron.pm Signed-off-by: Hammat Wele Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #194866|0 |1 is obsolete|| --- Comment #109 from David Nind --- Created attachment 194884 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194884&action=edit Bug 20956: (QA follow-up) Move subs to Koha::Patron, add unit tests Subroutines in members/member-flags.pl are combined and moved to Koha::Patron Removed the html-based english description of each permission from the logging Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #194865|0 |1 is obsolete|| --- Comment #108 from David Nind --- Created attachment 194883 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194883&action=edit Bug 20956: BorrowersLog is not logging permission changes Changes to permissions are not logged in the Koha action_logs, making it impossible to trace changes to staff permissions. This patch ensures that all changes to permissions are logged. Test plan: 1. Go to a staff interface 2. Search for a patron and change their permissions (note their borrowernumber) 3. Go to tools/Log viewer 4. Filter by Module = Patrons, Object = . 5. Click on submit ==> No permission changes are logged. 6. Apply the patch 7. Repeat step 1, 2, 3, 4, 5 ==> Permission changes are now logged 8. Check that the diff column contains the change: select diff from action_logs where object= order by action_id desc limit 1; Signed-off-by: David Nind Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #107 from Lisette Scheer --- Created attachment 194882 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194882&action=edit Bug 20956: (QA follow-up) Restore info pretty I restored the pretty code for the info file because the page needs to be able to wrap, like it does for other logs. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #106 from Lisette Scheer --- I restored the info pretty code to allow the text to wrap as other logs do -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #105 from Martin Renvoize (ashimema) --- (In reply to Lisette Scheer from comment #97) > (In reply to Martin Renvoize (ashimema) from comment #95) > > I'm going to move that final patch to the other bug.. and I have some > > further QA considerations here. > > > > I think we need to keep the action distinct.. i.e have a > > 'MODIFY_PERMISSIONS' as apposed to reusing the 'MODIFY'.. else were going to > > end up with two quite different forms of data mingled in the borrowers > > MODIFY logs. (This matters more now I'm working on bug 40136) > > I was going to split the patch because part of it is fixing no line breaks > in the info, which makes the table to wide. > > Your nesting/new action makes sense. > New action makes it easier to filter on in the table. I went with Nesting... and I think in another bug we should quickly add the 'diff' to the search options.. then you you do 'MODIFY' + 'permission' in the diff as your search to filter easily -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #104 from Martin Renvoize (ashimema) --- Created attachment 194871 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194871&action=edit Bug 20956: (follow-up) Nest permissions data under 'permissions' key in MEMBERS/MODIFY log Changes to patron permissions via member-flags.pl are logged as MEMBERS/MODIFY, indistinguishable from regular patron field edits. This patch wraps the before/after permission hashes under a 'permissions' key so that the diff renderer displays changes as 'permissions.cataloguing' etc., making it clear the diff relates to permissions rather than first-level patron fields. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #103 from Martin Renvoize (ashimema) --- Created attachment 194870 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194870&action=edit Bug 20956: Fix test failures Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956
--- Comment #102 from Martin Renvoize (ashimema)
---
Created attachment 194869
-->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194869&action=edit
Bug 20956: (QA follow-up) Match getuserflags() return structure
This patch changes Koha::Patron->permissions() to return the same
structure as C4::Auth::getuserflags() for architectural consistency.
Previously, permissions() only returned granted permissions:
{
catalogue => 1,
circulate => { override_renewals => 1 }
}
Now it returns ALL flags (matching getuserflags()):
{
superlibrarian => 0,
catalogue => 1,
circulate => { override_renewals => 1 },
borrowers => 0,
...
}
This provides several benefits:
1. Consistency - Matches established C4::Auth::getuserflags() pattern
2. Simpler checks - Can use "if ($flags->{module})" instead of
"if (exists $flags->{module})"
3. Future compatibility - Enables eventual deprecation of getuserflags()
in favor of the ORM-based $patron->permissions()
4. Helper compatibility - Works with existing _dispatch() logic
The logging functionality still works correctly - the diff will now
show explicit changes from 0 to 1 rather than absence to presence.
Test plan:
1. prove t/db_dependent/Koha/Patron.t
2. prove t/db_dependent/Log.t
3. Verify all tests pass
4. Confirm permissions() now returns all flags with 0/1 values
Signed-off-by: David Nind
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #101 from Martin Renvoize (ashimema) --- Created attachment 194868 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194868&action=edit Bug 20956: (QA follow-up) Fix tests and refactor to use ORM This patch addresses several QA concerns: 1. Fixed duplicate subtest name - renamed second "is_superlibrarian() tests" to "permissions() tests" which accurately reflects what it tests 2. Fixed incorrect hash assignment - permissions() returns a hashref, not a hash. Changed `my %permissions` to `my $permissions` 3. Improved test coverage - expanded from 2 to 8 assertions covering: - Hashref return type verification - Single flag permission - No permissions - Multiple flags - Granular subpermissions from user_permissions table 4. Refactored permissions() method to use DBIx::Class ORM instead of raw SQL queries, following Koha::Object architecture principles All tests now pass without warnings. Test plan: 1. prove t/db_dependent/Koha/Patron.t 2. prove t/db_dependent/Log.t 3. Verify all tests pass 4. Confirm the test now properly validates the intended behavior Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #100 from Martin Renvoize (ashimema) --- Created attachment 194867 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194867&action=edit Bug 20956: (follow-up) Tidyness and fix no pod coverage in Koha/Patron.pm Signed-off-by: Hammat Wele Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #99 from Martin Renvoize (ashimema) --- Created attachment 194866 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194866&action=edit Bug 20956: (QA follow-up) Move subs to Koha::Patron, add unit tests Subroutines in members/member-flags.pl are combined and moved to Koha::Patron Removed the html-based english description of each permission from the logging Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #98 from Martin Renvoize (ashimema) --- Created attachment 194865 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194865&action=edit Bug 20956: BorrowersLog is not logging permission changes Changes to permissions are not logged in the Koha action_logs, making it impossible to trace changes to staff permissions. This patch ensures that all changes to permissions are logged. Test plan: 1. Go to a staff interface 2. Search for a patron and change their permissions (note their borrowernumber) 3. Go to tools/Log viewer 4. Filter by Module = Patrons, Object = . 5. Click on submit ==> No permission changes are logged. 6. Apply the patch 7. Repeat step 1, 2, 3, 4, 5 ==> Permission changes are now logged 8. Check that the diff column contains the change: select diff from action_logs where object= order by action_id desc limit 1; Signed-off-by: David Nind Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Martin Renvoize (ashimema) changed: What|Removed |Added Attachment #194534|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Martin Renvoize (ashimema) changed: What|Removed |Added Attachment #194533|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Martin Renvoize (ashimema) changed: What|Removed |Added Attachment #194532|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Martin Renvoize (ashimema) changed: What|Removed |Added Attachment #194531|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Martin Renvoize (ashimema) changed: What|Removed |Added Attachment #194530|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Martin Renvoize (ashimema) changed: What|Removed |Added Attachment #194529|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Martin Renvoize (ashimema) changed: What|Removed |Added Attachment #194528|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Martin Renvoize (ashimema) changed: What|Removed |Added Attachment #194527|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #97 from Lisette Scheer --- (In reply to Martin Renvoize (ashimema) from comment #95) > I'm going to move that final patch to the other bug.. and I have some > further QA considerations here. > > I think we need to keep the action distinct.. i.e have a > 'MODIFY_PERMISSIONS' as apposed to reusing the 'MODIFY'.. else were going to > end up with two quite different forms of data mingled in the borrowers > MODIFY logs. (This matters more now I'm working on bug 40136) I was going to split the patch because part of it is fixing no line breaks in the info, which makes the table to wide. Your nesting/new action makes sense. New action makes it easier to filter on in the table. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #96 from Martin Renvoize (ashimema) --- Either MODIFY_PERMISSIONS.. or we stick to MODIFY but we nest the object inside a 'permissions' field maybe -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #95 from Martin Renvoize (ashimema) --- I'm going to move that final patch to the other bug.. and I have some further QA considerations here. I think we need to keep the action distinct.. i.e have a 'MODIFY_PERMISSIONS' as apposed to reusing the 'MODIFY'.. else were going to end up with two quite different forms of data mingled in the borrowers MODIFY logs. (This matters more now I'm working on bug 40136) -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #94 from David Nind --- (In reply to Ayoub Glizi-Vicioso from comment #91) > > 1- Look for the "Columns" drop-down menu in the table's action bar > (usually located at the top of the results table). Note that there is a cog > icon (⚙️) right before the "Columns" label. > > 2- Ensure that the "Diff" checkbox is checked. Ah, that was it! Thanks! David -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Martin Renvoize (ashimema) changed: What|Removed |Added Blocks||40136 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40136 [Bug 40136] Record diff in action logs when modifying a patron -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #93 from Martin Renvoize (ashimema) --- The actions logs are such a mixed bag still right now.. I don't think we should be introducing further special cases and different code paths. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #92 from Martin Renvoize (ashimema) --- This introduced another special case.. I'd rather we solved the diff view generically to all diffs display 'nicer'.. I've added a patch and screenshot to bug 36698 that works for all logs containing a diff -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956
--- Comment #91 from Ayoub Glizi-Vicioso ---
(In reply to David Nind from comment #90)
> Created attachment 194612 [details]
> Log viewer results - after patch
>
> Sorry, I'm not seeing the diff as shown in the screenshot on bug 36698.
>
>
> Screenshot attached showing what I see in the log viewer.
>
> I have applied the patch, run yarn build, restarted everything, and cleared
> the browser cache. I even shutdown KTD and restarted with the patch applied
> in case it was a build thing.
>
> Testing notes (using KTD):
> 1. Go to a staff interface.
> 2. Search for a patron and change their permissions (note their borrower
>number) (for example: Henry Acevedo (2352935676), borrower
>number 19, add and remove
>"Access to all librarian functions (superlibrarian)"
> 3. Go to Tools > Additional tools > Log viewer.
> 4. Filter the log entries as follows:
>- Module = Patrons
>- Object = (for example, 19)
> 5. Click on submit:
>==> There are no log entries for permission changes
> 6. Apply the patch, rebuild everything (yarn build), restart
>everything (restart_all), and clear your browser cache.
> 7. Repeat steps 1 to 5:
>==> Permission changes are now logged [1]
> 8. Check that the diff column in the database contains the full details
>of the changes: [2]
> select diff from action_logs where object= order by
> action_id desc limit 1;
> 9. The tests pass:
>9.1 prove t/db_dependent/Koha/Patron.t
>9.2 prove t/db_dependent/Log.t
>
>
> [1] Log viewer information:
>
> See the screenshot:
>
> ModifyHenry Acevedo (19) {}
>
> ModifyHenry Acevedo (19)
> {
>"superlibrarian" : 1
> }
>
> [2]
>
> select diff from action_logs where object=19;
> +--+
> | diff |
> +--+
> | {"D":{"superlibrarian":{"A":1}}} |
> | {"D":{"superlibrarian":{"R":1}}} |
> +--+
Hi David,
Regarding the missing diff display:
Since your SQL query confirms that the data is correctly recorded in the
action_logs table, it is possible that the column is simply hidden in the
interface.
To check this, you can:
1- Look for the "Columns" drop-down menu in the table's action bar (usually
located at the top of the results table). Note that there is a cog icon (⚙️)
right before the "Columns" label.
2- Ensure that the "Diff" checkbox is checked.
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added See Also||https://bugs.koha-community ||.org/bugzilla3/show_bug.cgi ||?id=36698 -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956
--- Comment #90 from David Nind ---
Created attachment 194612
-->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194612&action=edit
Log viewer results - after patch
Sorry, I'm not seeing the diff as shown in the screenshot on bug 36698.
Screenshot attached showing what I see in the log viewer.
I have applied the patch, run yarn build, restarted everything, and cleared the
browser cache. I even shutdown KTD and restarted with the patch applied in case
it was a build thing.
Testing notes (using KTD):
1. Go to a staff interface.
2. Search for a patron and change their permissions (note their borrower
number) (for example: Henry Acevedo (2352935676), borrower
number 19, add and remove
"Access to all librarian functions (superlibrarian)"
3. Go to Tools > Additional tools > Log viewer.
4. Filter the log entries as follows:
- Module = Patrons
- Object = (for example, 19)
5. Click on submit:
==> There are no log entries for permission changes
6. Apply the patch, rebuild everything (yarn build), restart
everything (restart_all), and clear your browser cache.
7. Repeat steps 1 to 5:
==> Permission changes are now logged [1]
8. Check that the diff column in the database contains the full details
of the changes: [2]
select diff from action_logs where object= order by
action_id desc limit 1;
9. The tests pass:
9.1 prove t/db_dependent/Koha/Patron.t
9.2 prove t/db_dependent/Log.t
[1] Log viewer information:
See the screenshot:
Modify Henry Acevedo (19) {}
Modify Henry Acevedo (19)
{
"superlibrarian" : 1
}
[2]
select diff from action_logs where object=19;
+--+
| diff |
+--+
| {"D":{"superlibrarian":{"A":1}}} |
| {"D":{"superlibrarian":{"R":1}}} |
+--+
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #89 from Lisette Scheer --- (In reply to Ayoub Glizi-Vicioso from comment #88) > Updated patch to address QA and usability feedback: > > Readability: Enabled pretty JSON formatting in both the "Info" and > "Diff" columns to resolve table width issues. > > Clarity: Enhanced the "Diff" display by replacing raw 0/1 values with > explicit Added ✅ and Removed ❌ labels. > > Standards: Ran tidy script and fixed spelling errors to meet Koha coding > standards. This is looking great! I haven't finished testing, but I'd like another sign off since it's a big visual change. I think a lot of the pretty work could be adapted on 36698 and I've posted a screenshot over there to get some feedback for a wider adoption down the road. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #88 from Ayoub Glizi-Vicioso --- Updated patch to address QA and usability feedback: Readability: Enabled pretty JSON formatting in both the "Info" and "Diff" columns to resolve table width issues. Clarity: Enhanced the "Diff" display by replacing raw 0/1 values with explicit Added ✅ and Removed ❌ labels. Standards: Ran tidy script and fixed spelling errors to meet Koha coding standards. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #87 from Ayoub Glizi-Vicioso --- Created attachment 194534 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194534&action=edit Bug 20956: (QA follow-up) Fix display of permission logs -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Ayoub Glizi-Vicioso changed: What|Removed |Added Attachment #192610|0 |1 is obsolete|| --- Comment #86 from Ayoub Glizi-Vicioso --- Created attachment 194533 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194533&action=edit Bug 20956: Fix test failures Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Ayoub Glizi-Vicioso changed: What|Removed |Added Attachment #192609|0 |1 is obsolete|| --- Comment #85 from Ayoub Glizi-Vicioso --- Created attachment 194532 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194532&action=edit Bug 20956: fixup syntax error Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956
Ayoub Glizi-Vicioso changed:
What|Removed |Added
Attachment #192608|0 |1
is obsolete||
--- Comment #84 from Ayoub Glizi-Vicioso ---
Created attachment 194531
-->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194531&action=edit
Bug 20956: (QA follow-up) Match getuserflags() return structure
This patch changes Koha::Patron->permissions() to return the same
structure as C4::Auth::getuserflags() for architectural consistency.
Previously, permissions() only returned granted permissions:
{
catalogue => 1,
circulate => { override_renewals => 1 }
}
Now it returns ALL flags (matching getuserflags()):
{
superlibrarian => 0,
catalogue => 1,
circulate => { override_renewals => 1 },
borrowers => 0,
...
}
This provides several benefits:
1. Consistency - Matches established C4::Auth::getuserflags() pattern
2. Simpler checks - Can use "if ($flags->{module})" instead of
"if (exists $flags->{module})"
3. Future compatibility - Enables eventual deprecation of getuserflags()
in favor of the ORM-based $patron->permissions()
4. Helper compatibility - Works with existing _dispatch() logic
The logging functionality still works correctly - the diff will now
show explicit changes from 0 to 1 rather than absence to presence.
Test plan:
1. prove t/db_dependent/Koha/Patron.t
2. prove t/db_dependent/Log.t
3. Verify all tests pass
4. Confirm permissions() now returns all flags with 0/1 values
Signed-off-by: Martin Renvoize
Signed-off-by: David Nind
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Ayoub Glizi-Vicioso changed: What|Removed |Added Attachment #192607|0 |1 is obsolete|| --- Comment #83 from Ayoub Glizi-Vicioso --- Created attachment 194530 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194530&action=edit Bug 20956: (QA follow-up) Fix tests and refactor to use ORM This patch addresses several QA concerns: 1. Fixed duplicate subtest name - renamed second "is_superlibrarian() tests" to "permissions() tests" which accurately reflects what it tests 2. Fixed incorrect hash assignment - permissions() returns a hashref, not a hash. Changed `my %permissions` to `my $permissions` 3. Improved test coverage - expanded from 2 to 8 assertions covering: - Hashref return type verification - Single flag permission - No permissions - Multiple flags - Granular subpermissions from user_permissions table 4. Refactored permissions() method to use DBIx::Class ORM instead of raw SQL queries, following Koha::Object architecture principles All tests now pass without warnings. Test plan: 1. prove t/db_dependent/Koha/Patron.t 2. prove t/db_dependent/Log.t 3. Verify all tests pass 4. Confirm the test now properly validates the intended behavior Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Ayoub Glizi-Vicioso changed: What|Removed |Added Attachment #192606|0 |1 is obsolete|| --- Comment #82 from Ayoub Glizi-Vicioso --- Created attachment 194529 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194529&action=edit Bug 20956: (follow-up) Tidyness and fix no pod coverage in Koha/Patron.pm Signed-off-by: Hammat Wele Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Ayoub Glizi-Vicioso changed: What|Removed |Added Attachment #192605|0 |1 is obsolete|| --- Comment #81 from Ayoub Glizi-Vicioso --- Created attachment 194528 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194528&action=edit Bug 20956: (QA follow-up) Move subs to Koha::Patron, add unit tests Subroutines in members/member-flags.pl are combined and moved to Koha::Patron Removed the html-based english description of each permission from the logging Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Ayoub Glizi-Vicioso changed: What|Removed |Added Attachment #192604|0 |1 is obsolete|| --- Comment #80 from Ayoub Glizi-Vicioso --- Created attachment 194527 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194527&action=edit Bug 20956: BorrowersLog is not logging permission changes Changes to permissions are not logged in the Koha action_logs, making it impossible to trace changes to staff permissions. This patch ensures that all changes to permissions are logged. Test plan: 1. Go to a staff interface 2. Search for a patron and change their permissions (note their borrowernumber) 3. Go to tools/Log viewer 4. Filter by Module = Patrons, Object = . 5. Click on submit ==> No permission changes are logged. 6. Apply the patch 7. Repeat step 1, 2, 3, 4, 5 ==> Permission changes are now logged 8. Check that the diff column contains the change: select diff from action_logs where object= order by action_id desc limit 1; Signed-off-by: David Nind Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Ayoub Glizi-Vicioso changed: What|Removed |Added Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Caroline Cyr La Rose changed: What|Removed |Added Assignee|[email protected] |ayoub.glizi-vicioso@inLibro ||.com -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Johanna Räisä changed: What|Removed |Added CC||[email protected] -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956
Lisette Scheer changed:
What|Removed |Added
Status|Signed Off |Failed QA
--- Comment #79 from Lisette Scheer ---
There's a couple of failures from the Koha Testing Docker QA tool:
[FAIL] Koha/Patron.pm
WARN spelling
Exemple ==> Example
FAIL tidiness
File is not tidy, please run `perl misc/devel/tidy.pl
Koha/Patron.pm`
[PASS] members/member-flags.pl
[FAIL] t/db_dependent/Koha/Patron.t
FAIL tidiness
File is not tidy, please run `perl misc/devel/tidy.pl
t/db_dependent/Koha/Patron.t`
Additionally, if you have a lot of permissions (say all but one of the tool
permissions) it doesn't line break for a long time and makes the table way too
wide.
if you
USE JSON (qw to_json);
and within the logaction replace $permissions_after:
to_json($permissions_after, { utf8 => 1, pretty => 1, canonical => 1 })
similar to the members log around line 409, it does break nicely, but the diff
no longer works for me, so that's not the whole solution.
I highly recommend using Koha testing docker and running the QA script before
attaching patches as it can catch things that cause failures, as well as
tidying the commit as needed when you git commit.
You can find instructions at the gitlab and please feel free to reach out if
you need help setting it up.
https://gitlab.com/koha-community/koha-testing-docker
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Michelle Spinney changed: What|Removed |Added CC||[email protected] -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #78 from David Nind --- Apologies, I think I came across as a bit grumpy in the previous comment (now that I have re-read it) 8-(... -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956
--- Comment #77 from David Nind ---
I've signed off, but one thing I don't really understand is what should be
shown in the log viewer - versus what is recorded in the database.
For example:
1. I added and removed superlibrarian permissions for Henry.
2. What is shown in the log viewer vs what is in the database:
- Added:
. Log viewer: {"superlibrarian":1}
. Database diff column: {"D":{"superlibrarian":{"A":1}}}
- Removed:
. Log viewer: {}
. Database diff column: {"D":{"superlibrarian":{"R":1}}}
3. What is shown in the info column in the log viewer is almost
meaningless/useless for a "normal" staff patrons - unless you know what it
means, or can access the database/run a report to show the diff. Which, in my
view, defeats the purpose of logging permission changes. While you can show the
diff column, the info column should at least provide reasonably understandable
information on what was changed, for example: XYZ permission added or removed.
Also, as I am not a developer, I have no idea for the test patches how to:
- Confirm the test now properly validates the intended behavour
- Confirm permissions() now returns all flags with 0/1 values
Feel free to change back to needs sign off if this is something that you
require a tester to do.
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #76 from David Nind --- Created attachment 192610 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192610&action=edit Bug 20956: Fix test failures Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #75 from David Nind --- Created attachment 192609 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192609&action=edit Bug 20956: fixup syntax error Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956
--- Comment #74 from David Nind ---
Created attachment 192608
-->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192608&action=edit
Bug 20956: (QA follow-up) Match getuserflags() return structure
This patch changes Koha::Patron->permissions() to return the same
structure as C4::Auth::getuserflags() for architectural consistency.
Previously, permissions() only returned granted permissions:
{
catalogue => 1,
circulate => { override_renewals => 1 }
}
Now it returns ALL flags (matching getuserflags()):
{
superlibrarian => 0,
catalogue => 1,
circulate => { override_renewals => 1 },
borrowers => 0,
...
}
This provides several benefits:
1. Consistency - Matches established C4::Auth::getuserflags() pattern
2. Simpler checks - Can use "if ($flags->{module})" instead of
"if (exists $flags->{module})"
3. Future compatibility - Enables eventual deprecation of getuserflags()
in favor of the ORM-based $patron->permissions()
4. Helper compatibility - Works with existing _dispatch() logic
The logging functionality still works correctly - the diff will now
show explicit changes from 0 to 1 rather than absence to presence.
Test plan:
1. prove t/db_dependent/Koha/Patron.t
2. prove t/db_dependent/Log.t
3. Verify all tests pass
4. Confirm permissions() now returns all flags with 0/1 values
Signed-off-by: Martin Renvoize
Signed-off-by: David Nind
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #73 from David Nind --- Created attachment 192607 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192607&action=edit Bug 20956: (QA follow-up) Fix tests and refactor to use ORM This patch addresses several QA concerns: 1. Fixed duplicate subtest name - renamed second "is_superlibrarian() tests" to "permissions() tests" which accurately reflects what it tests 2. Fixed incorrect hash assignment - permissions() returns a hashref, not a hash. Changed `my %permissions` to `my $permissions` 3. Improved test coverage - expanded from 2 to 8 assertions covering: - Hashref return type verification - Single flag permission - No permissions - Multiple flags - Granular subpermissions from user_permissions table 4. Refactored permissions() method to use DBIx::Class ORM instead of raw SQL queries, following Koha::Object architecture principles All tests now pass without warnings. Test plan: 1. prove t/db_dependent/Koha/Patron.t 2. prove t/db_dependent/Log.t 3. Verify all tests pass 4. Confirm the test now properly validates the intended behavior Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #72 from David Nind --- Created attachment 192606 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192606&action=edit Bug 20956: (follow-up) Tidyness and fix no pod coverage in Koha/Patron.pm Signed-off-by: Hammat Wele Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #71 from David Nind --- Created attachment 192605 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192605&action=edit Bug 20956: (QA follow-up) Move subs to Koha::Patron, add unit tests Subroutines in members/member-flags.pl are combined and moved to Koha::Patron Removed the html-based english description of each permission from the logging Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #70 from David Nind --- Created attachment 192604 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192604&action=edit Bug 20956: BorrowersLog is not logging permission changes Changes to permissions are not logged in the Koha action_logs, making it impossible to trace changes to staff permissions. This patch ensures that all changes to permissions are logged. Test plan: 1. Go to a staff interface 2. Search for a patron and change their permissions (note their borrowernumber) 3. Go to tools/Log viewer 4. Filter by Module = Patrons, Object = . 5. Click on submit ==> No permission changes are logged. 6. Apply the patch 7. Repeat step 1, 2, 3, 4, 5 ==> Permission changes are now logged 8. Check that the diff column contains the change: select diff from action_logs where object= order by action_id desc limit 1; Signed-off-by: David Nind Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize Signed-off-by: David Nind -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #192512|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #192511|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #192510|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #192509|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #192508|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #192507|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Attachment #192506|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 David Nind changed: What|Removed |Added Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Ayoub Glizi-Vicioso changed: What|Removed |Added Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #69 from Ayoub Glizi-Vicioso --- Created attachment 192512 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192512&action=edit Bug 20956: Fix test failures -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Ayoub Glizi-Vicioso changed: What|Removed |Added Attachment #191478|0 |1 is obsolete|| --- Comment #68 from Ayoub Glizi-Vicioso --- Created attachment 192511 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192511&action=edit Bug 20956: fixup syntax error -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956
Ayoub Glizi-Vicioso changed:
What|Removed |Added
Attachment #191477|0 |1
is obsolete||
--- Comment #67 from Ayoub Glizi-Vicioso ---
Created attachment 192510
-->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192510&action=edit
Bug 20956: (QA follow-up) Match getuserflags() return structure
This patch changes Koha::Patron->permissions() to return the same
structure as C4::Auth::getuserflags() for architectural consistency.
Previously, permissions() only returned granted permissions:
{
catalogue => 1,
circulate => { override_renewals => 1 }
}
Now it returns ALL flags (matching getuserflags()):
{
superlibrarian => 0,
catalogue => 1,
circulate => { override_renewals => 1 },
borrowers => 0,
...
}
This provides several benefits:
1. Consistency - Matches established C4::Auth::getuserflags() pattern
2. Simpler checks - Can use "if ($flags->{module})" instead of
"if (exists $flags->{module})"
3. Future compatibility - Enables eventual deprecation of getuserflags()
in favor of the ORM-based $patron->permissions()
4. Helper compatibility - Works with existing _dispatch() logic
The logging functionality still works correctly - the diff will now
show explicit changes from 0 to 1 rather than absence to presence.
Test plan:
1. prove t/db_dependent/Koha/Patron.t
2. prove t/db_dependent/Log.t
3. Verify all tests pass
4. Confirm permissions() now returns all flags with 0/1 values
Signed-off-by: Martin Renvoize
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Ayoub Glizi-Vicioso changed: What|Removed |Added Attachment #191476|0 |1 is obsolete|| --- Comment #66 from Ayoub Glizi-Vicioso --- Created attachment 192509 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192509&action=edit Bug 20956: (QA follow-up) Fix tests and refactor to use ORM This patch addresses several QA concerns: 1. Fixed duplicate subtest name - renamed second "is_superlibrarian() tests" to "permissions() tests" which accurately reflects what it tests 2. Fixed incorrect hash assignment - permissions() returns a hashref, not a hash. Changed `my %permissions` to `my $permissions` 3. Improved test coverage - expanded from 2 to 8 assertions covering: - Hashref return type verification - Single flag permission - No permissions - Multiple flags - Granular subpermissions from user_permissions table 4. Refactored permissions() method to use DBIx::Class ORM instead of raw SQL queries, following Koha::Object architecture principles All tests now pass without warnings. Test plan: 1. prove t/db_dependent/Koha/Patron.t 2. prove t/db_dependent/Log.t 3. Verify all tests pass 4. Confirm the test now properly validates the intended behavior Signed-off-by: Martin Renvoize -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 Ayoub Glizi-Vicioso changed: What|Removed |Added Attachment #191475|0 |1 is obsolete|| --- Comment #65 from Ayoub Glizi-Vicioso --- Created attachment 192508 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192508&action=edit Bug 20956: (follow-up) Tidyness and fix no pod coverage in Koha/Patron.pm Signed-off-by: Hammat Wele Signed-off-by: Martin Renvoize -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
