[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/
[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 #191474|0 |1 is obsolete|| --- Comment #64 from Ayoub Glizi-Vicioso --- Created attachment 192507 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192507&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 -- 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 #191473|0 |1 is obsolete|| --- Comment #63 from Ayoub Glizi-Vicioso --- Created attachment 192506 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192506&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 -- 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 |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 #62 from Martin Renvoize (ashimema) --- I'll take a look.. it's not even 3 weeks since I worked on this , yet it's already a distant memory. Hopefully it all comes flooding back once I'm in the code again. -- 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 |Failed QA
--- Comment #61 from David Nind ---
The tests for prove t/db_dependent/Koha/Patron.t fail for me (latest main):
prove t/db_dependent/Koha/Patron.t
t/db_dependent/Koha/Patron.t .. 6/46
# Failed test 'Patron has catalogue permission'
# at t/db_dependent/Koha/Patron.t line 826.
# got: undef
# expected: '1'
# Looks like you failed 1 test of 2.
# Failed test 'is_superlibrarian() tests'
# at t/db_dependent/Koha/Patron.t line 829.
t/db_dependent/Koha/Patron.t .. 46/46
# Failed test 'no warnings'
# at /usr/share/perl/5.36/Test/Builder.pm line 193.
# There were 1 warning(s)
# Previous test 0 ''
# Reference found where even-sized list expected at
t/db_dependent/Koha/Patron.t line 823.
# at t/db_dependent/Koha/Patron.t line 823.
# main::__ANON__() called at /usr/share/perl/5.36/Test/Builder.pm line
374
# eval {...} called at /usr/share/perl/5.36/Test/Builder.pm line 374
# Test::Builder::subtest(Test::Builder=HASH(0x5fc68e942028),
"is_superlibrarian() tests", CODE(0x5fc68e7698f0)) called at
/usr/share/perl/5.36/Test/More.pm line 809
# Test::More::subtest("is_superlibrarian() tests", CODE(0x5fc68e7698f0))
called at t/db_dependent/Koha/Patron.t line 829
#
# Looks like you planned 46 tests but ran 47.
# Looks like you failed 2 tests of 47 run.
t/db_dependent/Koha/Patron.t .. Dubious, test returned 2 (wstat 512, 0x200)
Failed 1/46 subtests
Test Summary Report
---
t/db_dependent/Koha/Patron.t (Wstat: 512 (exited 2) Tests: 47 Failed: 2)
Failed tests: 11, 47
Non-zero exit status: 2
Parse errors: Bad plan. You planned 46 tests but ran 47.
Files=1, Tests=47, 44 wallclock secs ( 0.09 usr 0.02 sys + 28.55 cusr 13.86
csys = 42.52 CPU)
Result: FAIL
--
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|Patch doesn't apply |Needs Signoff CC||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 Ayoub Glizi-Vicioso changed: What|Removed |Added Attachment #191472|0 |1 is obsolete|| --- Comment #60 from Ayoub Glizi-Vicioso --- Created attachment 191478 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191478&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 #190690|0 |1
is obsolete||
--- Comment #59 from Ayoub Glizi-Vicioso ---
Created attachment 191477
-->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191477&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 #190689|0 |1 is obsolete|| --- Comment #58 from Ayoub Glizi-Vicioso --- Created attachment 191476 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191476&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 #190688|0 |1 is obsolete|| --- Comment #57 from Ayoub Glizi-Vicioso --- Created attachment 191475 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191475&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/
[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 #190687|0 |1 is obsolete|| --- Comment #56 from Ayoub Glizi-Vicioso --- Created attachment 191474 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191474&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 -- 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 #190686|0 |1 is obsolete|| --- Comment #55 from Ayoub Glizi-Vicioso --- Created attachment 191473 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191473&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 -- 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 #54 from Ayoub Glizi-Vicioso --- Created attachment 191472 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191472&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 Lisette Scheer changed: What|Removed |Added QA Contact|[email protected]|[email protected] ||m Status|Signed Off |Patch doesn't apply --- Comment #53 from Lisette Scheer --- Patch doesn't apply today. -- 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 #52 from David Nind --- Still sitting as "Signed Off", should this be "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 --- Comment #51 from Martin Renvoize (ashimema) --- Any interest here now the xmas quiet period has come to an end? -- 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 CC||[email protected] ||, ||[email protected] ||.uk, ||lucy.vaux-harvey@openfifth. ||co.uk -- 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|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 #50 from Martin Renvoize (ashimema)
---
Created attachment 190690
-->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190690&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 --- Comment #49 from Martin Renvoize (ashimema) --- Created attachment 190689 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190689&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 --- Comment #48 from Martin Renvoize (ashimema) --- Created attachment 190688 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190688&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/
[Koha-bugs] [Bug 20956] BorrowersLog is not logging permission changes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20956 --- Comment #47 from Martin Renvoize (ashimema) --- Created attachment 190687 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190687&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 -- 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 #46 from Martin Renvoize (ashimema) --- Created attachment 190686 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190686&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 -- 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 #188263|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 #188261|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 #188260|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 Status|Failed QA |Needs Signoff Patch complexity|--- |Small patch -- 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 #45 from Martin Renvoize (ashimema)
---
(In reply to Marcel de Rooy from comment #44)
> Koha::Patron
>
> +sub permissions {
> +my ($self) = @_;
> +
> +my $dbh = C4::Context->dbh;
> +my $sth = $dbh->prepare("SELECT bit, flag FROM userflags ORDER BY bit");
> +$sth->execute();
>
> The idea of Koha::Object[s] was to remove SQL statements. So we should not
> just add them here.
> We could create a simple Koha object for userflags and permissions?
> Or we can use Koha::Database->new->schema->resultset(..) as is done
> elsewhere in this module too.
We have Koha::Auth::Permissions already, though I'm not entirely sold on it's
semantics either, to be honest. I'd be tempted to start migrating the perms
code into Koha::Patron or possibly Koha::Patron::Permissions?
I do think there's more work needed here, regardless.. I'm going to post a
couple of follow-ups shortly and maybe the rest can be submitted as follow-up
bugs
--
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 Marcel de Rooy changed: What|Removed |Added Status|BLOCKED |Failed 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
--- Comment #44 from Marcel de Rooy ---
Koha::Patron
+sub permissions {
+my ($self) = @_;
+
+my $dbh = C4::Context->dbh;
+my $sth = $dbh->prepare("SELECT bit, flag FROM userflags ORDER BY bit");
+$sth->execute();
The idea of Koha::Object[s] was to remove SQL statements. So we should not just
add them here.
We could create a simple Koha object for userflags and permissions?
Or we can use Koha::Database->new->schema->resultset(..) as is done elsewhere
in this module too.
--
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 #43 from Marcel de Rooy --- Tests are failing: * Proving /usr/share/koha/t/db_dependent/Koha/Patron.t FAIL # Failed test 'Patron has catalogue permission' # at /usr/share/koha/t/db_dependent/Koha/Patron.t line 826. # got: undef # expected: '1' # Looks like you failed 1 test of 2. # Failed test 'is_superlibrarian() tests' # at /usr/share/koha/t/db_dependent/Koha/Patron.t line 829. # Failed test 'no warnings' # at /usr/share/perl/5.36/Test/Builder.pm line 193. # There were 1 warning(s) # Previous test 0 '' # Reference found where even-sized list expected at /usr/share/koha/t/db_dependent/Koha/Patron.t line 823. -- 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 Marcel de Rooy changed: What|Removed |Added QA Contact|[email protected] |[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 Marcel de Rooy changed: What|Removed |Added Status|Signed Off |BLOCKED --- Comment #42 from Marcel de Rooy --- Looking here -- 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 CC||[email protected] ||o.uk -- 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 Hammat wele changed: What|Removed |Added Attachment #188262|0 |1 is obsolete|| --- Comment #41 from Hammat wele --- Created attachment 188263 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188263&action=edit Bug 20956: (follow-up) Tidyness and fix no pod coverage in Koha/Patron.pm Signed-off-by: Hammat Wele -- 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 Hammat wele changed: What|Removed |Added Status|Failed QA |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 #40 from Hammat wele --- Created attachment 188262 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188262&action=edit Bug 20956: (follow-up) Tidyness and fix no pod coverage in Koha/Patron.pm -- 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 Hammat wele changed: What|Removed |Added Attachment #187594|0 |1 is obsolete|| --- Comment #39 from Hammat wele --- Created attachment 188261 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188261&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 -- 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 Hammat wele changed: What|Removed |Added Attachment #187592|0 |1 is obsolete|| --- Comment #38 from Hammat wele --- Created attachment 188260 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188260&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 -- 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 #32 from Caroline Cyr La Rose --- The coding guideline was briefly discussed at the last dev meeting (https://koha-hedgedoc.servers.llownd.net/Development_IRC_meeting_24_September_2025) but nothing came of it as far as I could tell. -- 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 Kyle M Hall (khall) changed: What|Removed |Added QA Contact|[email protected] |[email protected] |y.org | -- 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 Lucas Gass (lukeg) changed: What|Removed |Added Status|Passed QA |Failed QA CC||[email protected] --- Comment #37 from Lucas Gass (lukeg) --- The sub introduced in Patron.pm is called permissions but the POD says: =head3 get_permissions -- 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 #34 from Kyle M Hall (khall) --- Created attachment 187593 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187593&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 -- 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
Kyle M Hall (khall) changed:
What|Removed |Added
Status|In Discussion |Passed QA
--- Comment #35 from Kyle M Hall (khall) ---
(In reply to Jonathan Druart from comment #29)
> I have not followed the last changes in this area (logging), but I don't
> think you should take care of the changes in C4/Logs.pm
>
> Well, I do think we should centralize it, but not without discussion and
> advertising.
>
>
>
> For instance, here is the call from Koha::Patron->store
> to_json(
> $info,
> { utf8 => 1, pretty => 1, canonical => 1 }
> )
>
> It's bad (to_json is slower), but it uses pretty and canonical, what you
> don't.
As lisette referenced, this topic was discussed and approved via bug 25159 and
the corresponding ACTN1 coding guideline which was approved at a developers
meeting.
A good next step would be to centralize permissions changes within Koha::Patron
but I think that is out of scope for this bug ( but would be a great next step
).
--
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 Kyle M Hall (khall) changed: What|Removed |Added Attachment #187593|0 |1 is obsolete|| --- Comment #36 from Kyle M Hall (khall) --- Created attachment 187594 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187594&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 -- 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 Kyle M Hall (khall) changed: What|Removed |Added Attachment #186388|0 |1 is obsolete|| --- Comment #33 from Kyle M Hall (khall) --- Created attachment 187592 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187592&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 -- 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 Marcel de Rooy changed: What|Removed |Added Status|Signed Off |In Discussion --- Comment #31 from Marcel de Rooy --- In view of former comments, changing status to In discussion. Please provide arguments to get this further while addressing raised questions. -- 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 #30 from Lisette Scheer --- (In reply to Jonathan Druart from comment #29) > I have not followed the last changes in this area (logging), but I don't > think you should take care of the changes in C4/Logs.pm The coding guidelines for action logs https://wiki.koha-community.org/wiki/Coding_Guidelines#ACTN1:_New_additions_to_actions_logs_should_use_the_JSON_Diff_format which was added after https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25159 does appear to use C4/Log Should we add this to the agenda for the September 24 dev meeting? -- 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 #29 from Jonathan Druart ---
I have not followed the last changes in this area (logging), but I don't think
you should take care of the changes in C4/Logs.pm
Well, I do think we should centralize it, but not without discussion and
advertising.
For instance, here is the call from Koha::Patron->store
to_json(
$info,
{ utf8 => 1, pretty => 1, canonical => 1 }
)
It's bad (to_json is slower), but it uses pretty and canonical, what you don't.
--
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 Hammat wele changed: What|Removed |Added Status|Failed QA |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 Hammat wele changed: What|Removed |Added Attachment #185491|0 |1 is obsolete|| --- Comment #28 from Hammat wele --- Created attachment 186388 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186388&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 -- 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 Marcel de Rooy changed: What|Removed |Added Status|Signed Off |Failed QA CC||[email protected] --- Comment #27 from Marcel de Rooy --- [FAIL] C4/Log.pm SKIP spelling FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl C4/Log.pm` [FAIL] members/member-flags.pl SKIP spelling FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl members/member-flags.pl` [FAIL] t/db_dependent/Log.t SKIP spelling FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl t/db_dependent/Log.t` -- 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 CC||[email protected] --- Comment #26 from David Nind --- Testing notes (using KTD): 1. For step 2 of the test plan, is the actual borrower number of the patron you changed. For me, I changed the permissions for Henry Acevedo - the borrow number to enter in the object field is 19. 2. For step 8 of the test plan, what shows in the log viewer and the SQL results: 2.1 If I add superlibrarian permissions to Henry: - The log viewer info column shows: {"superlibrarian":"Access to all librarian functions"} - The database query result: {"D":{"superlibrarian":{"A":"Access to all librarian functions"}}} 2.2 If I remove superlibrarian permissions from Henry: - The log viewer info column shows: {} - The database query result: {"D":{"superlibrarian":{"R":"Access to all librarian functions"}}} -- 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 #185483|0 |1 is obsolete|| --- Comment #25 from David Nind --- Created attachment 185491 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185491&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 -- 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 Hammat wele 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 Hammat wele changed: What|Removed |Added Attachment #182955|0 |1 is obsolete|| --- Comment #24 from Hammat wele --- Created attachment 185483 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185483&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; -- 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 Laura O'Neil 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|Needs Signoff |Failed QA
--- Comment #23 from Lisette Scheer ---
Great start! I'm glad to see this is moving.
A few issues:
The action log should also follow the coding guideline for the diff column:
https://wiki.koha-community.org/wiki/Coding_Guidelines#ACTN1:_New_additions_to_actions_logs_should_use_the_JSON_Diff_format
Per Comment 10 it shouldn't be toggleable, so it should not follow the
borrowers log sys pref.
Additionally only the top level permissions are saved, not if there are changes
to subpermissions. You'll get something like this:
{ "flags" : { "after" : 32, "before" : 32 } }
Please see also the discussion in comments 14-17 about how those
flags/subpermissions should ideally display as readable versions instead of
just numbers that need to be decoded.
I'm really excited about this getting some movement.
--
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 #22 from Blou --- This doesn't need to depend on Bug 20813 for a quick, simple solution. I removed the dependency -- 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 Blou changed: What|Removed |Added Depends on|20813 | Status|NEW |Needs Signoff Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20813 [Bug 20813] Revamp user permissions system -- 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 Hammat wele changed: What|Removed |Added Assignee|[email protected] |[email protected] |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug. ___ 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 #21 from Hammat wele --- Created attachment 182955 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=182955&action=edit Bug 20956: BorrowersLog is not logging permission changes Changes to the flags field in the borrowers table are currently not logged in the Koha action_logs, making it impossible to trace changes to staff permissions. This represents a security issue. 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 = borrowernumber. 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 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug. ___ 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 Hammat wele changed: What|Removed |Added CC||caroline.cyr-la-rose@inlibr ||o.com, ||[email protected], ||[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 Chris Rowlands 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 Chip Halvorsen changed: What|Removed |Added CC||Chip.Halvorsen@WestlakeLibr ||ary.org -- 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 Cook changed: What|Removed |Added Severity|major |normal -- You are receiving this mail because: You are the assignee for the bug. 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 Rebecca Coert 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 George Williams (NEKLS) 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 Katie Bliss 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 #20 from David Cook --- (In reply to Lisette Scheer from comment #19) > Now that we store the diff in the json, maybe we can tie this into Bug 36698 > to make a nice diff view of permission changes? Yeah, that could be nice. I think in the past I was concerned about using action_logs since they can be truncated periodically, but something would also be better than nothing, and this would be a nice way to do it. -- You are receiving this mail because: You are the assignee for the bug. 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 #19 from Lisette Scheer --- Now that we store the diff in the json, maybe we can tie this into Bug 36698 to make a nice diff view of permission changes? -- You are receiving this mail because: You are the assignee for the bug. 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 #18 from Fiona Borthwick --- Would like to resurrect this one please. Not storing changes to patron permissions is a security concern. We are also finding this requirement coming up in tenders. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug. ___ 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 Fiona Borthwick changed: What|Removed |Added CC||fiona.borthwick@ptfs-europe ||.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 --- Comment #17 from Katrin Fischer --- I agree that we need a nice display in the log viewer at least, if we store it somewhat machine readable I would not mind that much as long as it display nicely in the GUI. Please also keep translatability in mind (also a pro for a template side solution). -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug. ___ 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 #16 from Christopher Brannon --- (In reply to Magnus Enger from comment #14) > How verbose should we do the logging? As far as I can tell, the information > about permissions is stored in a combination of borrowers.flags and the > user_permissions table. > > The information from the user_permissions table could probably be logged as > is, and be pretty readable. > > But the flags is just a number, that needs to be interpreted in some way to > make sense. Should we log just the number, or the interpretation? If this is a log that a non-admin should be able to interpret, it should be the interpretation. You could log both, especially for debugging, but the interpretation should definitely be there. Why put only the number and make people jump through hoops to interpret if the system can do it without sacrificing performance? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug. ___ 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 #15 from Kyle M Hall --- (In reply to Magnus Enger from comment #14) > How verbose should we do the logging? As far as I can tell, the information > about permissions is stored in a combination of borrowers.flags and the > user_permissions table. > > The information from the user_permissions table could probably be logged as > is, and be pretty readable. > > But the flags is just a number, that needs to be interpreted in some way to > make sense. Should we log just the number, or the interpretation? My initial concern was performance, but considering user permissions changes are not a frequent activity I think either solution is reasonable. Either decode it and store it as a list of key/value pairs or just log the binary representation with a link to a wiki page where we can explain how to decode it. -- You are receiving this mail because: You are the assignee for the bug. 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/
