From 6107df83f746f67039853bd9988859b7312c3a7a Mon Sep 17 00:00:00 2001 From: Lucas Gass Date: Tue, 24 Feb 2026 22:20:20 +0000 Subject: [PATCH] Bug 41882: Enforce pickup location rules To test: 1- Set ReservesControlBranch to 'item's home library' 2- Set Main library's 'Holds and bookings policies by item type' so that PASS items can only be picked up at Main 3- Place a hold on a bib whose only item is a PASS belonging to Main 4- Use the Batch hold modification tool to identify the hold you just placed 5- Update the pickup location to North. Note that the update is successful, and there's no indication that the pickup location is now a problem. 6- Check in Main's PASS, and note that the hold doesn't trigger to fill 7- APPLY PATCH, restart_all 8- Try again, this time the hold should fail to update at all. 9- You should see a new table with the 'Failed hold' in it, it should also include a 'Result' column which explains why the hold failed (New pickup location is invalid). 10- Try modifying a hold to an invalid pickup location, but also attempt to modify something else on the hold at the same time, like expiration date. 11- The hold should not modify anything. If the pickup location is invalid all other modifications should NOT happen on that hold. 12 - Play with the Table settings for the holds_modified tables, ensuring you can hide/show all the columns. Signed-off-by: David Nind --- admin/columns_settings.yml | 2 + .../en/modules/tools/batch_modify_holds.tt | 192 ++++++++++++------ tools/batch_modify_holds.pl | 71 +++++-- 3 files changed, 186 insertions(+), 79 deletions(-) diff --git a/admin/columns_settings.yml b/admin/columns_settings.yml index c249e0d0fd..100ec269d2 100644 --- a/admin/columns_settings.yml +++ b/admin/columns_settings.yml @@ -2671,6 +2671,8 @@ modules: columnname: suspended_until - columnname: notes + - + columnname: result logviewer: logst: default_display_length: 20 diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_modify_holds.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_modify_holds.tt index de32ade223..da05227479 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_modify_holds.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_modify_holds.tt @@ -103,63 +103,133 @@ [% ELSIF view == "report" %] -

Modified holds

-
-
- - - - - - - - - - - - - - - - - [% FOREACH hold IN updated_holds_obj %] +

Modified holds report

+ [% IF updated_holds_obj %] +
+
+

Updated holds ([% total_updated | html %])

+
TitlePatronPickup locationPriorityExpiration dateStatusSuspendedSuspended untilNotes
+ - - - - - - + + + + + + + + + + + + + + [% FOREACH hold IN updated_holds_obj %] + + + + + + + - - - + + + + + + + [% END %] + +
[% INCLUDE 'biblio-title.inc' biblio=hold.biblio link = 1 %][% INCLUDE 'patron-title.inc' invert_name=1 patron=hold.borrower hide_patron_infos_if_needed=1 link_to="circulation_reserves" %][% hold.branch.branchname | html %][% hold.priority | html %][% hold.expirationdate | $KohaDates %] - [% IF hold.found == 'F' %] - Fulfilled - [% ELSIF hold.cancellationdate %] - Cancelled - [% ELSIF hold.found == 'W' %] - [% IF hold.cancellation_requests.count == 0 %] - Waiting - [% ELSE %] + TitlePatronPickup locationPriorityExpiration dateStatusSuspendedSuspended untilNotesResult
[% INCLUDE 'biblio-title.inc' biblio=hold.biblio link = 1 %][% INCLUDE 'patron-title.inc' invert_name=1 patron=hold.borrower hide_patron_infos_if_needed=1 link_to="circulation_reserves" %][% hold.branch.branchname | html %][% hold.priority | html %][% hold.expirationdate | $KohaDates %] + [% IF hold.found == 'F' %] + Fulfilled + [% ELSIF hold.cancellationdate %] Cancelled + [% ELSIF hold.found == 'W' %] + [% IF hold.cancellation_requests.count == 0 %] + Waiting + [% ELSE %] + Cancelled + [% END %] + [% ELSIF hold.found == 'T' %] + In transit + [% ELSE %] + Pending [% END %] - [% ELSIF hold.found == 'T' %] - In transit - [% ELSE %] - Pending - [% END %] - [% IF hold.suspend %]Yes[% ELSE %]No[% END %][% IF hold.suspend_until %][% hold.suspend_until | $KohaDates %][% ELSE %]-[% END %][% hold.reservenotes | html %][% IF hold.suspend %]Yes[% ELSE %]No[% END %][% IF hold.suspend_until %][% hold.suspend_until | $KohaDates %][% ELSE %]-[% END %][% hold.reservenotes | html %]Success
+
+ +
+ + [% END %] + [% IF failed_holds_obj %] +
+
+

Failed holds ([% total_failed | html %])

+
Important: The following holds were not modified.
+ + + + + + + + + + + + + + - [% END %] - -
TitlePatronPickup locationPriorityExpiration dateStatusSuspendedSuspended untilNotesResult
+ + + [% FOREACH failure IN failed_holds_obj %] + [% SET hold = failure.hold %] + + [% INCLUDE 'biblio-title.inc' biblio=hold.biblio link = 1 %] + [% INCLUDE 'patron-title.inc' invert_name=1 patron=hold.borrower hide_patron_infos_if_needed=1 link_to="circulation_reserves" %] + [% hold.branch.branchname | html %] + [% hold.priority | html %] + [% hold.expirationdate | $KohaDates %] + + [% IF hold.found == 'F' %] + Fulfilled + [% ELSIF hold.cancellationdate %] + Cancelled + [% ELSIF hold.found == 'W' %] + [% IF hold.cancellation_requests.count == 0 %] + Waiting + [% ELSE %] + Cancelled + [% END %] + [% ELSIF hold.found == 'T' %] + In transit + [% ELSE %] + Pending + [% END %] + + [% IF hold.suspend %]Yes[% ELSE %]No[% END %] + [% IF hold.suspend_until %][% hold.suspend_until | $KohaDates %][% ELSE %]-[% END %] + [% hold.reservenotes | html %] + + Failed + [% IF failure.reason == 'pickupLocationNotAllowed' %] + New pickup location is invalid + [% END %] + + + [% END %] + + +
+
- -
- Return to batch hold modification -
- - + + [% END %] +
+ Return to batch hold modification +
[% END %]