Bugzilla – Attachment 190689 Details for
Bug 20956
BorrowersLog is not logging permission changes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20956: (QA follow-up) Fix tests and refactor to use ORM
9ded760.patch (text/plain), 5.42 KB, created by
Martin Renvoize (ashimema)
on 2025-12-22 17:14:46 UTC
(
hide
)
Description:
Bug 20956: (QA follow-up) Fix tests and refactor to use ORM
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-12-22 17:14:46 UTC
Size:
5.42 KB
patch
obsolete
>From 9ded76004b781cfacadd365a9c80fed9c5f2895e Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Mon, 22 Dec 2025 16:38:42 +0000 >Subject: [PATCH] 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 <martin.renvoize@openfifth.co.uk> >--- > Koha/Patron.pm | 17 +++++++------ > t/db_dependent/Koha/Patron.t | 48 ++++++++++++++++++++++++++++++------ > 2 files changed, 50 insertions(+), 15 deletions(-) > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index a0da88e4001..73a504da31f 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -3598,22 +3598,23 @@ partial permissions with the given permissions as hash keys > sub permissions { > my ($self) = @_; > >- my $dbh = C4::Context->dbh; >- my $sth = $dbh->prepare("SELECT bit, flag FROM userflags ORDER BY bit"); >- $sth->execute(); >- my $userflags = { map { $_->[0] => $_->[1] } @{ $sth->fetchall_arrayref() } }; >- >- my $flags = $self->flags // 0; >+ # Get userflags using ORM >+ my $schema = Koha::Database->schema; >+ my @userflags = $schema->resultset('Userflag')->search( {}, { order_by => 'bit' } )->all; >+ my %userflags_map = map { $_->bit => $_->flag } @userflags; > >+ my $flags = $self->flags // 0; > my $active_flags = {}; > >- foreach my $bit ( keys %$userflags ) { >+ # Check which flags are active based on bit values >+ foreach my $bit ( keys %userflags_map ) { > if ( $flags & ( 1 << $bit ) ) { >- my $flag = $userflags->{$bit}; >+ my $flag = $userflags_map{$bit}; > $active_flags->{$flag} = 1; > } > } > >+ # Get granular subpermissions using existing helper > my $user_perms = get_user_subpermissions( $self->userid ); > > for my $module ( keys %$user_perms ) { >diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t >index be5875acf1e..d58fcf4fbb9 100755 >--- a/t/db_dependent/Koha/Patron.t >+++ b/t/db_dependent/Koha/Patron.t >@@ -19,7 +19,7 @@ > > use Modern::Perl; > >-use Test::More tests => 45; >+use Test::More tests => 46; > use Test::NoWarnings; > use Test::Exception; > use Test::Warn; >@@ -806,24 +806,58 @@ subtest 'is_superlibrarian() tests' => sub { > $schema->storage->txn_rollback; > }; > >-subtest 'is_superlibrarian() tests' => sub { >+subtest 'permissions() tests' => sub { > >- plan tests => 2; >+ plan tests => 8; > > $schema->storage->txn_begin; > >+ my $dbh = C4::Context->dbh; >+ >+ # Test patron with single flag permission (catalogue = bit 2, value 4) > my $patron = $builder->build_object( > { > class => 'Koha::Patrons', >- > value => { flags => 4 } > } > ); > >- my %permissions = $patron->permissions; >+ my $permissions = $patron->permissions; > >- is( scalar keys %permissions, 1, "Patron has one module permission" ); >- is( $permissions{catalogue}, 1, "Patron has catalogue permission" ); >+ is( ref($permissions), 'HASH', "permissions() returns a hashref" ); >+ is( scalar keys %$permissions, 1, "Patron has one module permission" ); >+ is( $permissions->{catalogue}, 1, "Patron has catalogue permission" ); >+ >+ # Test patron with no permissions >+ my $patron_no_perms = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { flags => 0 } >+ } >+ ); >+ my $no_perms = $patron_no_perms->permissions; >+ is( scalar keys %$no_perms, 0, "Patron with no flags has no permissions" ); >+ >+ # Test patron with multiple flags (catalogue=4 + circulate=2 = 6) >+ my $patron_multi = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { flags => 6 } >+ } >+ ); >+ my $multi_perms = $patron_multi->permissions; >+ is( $multi_perms->{catalogue}, 1, "Multi-flag patron has catalogue permission" ); >+ is( $multi_perms->{circulate}, 1, "Multi-flag patron has circulate permission" ); >+ >+ # Test patron with granular subpermissions >+ my $patron_subperm = $builder->build_object( { class => 'Koha::Patrons', value => { flags => 0 } } ); >+ $dbh->do( >+ "INSERT INTO user_permissions (borrowernumber, module_bit, code) VALUES (?, ?, ?)", >+ undef, $patron_subperm->borrowernumber, 1, 'override_renewals' >+ ); >+ my $subperms = $patron_subperm->get_from_storage->permissions; >+ is( ref( $subperms->{circulate} ), 'HASH', "Granular permissions stored as hashref" ); >+ is( $subperms->{circulate}->{override_renewals}, 1, "Subpermission correctly detected" ); > > $schema->storage->txn_rollback; > }; >-- >2.52.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20956
:
182955
|
185483
|
185491
|
186388
|
187592
|
187593
|
187594
|
188260
|
188261
|
188262
|
188263
|
190686
|
190687
|
190688
| 190689 |
190690