Bug 36135 - Add tool to batch modify holds
Summary: Add tool to batch modify holds
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low new feature
Assignee: Lucas Gass (lukeg)
QA Contact: Pedro Amorim
URL:
Keywords:
Depends on: 40332
Blocks:
  Show dependency treegraph
 
Reported: 2024-02-20 06:37 UTC by Emmi Takkinen
Modified: 2025-08-05 14:49 UTC (History)
21 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 36135: Add new permission batch_modify_holds (3.10 KB, patch)
2024-03-11 11:51 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add links to the batch modify holds tool (3.80 KB, patch)
2024-03-11 11:52 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add new tool to batch modify holds (24.89 KB, patch)
2024-03-11 11:52 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Allow using SQL results in batch modify holds tool (6.37 KB, patch)
2024-03-11 11:53 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: (follow-up) Adjust for CSRF (3.01 KB, patch)
2024-03-20 14:14 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36135: (follow-up) Adjust for CSRF (4.01 KB, patch)
2024-03-20 14:16 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36135: Add new permission batch_modify_holds (3.16 KB, patch)
2024-03-20 14:32 UTC, Laura Escamilla
Details | Diff | Splinter Review
Bug 36135: Add links to the batch modify holds tool (3.86 KB, patch)
2024-03-20 14:33 UTC, Laura Escamilla
Details | Diff | Splinter Review
Bug 36135: Add new tool to batch modify holds (24.95 KB, patch)
2024-03-20 14:33 UTC, Laura Escamilla
Details | Diff | Splinter Review
Bug 36135: Allow using SQL results in batch modify holds tool (6.44 KB, patch)
2024-03-20 14:33 UTC, Laura Escamilla
Details | Diff | Splinter Review
Bug 36135: (follow-up) Adjust for CSRF (4.07 KB, patch)
2024-03-20 14:33 UTC, Laura Escamilla
Details | Diff | Splinter Review
Bug 36135: Add several new features (8.01 KB, patch)
2024-03-25 11:16 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Fix QA issues and add improvemnents (22.13 KB, patch)
2024-03-28 09:30 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Fix QA issues and add improvements (22.43 KB, patch)
2024-03-28 12:28 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add new permission batch_modify_holds (3.21 KB, patch)
2024-03-28 14:12 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Add links to the batch modify holds tool (3.92 KB, patch)
2024-03-28 14:12 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Add new tool to batch modify holds (25.00 KB, patch)
2024-03-28 14:12 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Allow using SQL results in batch modify holds tool (6.49 KB, patch)
2024-03-28 14:13 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: (follow-up) Adjust for CSRF (4.13 KB, patch)
2024-03-28 14:13 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Add several new features (8.06 KB, patch)
2024-03-28 14:13 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix QA issues and add improvements (22.48 KB, patch)
2024-03-28 14:13 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Report errors to a modal (4.60 KB, patch)
2025-02-04 20:40 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix breadcrumbs/Bootstrap issues (2.96 KB, patch)
2025-02-04 20:58 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix some markup errors (1.84 KB, patch)
2025-02-12 18:24 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Add new permission batch_modify_holds (60.08 KB, patch)
2025-02-12 18:28 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Add links to the batch modify holds tool (3.79 KB, patch)
2025-02-12 18:28 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Add new tool to batch modify holds (25.09 KB, patch)
2025-02-12 18:28 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Allow using SQL results in batch modify holds tool (6.89 KB, patch)
2025-02-12 18:29 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: (follow-up) Adjust for CSRF (3.98 KB, patch)
2025-02-12 18:29 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Add several new features (8.04 KB, patch)
2025-02-12 18:29 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix QA issues and add improvements (16.39 KB, patch)
2025-02-12 18:29 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Report errors to a modal (4.60 KB, patch)
2025-02-12 18:29 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix breadcrumbs/Bootstrap issues (2.96 KB, patch)
2025-02-12 18:29 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix some markup errors (1.84 KB, patch)
2025-02-12 18:29 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: API the found_holds table (29.37 KB, patch)
2025-02-12 21:16 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Embed items (677 bytes, patch)
2025-02-12 21:18 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Add ability to fetch patron info via API holds endpoint (1.17 KB, patch)
2025-02-13 07:34 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add more API functionality to display holds to modify (53.00 KB, patch)
2025-02-13 07:34 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add new permission batch_modify_holds (3.28 KB, patch)
2025-02-18 06:31 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add links to the batch modify holds tool (3.79 KB, patch)
2025-02-18 06:31 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add new tool to batch modify holds (25.10 KB, patch)
2025-02-18 06:32 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Allow using SQL results in batch modify holds tool (6.90 KB, patch)
2025-02-18 06:32 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: (follow-up) Adjust for CSRF (3.98 KB, patch)
2025-02-18 06:33 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add several new features (8.05 KB, patch)
2025-02-18 06:33 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Fix QA issues and add improvements (16.39 KB, patch)
2025-02-18 06:34 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Fix breadcrumbs/Bootstrap issues (4.60 KB, patch)
2025-02-18 06:34 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Fix some markup errors (1.85 KB, patch)
2025-02-18 06:35 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Embed items (683 bytes, patch)
2025-02-18 06:35 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add ability to fetch patron info via API holds endpoint (1.17 KB, patch)
2025-02-18 06:35 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Use API and datatables to display holds to modify (47.79 KB, patch)
2025-02-18 06:36 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Save state of selected checkboxes and fix modifying suspend (19.40 KB, patch)
2025-02-18 06:36 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Save state of selected checkboxes and fix modifying suspend (19.97 KB, patch)
2025-02-18 08:44 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Batch clear hold notes and use select2 in multiple filters (3.93 KB, patch)
2025-03-05 12:16 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add new permission batch_modify_holds (3.28 KB, patch)
2025-03-07 07:46 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add links to the batch modify holds tool (3.75 KB, patch)
2025-03-07 07:47 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add new tool to batch modify holds (25.10 KB, patch)
2025-03-07 07:47 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Allow using SQL results in batch modify holds tool (6.90 KB, patch)
2025-03-07 07:48 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: (follow-up) Adjust for CSRF (3.98 KB, patch)
2025-03-07 07:48 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add several new features (8.05 KB, patch)
2025-03-07 07:49 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Fix QA issues and add improvements (16.39 KB, patch)
2025-03-07 07:49 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Report errors to a modal (4.60 KB, patch)
2025-03-07 07:50 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Fix breadcrumbs/Bootstrap issues (2.97 KB, patch)
2025-03-07 07:50 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Fix some markup errors (1.85 KB, patch)
2025-03-07 07:51 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Embed items (683 bytes, patch)
2025-03-07 07:51 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add ability to fetch patron info via API holds endpoint (1.17 KB, patch)
2025-03-07 07:52 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Use API and datatables to display holds to modify (47.79 KB, patch)
2025-03-07 07:52 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Save state of selected checkboxes and fix modifying suspend (19.97 KB, patch)
2025-03-07 07:52 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Batch clear hold notes and use select2 in multiple filters (3.94 KB, patch)
2025-03-07 07:53 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: (QA follow-up): fix typo in tool permissions (1.16 KB, patch)
2025-03-07 14:33 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 36135: (follow-up) Fix link style on guided report page (1.46 KB, patch)
2025-03-07 14:50 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix ability to send SQL results in batch modify holds tool (5.96 KB, patch)
2025-04-17 06:33 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add several fixes (10.90 KB, patch)
2025-06-11 10:47 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 36135: Add batch_modify_holds (29.48 KB, patch)
2025-06-16 22:09 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Allow using SQL results in batch modify holds tool (27.86 KB, patch)
2025-06-16 22:09 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Embed items (669 bytes, patch)
2025-06-16 22:09 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Add ability to fetch patron info via API holds endpoint (52.83 KB, patch)
2025-06-16 22:09 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: (follow-up) Fix link style on guided report page (52.52 KB, patch)
2025-06-16 22:09 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix syntax error in permissions.inc (1.45 KB, patch)
2025-06-16 22:09 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix forbidden pattern (1.00 KB, patch)
2025-06-16 22:10 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Add batch_modify_holds permissions (3.10 KB, patch)
2025-06-17 14:38 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Add ability to batch modify holds (56.22 KB, patch)
2025-06-17 14:38 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix syntax error in permissions.inc (1.47 KB, patch)
2025-06-17 14:38 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix forbidden pattern (1.03 KB, patch)
2025-06-17 14:38 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Make modified_holds table not use the API (11.73 KB, patch)
2025-07-03 17:04 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Correct tools-menu category logic (1.91 KB, patch)
2025-07-09 13:36 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 36135: (follow-up) Fix suspension search and other improvements (13.17 KB, patch)
2025-07-10 11:24 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 36135: Add batch_modify_holds permissions (3.14 KB, patch)
2025-07-10 17:36 UTC, Jason Robb
Details | Diff | Splinter Review
Bug 36135: Add ability to batch modify holds (56.26 KB, patch)
2025-07-10 17:36 UTC, Jason Robb
Details | Diff | Splinter Review
Bug 36135: Fix syntax error in permissions.inc (1.52 KB, patch)
2025-07-10 17:36 UTC, Jason Robb
Details | Diff | Splinter Review
Bug 36135: Fix forbidden pattern (1.08 KB, patch)
2025-07-10 17:36 UTC, Jason Robb
Details | Diff | Splinter Review
Bug 36135: Make modified_holds table not use the API (11.78 KB, patch)
2025-07-10 17:36 UTC, Jason Robb
Details | Diff | Splinter Review
Bug 36135: Correct tools-menu category logic (1.95 KB, patch)
2025-07-10 17:36 UTC, Jason Robb
Details | Diff | Splinter Review
Bug 36135: (follow-up) Fix suspension search and other improvements (13.22 KB, patch)
2025-07-10 17:36 UTC, Jason Robb
Details | Diff | Splinter Review
Bug 36135: Add batch_modify_holds permissions (3.14 KB, patch)
2025-08-01 16:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Add ability to batch modify holds (56.27 KB, patch)
2025-08-01 16:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix syntax error in permissions.inc (1.52 KB, patch)
2025-08-01 16:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Fix forbidden pattern (1.08 KB, patch)
2025-08-01 16:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Make modified_holds table not use the API (11.78 KB, patch)
2025-08-01 16:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: Correct tools-menu category logic (1.95 KB, patch)
2025-08-01 16:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36135: (follow-up) Fix suspension search and other improvements (13.22 KB, patch)
2025-08-01 16:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Funky filters (66.94 KB, image/png)
2025-08-01 16:42 UTC, Pedro Amorim
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Emmi Takkinen 2024-02-20 06:37:52 UTC
There are sometimes situations where libraries have to extend their holds pick up dates. We should at same kind of tool for this like "Batch extend due dates". However criteria for dates to extend can be complicated (we've excluded e.g. items with specific state, holds with found status, items on transfer to specific library). So rather than using predefined criteria for holds to edit (at least for the start), tool should have option to upload file of SQL query results.
Comment 1 Emmi Takkinen 2024-02-20 06:55:16 UTC
Corrected
Comment 2 Emmi Takkinen 2024-02-20 06:58:11 UTC
(In reply to Emmi Takkinen from comment #1)
> Corrected

... the title, we should extend expiration dates, not pick up dates.
Comment 3 Emmi Takkinen 2024-03-11 11:51:30 UTC Comment hidden (obsolete)
Comment 4 Emmi Takkinen 2024-03-11 11:52:14 UTC Comment hidden (obsolete)
Comment 5 Emmi Takkinen 2024-03-11 11:52:41 UTC Comment hidden (obsolete)
Comment 6 Emmi Takkinen 2024-03-11 11:53:08 UTC Comment hidden (obsolete)
Comment 7 Emmi Takkinen 2024-03-11 11:56:02 UTC
We did some briefing on this and ended up adding ability to modify other fields as well, not just expiration date. I'll adjust bugs title accordingly.
Comment 8 Laura Escamilla 2024-03-20 13:45:37 UTC
Hi Emmi, 

I am running into the following error when attempting to use the Batch Modify Holds tool: 

Programming error - op 'list' must start with 'cud-' for POST http://localhost:8081/intranet/tools/batch_modify_holds.pl (referer: http://localhost:8081/cgi-bin/koha/tools/batch_modify_holds.pl)
Comment 9 Nick Clemens (kidclamp) 2024-03-20 14:14:53 UTC Comment hidden (obsolete)
Comment 10 Nick Clemens (kidclamp) 2024-03-20 14:16:37 UTC Comment hidden (obsolete)
Comment 11 Laura Escamilla 2024-03-20 14:32:57 UTC Comment hidden (obsolete)
Comment 12 Laura Escamilla 2024-03-20 14:33:06 UTC Comment hidden (obsolete)
Comment 13 Laura Escamilla 2024-03-20 14:33:09 UTC Comment hidden (obsolete)
Comment 14 Laura Escamilla 2024-03-20 14:33:11 UTC Comment hidden (obsolete)
Comment 15 Laura Escamilla 2024-03-20 14:33:13 UTC Comment hidden (obsolete)
Comment 16 Laura Escamilla 2024-03-20 14:39:05 UTC
Hi! 

Thank you again for working on this patch! It works as described. :)

These could be follow-up bugs, but I think it would be helpful to have a column in the batch modify holds tool that either has the item barcode, or item number / bib record hyperlinked for easy access. 

It would also be helpful to display which items have a holds status that can not be suspended. It is difficult to determine at a glance which item cannot be suspended and it might be frustrating to libraries who are trying to suspend hundreds of holds at a time.
Comment 17 Emmi Takkinen 2024-03-21 06:28:28 UTC
(In reply to Laura Escamilla from comment #16)
> Hi! 
> 
> Thank you again for working on this patch! It works as described. :)
> 
> These could be follow-up bugs, but I think it would be helpful to have a
> column in the batch modify holds tool that either has the item barcode, or
> item number / bib record hyperlinked for easy access. 
> 
> It would also be helpful to display which items have a holds status that can
> not be suspended. It is difficult to determine at a glance which item cannot
> be suspended and it might be frustrating to libraries who are trying to
> suspend hundreds of holds at a time.

Hi!

Thank you for feedback :) I'll take a look what I can do to these and other feedback I received from our superlibrarians. I'll probably provide separate feedback patch.
Comment 18 Emmi Takkinen 2024-03-25 11:16:08 UTC Comment hidden (obsolete)
Comment 19 Lucas Gass (lukeg) 2024-03-25 22:16:06 UTC
This looks good but there are some small fixes to be made: 

