From a6fcbcd40a71854c879c612f9b4885e554680d22 Mon Sep 17 00:00:00 2001 From: William Lavoie Date: Thu, 13 Feb 2025 09:50:59 -0500 Subject: [PATCH] Bug 38040: Adding server-side verification Added a system preference 'IndependentBranchesHolds' and server-side verification in the perl scripts Test plan: 1. Set IndependentBranches, IndependentBranchesPatronModifications, IndependentBranchesTransfers and IndependentBranchesHolds to 'Yes'. 2. Set canreservefromotherbranches to "Don't allow (with independent branches)". 3. Give limited permissions to a staff patron - circulate - catalogue - delete_borrowers - edit_borrowers - list_borrowers - send_messages_to_borrowers - reserveforothers - updatecharges 4. As the superlibrarian, place several holds on a record with items from various branches. 5. Log in as the restricted staff 6. View the holds for the record --> Some will say "A patron from library XX" instead of the name -- OK 7. Click the trash bin icon to cancel a hold from another library --> Hold is cancelled -- NOT OK 8. Apply the patch and update the database 9. Validate that step 7 is not possible now and that holds from the staff's library can be modified or deleted. --- ..._38040-IndependentBranchesHolds_syspref.pl | 14 ++++ installer/data/mysql/mandatory/sysprefs.sql | 1 + .../prog/en/includes/holds_table.inc | 10 ++- .../en/modules/admin/preferences/admin.pref | 7 ++ .../prog/en/modules/reserve/request.tt | 15 +++-- reserve/request.pl | 65 ++++++++++++------- 6 files changed, 80 insertions(+), 32 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_38040-IndependentBranchesHolds_syspref.pl diff --git a/installer/data/mysql/atomicupdate/bug_38040-IndependentBranchesHolds_syspref.pl b/installer/data/mysql/atomicupdate/bug_38040-IndependentBranchesHolds_syspref.pl new file mode 100644 index 0000000000..a878b5d57f --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_38040-IndependentBranchesHolds_syspref.pl @@ -0,0 +1,14 @@ +use Modern::Perl; +use Koha::Installer::Output qw(say_warning say_success say_info); + +return { + bug_number => "38040", + description => "Prevent staff (but not superlibrarians) from modifying holds from other libraries:", + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + $dbh->do(q{INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ('IndependentBranchesHolds', '1', NULL, 'Prevent staff (but not superlibrarians) from modifying holds from other libraries', 'YESNO')}); + say_success( $out, "Added new system preference 'IndependentBranchesHolds'" ); + }, +}; diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 682b9fb950..d826f1aad2 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -335,6 +335,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('IndependentBranches','0',NULL,'If ON, increases security between libraries','YesNo'), ('IndependentBranchesPatronModifications','0', NULL, 'Show only modification request for the logged in branch','YesNo'), ('IndependentBranchesTransfers','0', NULL, 'Allow non-superlibrarians to transfer items between libraries','YesNo'), +('IndependentBranchesHolds','0', NULL, 'Allow non-superlibrarians to modify holds between libraries','YesNo'), ('intranet_includes','includes',NULL,'The includes directory you want for specific look of Koha (includes or includes_npl for example)','Free'), ('IntranetAddMastheadLibraryPulldown','0', NULL, 'Add a library select pulldown menu on the staff header search','YesNo'), ('IntranetBiblioDefaultView','normal','normal|marc|isbd|labeled_marc','Choose the default detail view in the staff interface; choose between normal, labeled_marc, marc or isbd','Choice'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc index c2d717a9fe..a2b2ffd1ad 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc @@ -33,7 +33,13 @@ [% SET all_priorities = [] %] [% FOREACH hold IN holds %] - [% all_priorities.push( hold.priority ) %] + [% IF CAN_user_superlibrarian || !Koha.Preference('IndependentBranchesHolds') %] + [% all_priorities.push( hold.priority ) %] + [% ELSE %] + [% IF Branches.GetLoggedInBranchcode == hold.branchcode %] + [% all_priorities.push( hold.priority ) %] + [% END %] + [% END %] [% END %] @@ -42,7 +48,7 @@ [%- first_priority = hold.priority -%] [%- found_holds = loop.index() -%] [%- END -%] - [%- IF Koha.Preference('HoldsSplitQueueNumbering') == 'actual' -%] + [%- IF Koha.Preference('HoldsSplitQueueNumbering') == 'actual' || (Koha.Preference('IndependentBranchesHolds') && !CAN_user_superlibrarian) -%] [%- this_priority = hold.priority -%] [%- ELSE -%] [%- this_priority = loop.count() - found_holds -%] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref index 18d21a955a..71ebf98b7e 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref @@ -148,6 +148,13 @@ Administration: choices: 1: "Yes" 0: "No" + - + - "Prevent staff (but not superlibrarians) from modifying holds from other libraries: " + - pref: IndependentBranchesHolds + default: 0 + choices: + 1: "Yes" + 0: "No" - - pref: ForceLibrarySelection choices: diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index 8c40294d01..b6b0df6941 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -1253,7 +1253,7 @@ [% END %] - [% IF Koha.Preference('HoldsSplitQueue') == 'branch' %] + [% IF Koha.Preference('HoldsSplitQueue') == 'branch' || ((!CAN_user_superlibrarian) && Koha.Preference('IndependentBranchesHolds'))%] [% SET branchcodes = [] %] @@ -1274,7 +1274,7 @@ [% END %]

