From 1557218b9217c9a90580d00236b5b2e19dbeddeb Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 8 Oct 2025 13:41:33 -0400 Subject: [PATCH] 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 --- Koha/Patron.pm | 59 +++++++++++++++++++++++++++++++++++- members/member-flags.pl | 47 ++++------------------------ t/db_dependent/Koha/Patron.t | 24 ++++++++++++++- 3 files changed, 87 insertions(+), 43 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 4d94d0b765e..b708315301f 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -27,7 +27,7 @@ use Try::Tiny; use DateTime (); use C4::Log qw( logaction ); -use C4::Auth qw( checkpw_hash ); +use C4::Auth qw( checkpw_hash get_user_subpermissions ); use C4::Context; use C4::Letters qw( GetPreparedLetter EnqueueLetter SendQueuedMessages ); use C4::Log qw( logaction ); @@ -3434,6 +3434,63 @@ sub is_anonymous { return ( $anonymous_patron && $self->borrowernumber eq $anonymous_patron ) ? 1 : 0; } +=head3 get_permissions + +my $flags = $patron->get_permissons + +Returns a structure such as: +{ + "permissions": 1, + "borrowers": 1, + "circulate": { + "manage_curbside_pickups": 1, + "overdues_report": 1, + "override_renewals": 1, + "manage_restrictions": 1, + "manage_checkout_notes": 1, + "manage_bookings": 1 + }, + "catalogue": 1, + "reserveforothers": { + "place_holds": 1, + "modify_holds_priority": 1 + } +} +where a 1 indicates full permissions and a hash indicates +partial permissions with the given permissions as hash keys + +=cut + +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; + + my $active_flag = {}; + + foreach my $bit ( keys %$userflags ) { + if ( $flags & ( 1 << $bit ) ) { + my $flag = $userflags->{$bit}; + $active_flag->{$flag} = 1; + } + } + + my $user_perms = get_user_subpermissions( $self->userid ); + + for my $module ( keys %$user_perms ) { + for my $code ( keys %{ $user_perms->{$module} } ) { + $active_flag->{$module}->{$code} = 1; + } + } + + return %$active_flag; +} + =head2 Internal methods =head3 _type diff --git a/members/member-flags.pl b/members/member-flags.pl index 42f04bf9828..0c48d16e54a 100755 --- a/members/member-flags.pl +++ b/members/member-flags.pl @@ -70,19 +70,18 @@ if ( $op eq 'cud-newflags' ) { # construct flags my $module_flags = 0; - my $sth = $dbh->prepare("SELECT bit,flag,flagdesc FROM userflags ORDER BY bit"); + my $sth = $dbh->prepare("SELECT bit,flag FROM userflags ORDER BY bit"); $sth->execute(); - my %userflags; - while ( my ( $bit, $flag, $flagdesc ) = $sth->fetchrow_array ) { + while ( my ( $bit, $flag ) = $sth->fetchrow_array ) { if ( exists $all_module_perms{$flag} ) { $module_flags += 2**$bit; } - $userflags{$bit} = [ $flag, $flagdesc ]; } + my %permissions_before = $patron->permissions(); + $sth = $dbh->prepare("UPDATE borrowers SET flags=? WHERE borrowernumber=?"); - my $old_flags = $patron->flags // 0; - my %permissions_before = get_permissions( \%userflags, $old_flags ); + my $old_flags = $patron->flags // 0; if ( ( $old_flags == 1 || $module_flags == 1 ) && $old_flags != $module_flags ) { @@ -109,7 +108,7 @@ if ( $op eq 'cud-newflags' ) { } } - my %permissions_after = get_permissions( \%userflags, $module_flags ); + my %permissions_after = $patron->get_from_storage->permissions(); logaction( 'MEMBERS', 'MODIFY', $member, \%permissions_after, undef, \%permissions_before ); print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=$member"); } else { @@ -204,38 +203,4 @@ if ( $op eq 'cud-newflags' ) { ); output_html_with_http_headers $input, $cookie, $template->output; - -} - -sub get_permissions { - my ( $userflags, $old_flags ) = @_; - - my %active_flag = decode_flags( $userflags, $old_flags ); - my $user_perms = get_user_subpermissions( $bor->{'userid'} ); - my %permissions; - my $dbh = C4::Context->dbh(); - my $sth = $dbh->prepare("SELECT code, description FROM permissions"); - $sth->execute(); - - while ( my ( $code, $desc ) = $sth->fetchrow_array ) { - $permissions{$code} = $desc; - } - for my $module ( keys %$user_perms ) { - for my $code ( keys %{ $user_perms->{$module} } ) { - $active_flag{$code} = $permissions{$code}; - } - } - return %active_flag; -} - -sub decode_flags { - my ( $userflags, $flags ) = @_; - my %active_flags; - foreach my $bit ( keys %$userflags ) { - if ( $flags & ( 2**$bit ) ) { - my ( $flag, $desc ) = @{ $userflags->{$bit} }; - $active_flags{$flag} = $desc; - } - } - return %active_flags; } diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index ae9e999c954..a5a9b031bba 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 => 42; +use Test::More tests => 43; use Test::NoWarnings; use Test::Exception; use Test::Warn; @@ -806,6 +806,28 @@ subtest 'is_superlibrarian() tests' => sub { $schema->storage->txn_rollback; }; +subtest 'is_superlibrarian() tests' => sub { + + plan tests => 2; + + $schema->storage->txn_begin; + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + + value => { flags => 4 } + } + ); + + my %permissions = $patron->permissions; + + is( scalar keys %permissions, 1, "Patron has one module permission" ); + is( $permissions{catalogue}, 1, "Patron has catalogue permission" ); + + $schema->storage->txn_rollback; +}; + subtest 'extended_attributes' => sub { plan tests => 16; -- 2.39.5 (Apple Git-154)