1.  WARN	installer/data/mysql/atomicupdate/bug_36135.pl
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 0, now: 4)

Perltidy your atomicupdate

2. 	forbidden pattern: simple-quote string (line 312)
 
Line 312: tools/batch_modify_holds.tt, needs double quotes, not single. 

3.    FAIL	  tt_valid
		lines 134

This probably needs to be something like: 

[% IF hold.found %]
    <tr class="highlighted-row">
[% ELSE %]
    <tr>
[% END %]

4. Line 123: <th>Title</title>
5. Line 111: Missing a closing </div>


6. Overall there needs to be some improvement to the HTML:

<label for="suspend_until">Expiration date:</label>
<label for="suspend_until">Suspended until:</label> 

These should probably be <legend> not <label>. 

<li> elements inside of <li>? Not a blocker but should these be separate fieldsets? 


7. These both use the ID "to" and they should use #expirationdate_to and #suspend_until_to, respectively.


 56                                         <li>
 57                                             <label for="expirationdate_to">To</label>
 58                                             <input type="text" size="10" id="to" name="expirationdate_to" class="flatpickr"/>
 59                                         </li>


 91                                             <label for="suspend_until_to">To</label>
 92                                             <input type="text" size="10" id="to" name="suspend_until_to" class="flatpickr"/>
