Bugzilla – Attachment 178016 Details for
Bug 38040
IndependentBranches doesn't prevent editing other libraries' holds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 38040: Adding server-side verification
Bug-38040-Adding-server-side-verification.patch (text/plain), 12.90 KB, created by
William Lavoie
on 2025-02-13 14:58:14 UTC
(
hide
)
Description:
Bug 38040: Adding server-side verification
Filename:
MIME Type:
Creator:
William Lavoie
Created:
2025-02-13 14:58:14 UTC
Size:
12.90 KB
patch
obsolete
>From 1808b8eb02faea85b655af3a437e5f1c29e8c3b6 Mon Sep 17 00:00:00 2001 >From: William Lavoie <william.lavoie@inLibro.com> >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 | 13 ++-- > reserve/request.pl | 65 ++++++++++++------- > 6 files changed, 79 insertions(+), 31 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..d5953ff081 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 %] > > <tbody> >@@ -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..db1e65f2dd 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..daa05dce68 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 @@ > </h3> > [% END %] > >- [% IF Koha.Preference('HoldsSplitQueue') == 'branch' %] >+ [% IF Koha.Preference('HoldsSplitQueue') == 'branch' || ((!CAN_user_superlibrarian) && Koha.Preference('IndependentBranchesHolds'))%] > > [% SET branchcodes = [] %] > >@@ -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 e6c4b9f368..8abb654156 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -70,7 +70,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, >@@ -101,8 +101,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{}; > >@@ -114,44 +115,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
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 38040
:
173400
|
176050
|
177197
|
177198
|
177292
|
177472
|
177492
|
178014
|
178015
|
178016
|
179186
|
179187
|
179714
|
180531
|
180532
|
180581
|
180582
|
180898
|
180899
|
180900
|
182444
|
182445
|
182446
|
182447