[% Branches.GetName( b ) | html %]

- + [% INCLUDE holds_table.inc holds=holds_by_branch %]
[% END # /FOREACh b %] @@ -1806,7 +1806,7 @@ return false; }); - [% IF (!CAN_user_superlibrarian) %] + [% IF ((!CAN_user_superlibrarian) || Koha.Preference('IndependentBranchesHolds')) %] var user_branch = "[% Branches.GetLoggedInBranchcode %]"; if(user_branch !== undefined && user_branch !== ""){ $('#patron_holds_table tbody tr').each(function() { @@ -1814,10 +1814,13 @@ var rankRequest = $(this).find("select[name='rank-request']"); if (!link.length){ $(this).find('.cancel-hold, .fa-trash').addClass('disabled_trash'); + $(this).find('.select_hold, .rank-request, td a[data-op="cud-move"], .flatpickr, .toggle-suspend, .clear_date, [name*="change_hold_type_"]').addClass('disabled').attr('disabled', 'disabled'); + $(this).find('.select_hold_all, .hold-arrow').addClass('disabled').attr('disabled', 'disabled'); + $(this).find('.suspend-hold').prop('disabled', true); + $(this).find('.icon-move-hold-top').css('border-top', '2px solid #999999'); + $(this).find('.icon-move-hold-bottom').css('border-bottom', '2px solid #999999'); } - $(this).find('.select_hold, .rank-request, td a[data-op="cud-move"], .flatpickr, .toggle-suspend, .clear_date, [name*="change_hold_type_"]').addClass('disabled').attr('disabled', 'disabled'); - $('.select_hold_all, .hold-arrow').addClass('disabled').attr('disabled', 'disabled'); - $('.suspend-hold').prop('disabled', true); + }); } [% END %] diff --git a/reserve/request.pl b/reserve/request.pl index 4aa035c994..239c15de92 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -66,7 +66,7 @@ my ( $template, $borrowernumber, $cookie, $flags ) = get_template_and_user( ); my $is_super_librarian = C4::Context->IsSuperLibrarian(); -my $user_branch = C4::Context->preference("IndependentBranches") ? C4::Context->userenv->{'branch'}: undef; +my $user_branch = C4::Context->preference("IndependentBranchesHolds") ? C4::Context->userenv->{'branch'}: undef; $template->param( is_super_librarian => $is_super_librarian, @@ -97,8 +97,9 @@ my $warnings; my $messages; my $exceeded_maxreserves; my $exceeded_holds_per_record; -my @failed_holds = $input->multi_param('failed_holds'); -my $form_submitted = $input->param('form_submitted'); +my @failed_holds = $input->multi_param('failed_holds'); +my $form_submitted = $input->param('form_submitted'); +my $can_modify_all_holds = $is_super_librarian || !defined($user_branch); my $op = $input->param('op') || q{}; @@ -110,44 +111,60 @@ if ( $op eq 'cud-move' ) { my $first_priority = $input->param('first_priority'); my $last_priority = $input->param('last_priority'); my $hold_itemnumber = $input->param('itemnumber'); - if ( $prev_priority == 0 && $next_priority == 1 ) { - C4::Reserves::RevertWaitingStatus( { itemnumber => $hold_itemnumber } ); - } else { - AlterPriority( - $where, $reserve_id, $prev_priority, - $next_priority, $first_priority, $last_priority - ); - } + if ( $prev_priority == 0 && $next_priority == 1 ) { + C4::Reserves::RevertWaitingStatus( { itemnumber => $hold_itemnumber } ); + } else { + AlterPriority( + $where, $reserve_id, $prev_priority, + $next_priority, $first_priority, $last_priority + ); + } } elsif ( $op eq 'cud-cancel' ) { my $reserve_id = $input->param('reserve_id'); my $cancellation_reason = $input->param("cancellation-reason"); my $hold = Koha::Holds->find($reserve_id); - $hold->cancel( { cancellation_reason => $cancellation_reason } ) if $hold; + if ( $can_modify_all_holds || $hold -> branchcode eq $user_branch ) { + $hold->cancel( { cancellation_reason => $cancellation_reason } ) if $hold; + } } elsif ( $op eq 'cud-setLowestPriority' ) { my $reserve_id = $input->param('reserve_id'); - ToggleLowestPriority($reserve_id); + if ( $can_modify_all_holds || Koha::Holds->find($reserve_id) -> branchcode eq $user_branch ) { + ToggleLowestPriority($reserve_id); + } } elsif ( $op eq 'cud-suspend' ) { my $reserve_id = $input->param('reserve_id'); my $suspend_until = $input->param('suspend_until'); my $hold = Koha::Holds->find($reserve_id); - $hold->suspend_hold($suspend_until) if $hold; + if (( $can_modify_all_holds || $hold -> branchcode) eq $user_branch ) { + $hold->suspend_hold($suspend_until) if $hold; + } } elsif ( $op eq 'cud-unsuspend' ) { my $reserve_id = $input->param('reserve_id'); my $hold = Koha::Holds->find($reserve_id); - $hold->resume() if $hold; + if ( $can_modify_all_holds || $hold -> branchcode eq $user_branch ) { + $hold->resume() if $hold; + } } elsif ( $op eq 'cud-cancel_bulk' ) { my $cancellation_reason = $input->param("cancellation-reason"); my @hold_ids = split( ',', scalar $input->param("ids") ); - my $params = { - reason => $cancellation_reason, - hold_ids => \@hold_ids, - }; - my $job_id = Koha::BackgroundJob::BatchCancelHold->new->enqueue($params); + for (my $i = 0; $i < @hold_ids; $i++) { + if (!( $can_modify_all_holds || (Koha::Holds->find($hold_ids[$i])->branchcode) eq $user_branch )) { + splice(@hold_ids, $i, 1); + $i--; + } + } + unless (scalar @hold_ids == 0) { + my $params = { + reason => $cancellation_reason, + hold_ids => \@hold_ids, + }; + my $job_id = Koha::BackgroundJob::BatchCancelHold->new->enqueue($params); - $template->param( - enqueued => 1, - job_id => $job_id - ); + $template->param( + enqueued => 1, + job_id => $job_id + ); + } } if ($findborrower) { -- 2.43.0