Comment 20 Emmi Takkinen 2024-03-28 09:30:17 UTC Comment hidden (obsolete)
Comment 21 Emmi Takkinen 2024-03-28 09:35:24 UTC
Fixed QA issues which Lucas pointed out. For this

><label for="suspend_until">Expiration date:</label>
><label for="suspend_until">Suspended until:</label> 
>These should probably be <legend> not <label>.

I removed them and added labels "Expiration date from:" and "Expiration date to:" labels for corresponding li elements. Same for suspend until. Legend is used for as caption of fieldset so it wasn't suitable to be used in this case.
Comment 22 Emmi Takkinen 2024-03-28 12:28:11 UTC Comment hidden (obsolete)
Comment 23 Lucas Gass (lukeg) 2024-03-28 14:12:00 UTC
Thanks Emmi! This looks great.
Comment 24 Lucas Gass (lukeg) 2024-03-28 14:12:53 UTC Comment hidden (obsolete)
Comment 25 Lucas Gass (lukeg) 2024-03-28 14:12:55 UTC Comment hidden (obsolete)
Comment 26 Lucas Gass (lukeg) 2024-03-28 14:12:58 UTC Comment hidden (obsolete)
Comment 27 Lucas Gass (lukeg) 2024-03-28 14:13:00 UTC Comment hidden (obsolete)
Comment 28 Lucas Gass (lukeg) 2024-03-28 14:13:02 UTC Comment hidden (obsolete)
Comment 29 Lucas Gass (lukeg) 2024-03-28 14:13:05 UTC Comment hidden (obsolete)
Comment 30 Lucas Gass (lukeg) 2024-03-28 14:13:07 UTC Comment hidden (obsolete)
Comment 31 Nick Clemens (kidclamp) 2024-05-09 13:13:13 UTC
I like this a lot - self contained and makes minimal changes, the patches are clean and easy to read, but I think we need to consider some things here before this moves ahead

Must fix:
The JS alerts to prevent submission fire an alert per warning, so you need to click through several if you make a mistake - these should be combined into a single alert

There is no way to see the limits you have placed on your search - I think these need to be shown to the user, ideally they would be editable, but showing them would be a start

Should fix:
While I like the idea of minimalizing data for privacy, excluding the borrower info on the holds feels a bit too far. Ideally this would be under columns configuration, and we have patron-title.inc for showing borrower info only to staff who should see it. The current column selection feels 

The display should be paginated, ideally it would use the holds API to load the holds to ensure performance, as scrolling can get really long really fast. It would be nice too to be able to select all in a filtered search - right now 'Select all' selects even those not visible in the table

Could fix:
If implemented with the datatables API this could then work like the patron search, with filters present on the page. 

The batch edit implies more batch actions - cancellation, reverting, etc. but these can all be follow-ups I think
Comment 32 Emmi Takkinen 2024-05-13 05:27:30 UTC
(In reply to Nick Clemens (kidclamp) from comment #31)
> I like this a lot - self contained and makes minimal changes, the patches
> are clean and easy to read, but I think we need to consider some things here
> before this moves ahead
> 
> Must fix:
> The JS alerts to prevent submission fire an alert per warning, so you need
> to click through several if you make a mistake - these should be combined
> into a single alert
> 
> There is no way to see the limits you have placed on your search - I think
> these need to be shown to the user, ideally they would be editable, but
> showing them would be a start
> 
> Should fix:
> While I like the idea of minimalizing data for privacy, excluding the
> borrower info on the holds feels a bit too far. Ideally this would be under
> columns configuration, and we have patron-title.inc for showing borrower
> info only to staff who should see it. The current column selection feels 
> 
> The display should be paginated, ideally it would use the holds API to load
> the holds to ensure performance, as scrolling can get really long really
> fast. It would be nice too to be able to select all in a filtered search -
> right now 'Select all' selects even those not visible in the table
> 
> Could fix:
> If implemented with the datatables API this could then work like the patron
> search, with filters present on the page. 
> 
> The batch edit implies more batch actions - cancellation, reverting, etc.
> but these can all be follow-ups I think

Thank you Nick for feedback. First two points are indeed something that should be fixed. I'm just bit unsure how to display those search terms in search result page. I also agree that using API does have benefits and table should be paginated (even my test data might produce result of over 2000 holds and that is cumbersome for users go through). I'll take a look what I can do to these. Setting this as assigned for now.
Comment 33 Emmi Takkinen 2024-05-13 11:56:47 UTC
Moving on with this really depends on whether API is used or not in this page. Otherwise adding it later would probably mean making same work again with some of the features Nick suggested.

As for the filters displayed on page, I think it would be better if they are displayed like in item search page behind "Edit search" button. Putting them on margin just doesn't seem (visually) good idea since there are other links there already.
Comment 34 Emmi Takkinen 2024-05-17 09:29:36 UTC
Couple problems, holds endpoint doesn't show patron_expiration_date column (see bug 29975). It also doesn't allow fetching items barcode, adding items to datatables embed property leads to error. So we need to make changes to holds endpoint to make this function properly.
Comment 35 Laura O'Neil 2025-01-13 14:30:13 UTC
I would love to see this one pick up again. It's a popular request from our partner libraries.
Comment 36 Lucas Gass (lukeg) 2025-02-04 16:08:50 UTC
Looks like the templates now need to be adjusted for Bootstrap 5 changes.
Comment 37 Lucas Gass (lukeg) 2025-02-04 20:40:30 UTC Comment hidden (obsolete)
Comment 38 Lucas Gass (lukeg) 2025-02-04 20:58:58 UTC Comment hidden (obsolete)
Comment 39 Lucas Gass (lukeg) 2025-02-12 18:24:49 UTC Comment hidden (obsolete)
Comment 40 Lucas Gass (lukeg) 2025-02-12 18:28:50 UTC Comment hidden (obsolete)
Comment 41 Lucas Gass (lukeg) 2025-02-12 18:28:54 UTC Comment hidden (obsolete)
Comment 42 Lucas Gass (lukeg) 2025-02-12 18:28:57 UTC Comment hidden (obsolete)
Comment 43 Lucas Gass (lukeg) 2025-02-12 18:29:01 UTC Comment hidden (obsolete)
Comment 44 Lucas Gass (lukeg) 2025-02-12 18:29:04 UTC Comment hidden (obsolete)
Comment 45 Lucas Gass (lukeg) 2025-02-12 18:29:07 UTC Comment hidden (obsolete)
Comment 46 Lucas Gass (lukeg) 2025-02-12 18:29:11 UTC Comment hidden (obsolete)
Comment 47 Lucas Gass (lukeg) 2025-02-12 18:29:14 UTC Comment hidden (obsolete)
Comment 48 Lucas Gass (lukeg) 2025-02-12 18:29:17 UTC Comment hidden (obsolete)
Comment 49 Lucas Gass (lukeg) 2025-02-12 18:29:20 UTC Comment hidden (obsolete)
Comment 50 Lucas Gass (lukeg) 2025-02-12 18:30:24 UTC Comment hidden (obsolete)
Comment 51 Lucas Gass (lukeg) 2025-02-12 21:16:05 UTC Comment hidden (obsolete)
Comment 52 Lucas Gass (lukeg) 2025-02-12 21:18:29 UTC Comment hidden (obsolete)
Comment 53 Lucas Gass (lukeg) 2025-02-12 21:26:21 UTC
This is still a work in progress but want to mention a few things:

1. Handling state across paginated AJAX table is hard, for me at least. So, I don't think we actually need checkboxes. Instead, you do the search and can bulk update everything that is returned. There should be an option to refine the search.

2. Instead of handling errors with JS before you submit I think can simply report back on holds that were not updated. Example, trying to suspend a waiting hold. 

3. The report table in the final step should report back successes and errors, so I think it cannot be an API/AJAX table since you can't get that data from the API. But maybe a standard DataTable?
Comment 54 Emmi Takkinen 2025-02-13 07:34:22 UTC Comment hidden (obsolete)
Comment 55 Emmi Takkinen 2025-02-13 07:34:49 UTC Comment hidden (obsolete)
Comment 56 Emmi Takkinen 2025-02-13 08:05:56 UTC
Seems that Lucas and I both have been working with this one :D Sorry for not updating status on this, we've been testing this on our testing environment and I've been holding up to provide patches here until there are no more major bugs in this.

I added my patches top of Lucas' patches, they use API to fetch holds and display them in datatable. However modifying them is still done in back-end where they are then send back to front-end as JSON and to another datatable. There is also a feature to modify search before submiting the changes. There is still a problem with handling the suspend status which I'm currently working with (changing it doesn't work in other languages than english). 

> 1. Handling state across paginated AJAX table is hard, for me at least. So,
> I don't think we actually need checkboxes. Instead, you do the search and
> can bulk update everything that is returned. There should be an option to
> refine the search.
>
I do agree, handling checkboxes is a mess with pagination. But I'm not yet ready to give up on that so I'll see what I can do.

> 2. Instead of handling errors with JS before you submit I think can simply
> report back on holds that were not updated. Example, trying to suspend a
> waiting hold. 
> 
I'm not sure if this is a good idea. It feels more user-friendly to provide feedback for users before anything is changed.

> 3. The report table in the final step should report back successes and
> errors, so I think it cannot be an API/AJAX table since you can't get that
> data from the API. But maybe a standard DataTable?

Again if we handle errors before making modifications there is no need for this.

I think we need more feedback from users on how to handle these.
Comment 57 Emmi Takkinen 2025-02-13 08:39:20 UTC
There seems to be a way to save the state of checkboxes with pagination, at least I was able to do this via patron search. So we can probably use same method with this also.
Comment 58 Lucas Gass (lukeg) 2025-02-13 14:34:27 UTC
(In reply to Emmi Takkinen from comment #57)
> There seems to be a way to save the state of checkboxes with pagination, at
> least I was able to do this via patron search. So we can probably use same
> method with this also.

Emmi, my apologies here. Sorry for the confusion, I meant to help here but looks like I only got in the way! 

Maybe using something like this to manage state?

https://datatables.net/extensions/select/index



Lucas
Comment 59 Emmi Takkinen 2025-02-17 12:39:38 UTC
(In reply to Lucas Gass (lukeg) from comment #58)
> (In reply to Emmi Takkinen from comment #57)
> > There seems to be a way to save the state of checkboxes with pagination, at
> > least I was able to do this via patron search. So we can probably use same
> > method with this also.
> 
> Emmi, my apologies here. Sorry for the confusion, I meant to help here but
> looks like I only got in the way! 
> 
> Maybe using something like this to manage state?
> 
> https://datatables.net/extensions/select/index
> 
> 
> 
> Lucas

Hmm, for some reason this isn't working. Which is a bummer, this seems like easy way achieve saving the state. Maybe I'm missing something here even thought at least in paper this should be pretty simple to take in use :D
Comment 60 Emmi Takkinen 2025-02-18 06:31:25 UTC Comment hidden (obsolete)
Comment 61 Emmi Takkinen 2025-02-18 06:31:57 UTC Comment hidden (obsolete)
Comment 62 Emmi Takkinen 2025-02-18 06:32:29 UTC Comment hidden (obsolete)
Comment 63 Emmi Takkinen 2025-02-18 06:32:57 UTC Comment hidden (obsolete)
Comment 64 Emmi Takkinen 2025-02-18 06:33:21 UTC Comment hidden (obsolete)
Comment 65 Emmi Takkinen 2025-02-18 06:33:56 UTC Comment hidden (obsolete)
Comment 66 Emmi Takkinen 2025-02-18 06:34:25 UTC Comment hidden (obsolete)
Comment 67 Emmi Takkinen 2025-02-18 06:34:50 UTC Comment hidden (obsolete)
Comment 68 Emmi Takkinen 2025-02-18 06:35:11 UTC Comment hidden (obsolete)
Comment 69 Emmi Takkinen 2025-02-18 06:35:31 UTC Comment hidden (obsolete)
Comment 70 Emmi Takkinen 2025-02-18 06:35:50 UTC Comment hidden (obsolete)
Comment 71 Emmi Takkinen 2025-02-18 06:36:14 UTC Comment hidden (obsolete)
Comment 72 Emmi Takkinen 2025-02-18 06:36:32 UTC Comment hidden (obsolete)
Comment 73 Emmi Takkinen 2025-02-18 06:37:17 UTC
All patches are now rebased and ready for testing.
Comment 74 Emmi Takkinen 2025-02-18 08:20:56 UTC
There is still some issues with modifying suspend status.
Comment 75 Emmi Takkinen 2025-02-18 08:44:37 UTC
Created attachment 178167 [details] [review]
Bug 36135: Save state of selected checkboxes and fix modifying suspend
Comment 76 Avery Campbell (Butte County Library (CA)) 2025-02-19 18:02:29 UTC
Batch editing holds would be great to have. I don't know if batch changing priorities is planned as part of this, but we have a large number of tech devices (hotpots) which are being deactivated. We have placed them on hold so we can get them before another patron checks out an unusable device. I'm working on moving them to the top of the records' hold queue. Batch changing the priorities would be much better than manually clicking the double-up for all ~180 items. This would be a huge time saver if another library were to need to do something this in the future.
Comment 77 Emmi Takkinen 2025-03-05 12:16:49 UTC Comment hidden (obsolete)
Comment 78 Lucas Gass (lukeg) 2025-03-05 14:33:42 UTC
Sorry Emmi, patches don't apply to current main.
Comment 79 Emmi Takkinen 2025-03-06 07:14:44 UTC
(In reply to Lucas Gass (lukeg) from comment #78)
> Sorry Emmi, patches don't apply to current main.

That's odd, my main is up to date and patches apply cleanly to it.
Comment 80 Andrew Fuerste-Henry 2025-03-06 13:39:03 UTC
I've just tried to apply these and was also unable. This is on my ktd system, on main, immediately after a git pull.

Specifically:
Applying: Bug 36135: Add links to the batch modify holds tool
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc
error: Failed to merge in the changes.
Patch failed at 0001 Bug 36135: Add links to the batch modify holds tool
Comment 81 Emmi Takkinen 2025-03-07 06:33:18 UTC
(In reply to Andrew Fuerste-Henry from comment #80)
> I've just tried to apply these and was also unable. This is on my ktd
> system, on main, immediately after a git pull.
> 
> Specifically:
> Applying: Bug 36135: Add links to the batch modify holds tool
> Using index info to reconstruct a base tree...
> M	koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc
> Falling back to patching base and 3-way merge...
> Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc
> CONFLICT (content): Merge conflict in
> koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc
> error: Failed to merge in the changes.
> Patch failed at 0001 Bug 36135: Add links to the batch modify holds tool

Ah, I see. I'm using git-installation for testing, so I guess it and ktd differ from each other somehow.
Comment 82 Emmi Takkinen 2025-03-07 06:47:29 UTC
(In reply to Emmi Takkinen from comment #81)
> (In reply to Andrew Fuerste-Henry from comment #80)
> > I've just tried to apply these and was also unable. This is on my ktd
> > system, on main, immediately after a git pull.
> > 
> > Specifically:
> > Applying: Bug 36135: Add links to the batch modify holds tool
> > Using index info to reconstruct a base tree...
> > M	koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc
> > Falling back to patching base and 3-way merge...
> > Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc
> > CONFLICT (content): Merge conflict in
> > koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc
> > error: Failed to merge in the changes.
> > Patch failed at 0001 Bug 36135: Add links to the batch modify holds tool
> 
> Ah, I see. I'm using git-installation for testing, so I guess it and ktd
> differ from each other somehow.

And thinking this a little more, it's not possible that they differ. I'll try rebase again.
Comment 83 Emmi Takkinen 2025-03-07 07:46:38 UTC Comment hidden (obsolete)
Comment 84 Emmi Takkinen 2025-03-07 07:47:13 UTC Comment hidden (obsolete)
Comment 85 Emmi Takkinen 2025-03-07 07:47:49 UTC Comment hidden (obsolete)
Comment 86 Emmi Takkinen 2025-03-07 07:48:20 UTC Comment hidden (obsolete)
Comment 87 Emmi Takkinen 2025-03-07 07:48:56 UTC Comment hidden (obsolete)
Comment 88 Emmi Takkinen 2025-03-07 07:49:25 UTC Comment hidden (obsolete)
Comment 89 Emmi Takkinen 2025-03-07 07:49:55 UTC Comment hidden (obsolete)
Comment 90 Emmi Takkinen 2025-03-07 07:50:25 UTC Comment hidden (obsolete)
Comment 91 Emmi Takkinen 2025-03-07 07:50:46 UTC Comment hidden (obsolete)
Comment 92 Emmi Takkinen 2025-03-07 07:51:07 UTC Comment hidden (obsolete)
Comment 93 Emmi Takkinen 2025-03-07 07:51:33 UTC Comment hidden (obsolete)
Comment 94 Emmi Takkinen 2025-03-07 07:52:01 UTC Comment hidden (obsolete)
Comment 95 Emmi Takkinen 2025-03-07 07:52:28 UTC Comment hidden (obsolete)
Comment 96 Emmi Takkinen 2025-03-07 07:52:53 UTC Comment hidden (obsolete)
Comment 97 Emmi Takkinen 2025-03-07 07:53:15 UTC Comment hidden (obsolete)
Comment 98 Emmi Takkinen 2025-03-07 08:01:06 UTC
There was something wrong with patches I had in my development branch, they applied cleanly against current main. But after creating a new clean branch and applying patches from Bugzilla I got same merge conflict as Andrew. I had also dropped one of the patches at some point. These should now apply, but I was wondering do they still need some clean up to match changes made for tidying up the whole code base (bug 38664).
Comment 99 Andrew Fuerste-Henry 2025-03-07 14:33:23 UTC Comment hidden (obsolete)
Comment 100 Lucas Gass (lukeg) 2025-03-07 14:50:06 UTC Comment hidden (obsolete)
Comment 101 Emmanuel Bétemps 2025-04-02 08:41:31 UTC
Hi,

I created 3 holds pour the same patron
Steps 1 to 4 are ok but when I click on the "Batch modify holds" option, the Tolls page doesn't list any hold
I just see the staff interface, the title of this tool (Batch modify holds) but no hold is displayed
Comment 102 Emmanuel Bétemps 2025-04-02 08:43:48 UTC
oups
not 'pour' but 'for' the same patron
Comment 103 Emmi Takkinen 2025-04-03 05:55:35 UTC
(In reply to Emmanuel Bétemps from comment #101)
> Hi,
> 
> I created 3 holds pour the same patron
> Steps 1 to 4 are ok but when I click on the "Batch modify holds" option, the
> Tolls page doesn't list any hold
> I just see the staff interface, the title of this tool (Batch modify holds)
> but no hold is displayed

I'm guessing you tested importing SQL results to tool? We noticed the same problem a while ago and I'm think it has something to do with API.
Comment 104 Emmi Takkinen 2025-04-17 06:33:20 UTC Comment hidden (obsolete)
Comment 105 Andrew Fuerste-Henry 2025-04-17 14:01:41 UTC
On searching, it appears this only uses dates (expiration or suspension) if both ends of the range are supplied. If only one end of the date range is supplied, we should interpret that as an open-ended search -- entering 2025-05-01 fr "Expiration date from" and leaving "Expiration date to" empty should only return holds that have an expiration date greater than 2025-05-01.

We're missing the word "and" when repeating back the user's suspend dates on search results: "suspend date between 2025-04-17-2025-04-18"

You give the message "You have to suspend holds if new suspend until date is set." if one tries to set Suspend Until date without also clicking Suspend Holds. Could you automatically select Suspend Holds for the user if they set a suspend until?

Error when trying to send holds from a report: 
Programming error - op 'form' must start with 'cud-' for POST http://localhost:8081/intranet/tools/batch_modify_holds.pl (referer: http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?id=1&op=run)
Comment 106 Lucas Gass (lukeg) 2025-05-08 20:15:02 UTC
I can select checkboxes from multiple pages now, but when click 'Modify holds' only the ones on my current page are handled. 

Also, we have Bug 23258 which creates/cancels holds. It would be great if we could bring these 2 bugs together, instead of having 2 seperate but similar features?
Comment 107 Emmi Takkinen 2025-05-09 05:59:49 UTC
(In reply to Lucas Gass (lukeg) from comment #106)
> I can select checkboxes from multiple pages now, but when click 'Modify
> holds' only the ones on my current page are handled. 
Oof, so it seems. I'll fix it with other problems Andrew pointed out.

> Also, we have Bug 23258 which creates/cancels holds. It would be great if we
> could bring these 2 bugs together, instead of having 2 seperate but similar
> features?
I see your point and agree, these should probably be one and same tool. I'm just wondering what's the best plan here, push tools as they are to main (when they are ready to be pushed to main) and then "merge" them. Or push one to main and then add other to it? Or merge them before the push?
Comment 108 Emmi Takkinen 2025-05-09 09:14:35 UTC
(In reply to Emmi Takkinen from comment #107)
> (In reply to Lucas Gass (lukeg) from comment #106)
> > I can select checkboxes from multiple pages now, but when click 'Modify
> > holds' only the ones on my current page are handled. 
> Oof, so it seems. I'll fix it with other problems Andrew pointed out.
> 
> > Also, we have Bug 23258 which creates/cancels holds. It would be great if we
> > could bring these 2 bugs together, instead of having 2 seperate but similar
> > features?
> I see your point and agree, these should probably be one and same tool. I'm
> just wondering what's the best plan here, push tools as they are to main
> (when they are ready to be pushed to main) and then "merge" them. Or push
> one to main and then add other to it? Or merge them before the push?

Although... Now when I think about this more, our tools to modify and delete patrons, items and records are all separated. So from that point of view these hold tools could also be different tools.
Comment 109 Emmi Takkinen 2025-05-12 11:10:03 UTC
(In reply to Lucas Gass (lukeg) from comment #106)
> I can select checkboxes from multiple pages now, but when click 'Modify
> holds' only the ones on my current page are handled. 
> 
Looked into this and it seems to be the default for datatables to handle data like this. I tried turning option "serverSide" as false as suggested in the DataTables forum to fetch all data from table, but no luck, it still changes only the holds in current page. We could fetch hold_ids from localStorage, but then we lose ability to check in front-end if modification is possible (e.g. if user tries to suspend hold with found status) and we can't display error message pop-up before attempting to modify holds. 

I came up with following ways to handle this:
1) Fetch found and suspended status for selected holds with API and display error message as pop-up like it is currently displayed.  
2) Make check from back-end and send error message from there back to front.
3) Do not display error message, but do not modify holds which can't be modified either. Show "Following holds couldn't be modified. Reason:..." in "Holds modified" page.
Comment 110 Emmi Takkinen 2025-06-03 07:08:15 UTC
We have encountered some other tricky problems with this (modified holds table isn't rendered in some cases), so getting this to move might take a while.
Comment 111 Emmi Takkinen 2025-06-11 10:47:46 UTC Comment hidden (obsolete)
Comment 112 Lucas Gass (lukeg) 2025-06-16 22:09:36 UTC Comment hidden (obsolete)
Comment 113 Lucas Gass (lukeg) 2025-06-16 22:09:40 UTC Comment hidden (obsolete)
Comment 114 Lucas Gass (lukeg) 2025-06-16 22:09:44 UTC Comment hidden (obsolete)
Comment 115 Lucas Gass (lukeg) 2025-06-16 22:09:48 UTC Comment hidden (obsolete)
Comment 116 Lucas Gass (lukeg) 2025-06-16 22:09:52 UTC Comment hidden (obsolete)
Comment 117 Lucas Gass (lukeg) 2025-06-16 22:09:56 UTC Comment hidden (obsolete)
Comment 118 Lucas Gass (lukeg) 2025-06-16 22:10:00 UTC Comment hidden (obsolete)
Comment 119 Lucas Gass (lukeg) 2025-06-16 22:31:23 UTC
I squashed some of these patches and am resetting to NSO.
Comment 120 Lucas Gass (lukeg) 2025-06-17 14:38:39 UTC
Created attachment 183305 [details] [review]
Bug 36135: Add batch_modify_holds permissions

Sponsored-by: Koha-Suomi Oy
Comment 121 Lucas Gass (lukeg) 2025-06-17 14:38:43 UTC
Created attachment 183306 [details] [review]
Bug 36135: Add ability to batch modify holds

TO TEST:

0. APPLY PATCHES, updatedatabase, restart_all
1. Create a user with limited permissions, making sure you give them the "batch_modify_holds" permission
2. Login as chosen patron.
3. Navigate to Tools page.
4. Note there's a link "Batch modify holds".
5. Remove permission from user.
6. Link to new tool is not displayed.

7. Navigate to new tool Tools->Batch modify holds.
8. Fill in some criteria to search holds with.
9. Confirm that holds found match your search criteria.
10. Set e.g. new expiration date for all or just selected holds.
11. Confirm that holds were modified as they should have.
12. Confirm this also from database.

13. Find holds with a found/waiting status.
14. Attempt to modify their suspend status.
15. Alert box with text "One or more holds have found status and can't be suspended." should be displayed.
16. Play around, test different modifications.

17. Create SQL report which contains column for reserve_id.
18. e.g "SELECT reserve_id FROM reserves limit 10".
19.. Run report.
20. Over report select "Batch operations" should be displayed and under that there should be option "Batch modify holds".
21. Click "Batch modify holds" option.
22. Confirm that holds displayed in "Batch mofidy holds" tool are same as found with your report.

23. Do a holds search that will bring up multiple pages of results
24. Try selecting some holds from different pages to modify
25. Ensure all selected holds are being modified
26. Play around, test different modifications.

Sponsored-by: Koha-Suomi Oy
Comment 122 Lucas Gass (lukeg) 2025-06-17 14:38:47 UTC
Created attachment 183307 [details] [review]
Bug 36135: Fix syntax error in permissions.inc

Sponsored-by: Koha-Suomi Oy
Comment 123 Lucas Gass (lukeg) 2025-06-17 14:38:50 UTC
Created attachment 183308 [details] [review]
Bug 36135: Fix forbidden pattern

Sponsored-by: Koha-Suomi Oy
Comment 124 Lucas Gass (lukeg) 2025-06-17 14:40:46 UTC
squashed patches and added complete test plan, reading for testing!
Comment 125 Andrew Fuerste-Henry 2025-06-17 18:48:48 UTC
(In reply to Lucas Gass (lukeg) from comment #124)
> squashed patches and added complete test plan, reading for testing!

Noting that this all works as intended by chokes if you try to modify a lot of holds at once due to a URI length error. Confirmed it works with about holds but dies with about 1000, so the issue is somewhere in there :)
Comment 126 Lucas Gass (lukeg) 2025-06-17 19:45:39 UTC
(In reply to Andrew Fuerste-Henry from comment #125)
> (In reply to Lucas Gass (lukeg) from comment #124)
> > squashed patches and added complete test plan, reading for testing!
> 
> Noting that this all works as intended by chokes if you try to modify a lot
> of holds at once due to a URI length error. Confirmed it works with about
> holds but dies with about 1000, so the issue is somewhere in there :)

This seems to be a dataTable issue. All of the modified hold ID's are send back to be filtered and the number of IDs seems to be too much for dataTables when we approach 1000 modified holds.
Comment 127 Lucas Gass (lukeg) 2025-06-17 21:46:55 UTC
(In reply to Lucas Gass (lukeg) from comment #126)
> (In reply to Andrew Fuerste-Henry from comment #125)
> > (In reply to Lucas Gass (lukeg) from comment #124)
> > > squashed patches and added complete test plan, reading for testing!
> > 
> > Noting that this all works as intended by chokes if you try to modify a lot
> > of holds at once due to a URI length error. Confirmed it works with about
> > holds but dies with about 1000, so the issue is somewhere in there :)
> 
> This seems to be a dataTable issue. All of the modified hold ID's are send
> back to be filtered and the number of IDs seems to be too much for
> dataTables when we approach 1000 modified holds.

I think we could make the modification results table not be generated from the API.
Comment 128 Lucas Gass (lukeg) 2025-07-03 17:04:09 UTC
Created attachment 183771 [details] [review]
Bug 36135: Make modified_holds table not use the API
Comment 129 Andrew Fuerste-Henry 2025-07-09 13:36:49 UTC
Created attachment 183893 [details] [review]
Bug 36135: Correct tools-menu category logic
Comment 130 Andrew Fuerste-Henry 2025-07-09 14:14:33 UTC
Small searching issue here. When searching by expiration date, one can provide both To and From dates to do a Between search, or one can provide just one date to perform a Before or After search.
But when searching by suspension date, one *must* provide two dates and do a Between search. If only one date is given, the search is not limited on suspension date at all.
Comment 131 Owen Leonard 2025-07-10 11:24:04 UTC
Created attachment 183924 [details] [review]
Bug 36135: (follow-up) Fix suspension search and other improvements

This patch tries to fix the hold suspension search by copying the code
from the expiration date search function.

The patch also makes these changes:

- Change page title to use i18n functions
- Add specific breadcrumb for results page
- Update headings between search and results page
- Correct table settings (definition was missing from
  columns_settings.yml
- Correct capitalization (e.g. "Pickup location" instead of "Pickup
  Location"
- Use include files for displaying title and patron in update results
- Add subtitle and medium to title in search results
- Remove quotes from DataTables option names
- Add hold date to the search results view.
  - Without a specified default sort, the table will sort the first
    column. Since the first column is checkboxes, we ought to explicitly
    set a default sorting column. I thought hold date seemed like a
    better default than title.
Comment 132 Jason Robb 2025-07-10 17:36:01 UTC
Created attachment 183959 [details] [review]
Bug 36135: Add batch_modify_holds permissions

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 133 Jason Robb 2025-07-10 17:36:05 UTC
Created attachment 183960 [details] [review]
Bug 36135: Add ability to batch modify holds

TO TEST:

0. APPLY PATCHES, updatedatabase, restart_all
1. Create a user with limited permissions, making sure you give them the "batch_modify_holds" permission
2. Login as chosen patron.
3. Navigate to Tools page.
4. Note there's a link "Batch modify holds".
5. Remove permission from user.
6. Link to new tool is not displayed.

7. Navigate to new tool Tools->Batch modify holds.
8. Fill in some criteria to search holds with.
9. Confirm that holds found match your search criteria.
10. Set e.g. new expiration date for all or just selected holds.
11. Confirm that holds were modified as they should have.
12. Confirm this also from database.

13. Find holds with a found/waiting status.
14. Attempt to modify their suspend status.
15. Alert box with text "One or more holds have found status and can't be suspended." should be displayed.
16. Play around, test different modifications.

17. Create SQL report which contains column for reserve_id.
18. e.g "SELECT reserve_id FROM reserves limit 10".
19.. Run report.
20. Over report select "Batch operations" should be displayed and under that there should be option "Batch modify holds".
21. Click "Batch modify holds" option.
22. Confirm that holds displayed in "Batch mofidy holds" tool are same as found with your report.

23. Do a holds search that will bring up multiple pages of results
24. Try selecting some holds from different pages to modify
25. Ensure all selected holds are being modified
26. Play around, test different modifications.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 134 Jason Robb 2025-07-10 17:36:09 UTC
Created attachment 183961 [details] [review]
Bug 36135: Fix syntax error in permissions.inc

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 135 Jason Robb 2025-07-10 17:36:13 UTC
Created attachment 183962 [details] [review]
Bug 36135: Fix forbidden pattern

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 136 Jason Robb 2025-07-10 17:36:16 UTC
Created attachment 183963 [details] [review]
Bug 36135: Make modified_holds table not use the API

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 137 Jason Robb 2025-07-10 17:36:20 UTC
Created attachment 183964 [details] [review]
Bug 36135: Correct tools-menu category logic

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 138 Jason Robb 2025-07-10 17:36:24 UTC
Created attachment 183965 [details] [review]
Bug 36135: (follow-up) Fix suspension search and other improvements

This patch tries to fix the hold suspension search by copying the code
from the expiration date search function.

The patch also makes these changes:

- Change page title to use i18n functions
- Add specific breadcrumb for results page
- Update headings between search and results page
- Correct table settings (definition was missing from
  columns_settings.yml
- Correct capitalization (e.g. "Pickup location" instead of "Pickup
  Location"
- Use include files for displaying title and patron in update results
- Add subtitle and medium to title in search results
- Remove quotes from DataTables option names
- Add hold date to the search results view.
  - Without a specified default sort, the table will sort the first
    column. Since the first column is checkboxes, we ought to explicitly
    set a default sorting column. I thought hold date seemed like a
    better default than title.

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 139 Jason Robb 2025-07-10 18:13:15 UTC
Worked great in testing, excited for this!

I did get confused when testing batch modification from a report. If you try to start a new search from that same page of results, the report-supplied filter remains in addition to any newly applied filters. If you want to lift that filter, you have to fully exit the tool and start over.

It's not a major thing but maybe in the future there could be an indication that the results are filtered with an option to clear that filter, similar to how you can see when a filter is present when browsing saved reports.
Comment 140 Emmi Takkinen 2025-07-18 09:56:53 UTC
Thank you Lucas for taking over this! Our libraries have used this tool for a while now and they have pointed out that it should be possible to also modify holds with different status. We thought this should be prevented when we were planning what users should be able to do with the tool but now this has quickly become an issue when holds need to be modified etc. due library being closed.
Comment 141 Lucas Gass (lukeg) 2025-08-01 16:15:04 UTC
Created attachment 185051 [details] [review]
Bug 36135: Add batch_modify_holds permissions

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 142 Lucas Gass (lukeg) 2025-08-01 16:15:09 UTC
Created attachment 185052 [details] [review]
Bug 36135: Add ability to batch modify holds

TO TEST:

0. APPLY PATCHES, updatedatabase, restart_all
1. Create a user with limited permissions, making sure you give them the "batch_modify_holds" permission
2. Login as chosen patron.
3. Navigate to Tools page.
4. Note there's a link "Batch modify holds".
5. Remove permission from user.
6. Link to new tool is not displayed.

7. Navigate to new tool Tools->Batch modify holds.
8. Fill in some criteria to search holds with.
9. Confirm that holds found match your search criteria.
10. Set e.g. new expiration date for all or just selected holds.
11. Confirm that holds were modified as they should have.
12. Confirm this also from database.

13. Find holds with a found/waiting status.
14. Attempt to modify their suspend status.
15. Alert box with text "One or more holds have found status and can't be suspended." should be displayed.
16. Play around, test different modifications.

17. Create SQL report which contains column for reserve_id.
18. e.g "SELECT reserve_id FROM reserves limit 10".
19.. Run report.
20. Over report select "Batch operations" should be displayed and under that there should be option "Batch modify holds".
21. Click "Batch modify holds" option.
22. Confirm that holds displayed in "Batch mofidy holds" tool are same as found with your report.

23. Do a holds search that will bring up multiple pages of results
24. Try selecting some holds from different pages to modify
25. Ensure all selected holds are being modified
26. Play around, test different modifications.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 143 Lucas Gass (lukeg) 2025-08-01 16:15:14 UTC
Created attachment 185053 [details] [review]
Bug 36135: Fix syntax error in permissions.inc

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 144 Lucas Gass (lukeg) 2025-08-01 16:15:19 UTC
Created attachment 185054 [details] [review]
Bug 36135: Fix forbidden pattern

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 145 Lucas Gass (lukeg) 2025-08-01 16:15:25 UTC
Created attachment 185055 [details] [review]
Bug 36135: Make modified_holds table not use the API

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 146 Lucas Gass (lukeg) 2025-08-01 16:15:31 UTC
Created attachment 185056 [details] [review]
Bug 36135: Correct tools-menu category logic

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 147 Lucas Gass (lukeg) 2025-08-01 16:15:37 UTC
Created attachment 185057 [details] [review]
Bug 36135: (follow-up) Fix suspension search and other improvements

This patch tries to fix the hold suspension search by copying the code
from the expiration date search function.

The patch also makes these changes:

- Change page title to use i18n functions
- Add specific breadcrumb for results page
- Update headings between search and results page
- Correct table settings (definition was missing from
  columns_settings.yml
- Correct capitalization (e.g. "Pickup location" instead of "Pickup
  Location"
- Use include files for displaying title and patron in update results
- Add subtitle and medium to title in search results
- Remove quotes from DataTables option names
- Add hold date to the search results view.
  - Without a specified default sort, the table will sort the first
    column. Since the first column is checkboxes, we ought to explicitly
    set a default sorting column. I thought hold date seemed like a
    better default than title.

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 148 Pedro Amorim 2025-08-01 16:42:46 UTC
Created attachment 185073 [details]
Funky filters

1) The new permission batch_modify_holds is not enough to use this tool. If the staff member has that but does not have reserveforothers->place_holds, they can still access the tool but get a 403 error when performing a search at /cgi-bin/koha/tools/batch_modify_holds.pl

2) Something funky happening with select2 filters, check attached picture.
Comment 149 Lucas Gass (lukeg) 2025-08-01 19:57:16 UTC
Thanks for looking, Pedro:

> 1) The new permission batch_modify_holds is not enough to use this tool. If
> the staff member has that but does not have reserveforothers->place_holds,
> they can still access the tool but get a 403 error when performing a search
> at /cgi-bin/koha/tools/batch_modify_holds.pl
Good catch, I'll have a closer look.


> 2) Something funky happening with select2 filters, check attached picture.
I've noticed this too with other select2 dropdowns. I filed a bug to hopefully fix that everywhere:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40583