Bug 36135 - Add tool to batch modify holds
Summary: Add tool to batch modify holds
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Emmi Takkinen
QA Contact: Testopia
URL:
Keywords:
Depends on: 37032
Blocks:
  Show dependency treegraph
 
Reported: 2024-02-20 06:37 UTC by Emmi Takkinen
Modified: 2025-03-07 14:50 UTC (History)
15 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
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

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
Created attachment 163021 [details] [review]
Bug 36135: Add new permission batch_modify_holds

This patch adds new permission "batch_modify_holds".

To test:
1. Apply this patch.
2. Run updatedatabase.pl.
3. Check that new permission was added to Patrons->Set permissions.

Sponsored-by: Koha-Suomi Oy
Comment 4 Emmi Takkinen 2024-03-11 11:52:14 UTC
Created attachment 163022 [details] [review]
Bug 36135: Add links to the batch modify holds tool

This patch adds links to new tool batch modify holds tool.

To test:
1. Apply this patch.
2. Add patron permission "batch_modify_holds".
3. Login as chosen patron.
4. Navigate to Tools page.
=> Note there's a link "Batch modify holds".
5. Remove permission from user.
=> Link to new tool is not displayed.

Sponsored-by: Koha-Suomi Oy
Comment 5 Emmi Takkinen 2024-03-11 11:52:41 UTC
Created attachment 163023 [details] [review]
Bug 36135: Add new tool to batch modify holds

We sometimes need to modify holds due library closing down etc.
For this we need tool to batch modify holds which allows us
to filter holds and modify them.

Tool allows users to select holds with following parameters:
 - expiration date
    -> if patron_expiration_date is set, it is used as main search filter
    -> otherwise expirationdate is used
 - library
 - found status
 - suspended status
 - suspend until date
 - hold note

Users are able to modify following  with tool:
 - expiration date (not expiration date set by patron)
 - pick up library
 - suspend status (only if hold has no found status)
 - suspend until date (only if suspend status is set)
 - hold note

To test:
1. Apply this patch.
2. Navigate to new tool Tools->Batch modify holds.
3. Fill in some criteria to search holds with.
=> Confirm that holds found match your search criteria.
4. Set e.g. new expiration date for all or just selected holds.
=> Confirm that holds were modified as they should have.
=> Confirm this also from database.

To test suspending holds with found status:
1. Find holds with found status.
2. Attempt to modify their suspend status.
=> Alert box with text "One or more holds have found status
and can't be suspended." should be displayed.

Play around, test different search compinations and modifications.

Sponsored-by: Koha-Suomi Oy
Comment 6 Emmi Takkinen 2024-03-11 11:53:08 UTC
Created attachment 163024 [details] [review]
Bug 36135: Allow using SQL results in batch modify holds tool

This patch adds ability to use SQL results for batch holds
tool if report contains column reserve_id.

To test:
1. Create SQL report which contains column for reserve_id.
=> e.g "SELECT reserve_id FROM reserves limit 10".
2. Run report.
3. Over report select "Batch operations" should be displayed
and under that there should be option "Batch modify holds".
4. Click "Batch modify holds" option.
=> Confirm that holds displayed in "Batch mofidy holds" tool
are same as found with your report.

Sponsored-by: Koha-Suomi Oy
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
Created attachment 163529 [details] [review]
Bug 36135: (follow-up) Adjust for CSRF
Comment 10 Nick Clemens (kidclamp) 2024-03-20 14:16:37 UTC
Created attachment 163530 [details] [review]
Bug 36135: (follow-up) Adjust for CSRF
Comment 11 Laura Escamilla 2024-03-20 14:32:57 UTC
Created attachment 163540 [details] [review]
Bug 36135: Add new permission batch_modify_holds

This patch adds new permission "batch_modify_holds".

To test:
1. Apply this patch.
2. Run updatedatabase.pl.
3. Check that new permission was added to Patrons->Set permissions.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Comment 12 Laura Escamilla 2024-03-20 14:33:06 UTC
Created attachment 163541 [details] [review]
Bug 36135: Add links to the batch modify holds tool

This patch adds links to new tool batch modify holds tool.

To test:
1. Apply this patch.
2. Add patron permission "batch_modify_holds".
3. Login as chosen patron.
4. Navigate to Tools page.
=> Note there's a link "Batch modify holds".
5. Remove permission from user.
=> Link to new tool is not displayed.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Comment 13 Laura Escamilla 2024-03-20 14:33:09 UTC
Created attachment 163542 [details] [review]
Bug 36135: Add new tool to batch modify holds

We sometimes need to modify holds due library closing down etc.
For this we need tool to batch modify holds which allows us
to filter holds and modify them.

Tool allows users to select holds with following parameters:
 - expiration date
    -> if patron_expiration_date is set, it is used as main search filter
    -> otherwise expirationdate is used
 - library
 - found status
 - suspended status
 - suspend until date
 - hold note

Users are able to modify following  with tool:
 - expiration date (not expiration date set by patron)
 - pick up library
 - suspend status (only if hold has no found status)
 - suspend until date (only if suspend status is set)
 - hold note

To test:
1. Apply this patch.
2. Navigate to new tool Tools->Batch modify holds.
3. Fill in some criteria to search holds with.
=> Confirm that holds found match your search criteria.
4. Set e.g. new expiration date for all or just selected holds.
=> Confirm that holds were modified as they should have.
=> Confirm this also from database.

To test suspending holds with found status:
1. Find holds with found status.
2. Attempt to modify their suspend status.
=> Alert box with text "One or more holds have found status
and can't be suspended." should be displayed.

Play around, test different search compinations and modifications.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Comment 14 Laura Escamilla 2024-03-20 14:33:11 UTC
Created attachment 163543 [details] [review]
Bug 36135: Allow using SQL results in batch modify holds tool

This patch adds ability to use SQL results for batch holds
tool if report contains column reserve_id.

To test:
1. Create SQL report which contains column for reserve_id.
=> e.g "SELECT reserve_id FROM reserves limit 10".
2. Run report.
3. Over report select "Batch operations" should be displayed
and under that there should be option "Batch modify holds".
4. Click "Batch modify holds" option.
=> Confirm that holds displayed in "Batch mofidy holds" tool
are same as found with your report.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Comment 15 Laura Escamilla 2024-03-20 14:33:13 UTC
Created attachment 163544 [details] [review]
Bug 36135: (follow-up) Adjust for CSRF

Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
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
Created attachment 163796 [details] [review]
Bug 36135: Add several new features

This patch adds following new features to batch
modify holds tool.

- Do not automatically select login library as search param
- Add biblios title and items barcode to found holds table
- Add count of found holds at top of the table
- Add ability to do not change pick up library
- Display those hold rows with some found status in different
color

Sponsored-by: Koha-Suomi Oy
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
Created attachment 164037 [details] [review]
Bug 36135: Fix QA issues and add improvemnents

This patch:

- Fixes issues reported by qa-tool
- Adds info about how many holds were modified
- Add title and barcode columns to table of
modified holds
- Adds div with class "page-section" to
"Modify holds" table

Sponsored-by: Koha-Suomi Oy
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
Created attachment 164051 [details] [review]
Bug 36135: Fix QA issues and add improvements

This patch:

- Fixes issues reported by qa-tool
- Adds info about how many holds were modified
- Add title and barcode columns to table of
modified holds
- Adds div with class "page-section" to
"Modify holds" table

Sponsored-by: Koha-Suomi Oy
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
Created attachment 164057 [details] [review]
Bug 36135: Add new permission batch_modify_holds

This patch adds new permission "batch_modify_holds".

To test:
1. Apply this patch.
2. Run updatedatabase.pl.
3. Check that new permission was added to Patrons->Set permissions.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 25 Lucas Gass (lukeg) 2024-03-28 14:12:55 UTC
Created attachment 164058 [details] [review]
Bug 36135: Add links to the batch modify holds tool

This patch adds links to new tool batch modify holds tool.

To test:
1. Apply this patch.
2. Add patron permission "batch_modify_holds".
3. Login as chosen patron.
4. Navigate to Tools page.
=> Note there's a link "Batch modify holds".
5. Remove permission from user.
=> Link to new tool is not displayed.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 26 Lucas Gass (lukeg) 2024-03-28 14:12:58 UTC
Created attachment 164059 [details] [review]
Bug 36135: Add new tool to batch modify holds

We sometimes need to modify holds due library closing down etc.
For this we need tool to batch modify holds which allows us
to filter holds and modify them.

Tool allows users to select holds with following parameters:
 - expiration date
    -> if patron_expiration_date is set, it is used as main search filter
    -> otherwise expirationdate is used
 - library
 - found status
 - suspended status
 - suspend until date
 - hold note

Users are able to modify following  with tool:
 - expiration date (not expiration date set by patron)
 - pick up library
 - suspend status (only if hold has no found status)
 - suspend until date (only if suspend status is set)
 - hold note

To test:
1. Apply this patch.
2. Navigate to new tool Tools->Batch modify holds.
3. Fill in some criteria to search holds with.
=> Confirm that holds found match your search criteria.
4. Set e.g. new expiration date for all or just selected holds.
=> Confirm that holds were modified as they should have.
=> Confirm this also from database.

To test suspending holds with found status:
1. Find holds with found status.
2. Attempt to modify their suspend status.
=> Alert box with text "One or more holds have found status
and can't be suspended." should be displayed.

Play around, test different search compinations and modifications.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 27 Lucas Gass (lukeg) 2024-03-28 14:13:00 UTC
Created attachment 164060 [details] [review]
Bug 36135: Allow using SQL results in batch modify holds tool

This patch adds ability to use SQL results for batch holds
tool if report contains column reserve_id.

To test:
1. Create SQL report which contains column for reserve_id.
=> e.g "SELECT reserve_id FROM reserves limit 10".
2. Run report.
3. Over report select "Batch operations" should be displayed
and under that there should be option "Batch modify holds".
4. Click "Batch modify holds" option.
=> Confirm that holds displayed in "Batch mofidy holds" tool
are same as found with your report.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 28 Lucas Gass (lukeg) 2024-03-28 14:13:02 UTC
Created attachment 164061 [details] [review]
Bug 36135: (follow-up) Adjust for CSRF

Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 29 Lucas Gass (lukeg) 2024-03-28 14:13:05 UTC
Created attachment 164062 [details] [review]
Bug 36135: Add several new features

This patch adds following new features to batch
modify holds tool.

- Do not automatically select login library as search param
- Add biblios title and items barcode to found holds table
- Add count of found holds at top of the table
- Add ability to do not change pick up library
- Display those hold rows with some found status in different
color

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 30 Lucas Gass (lukeg) 2024-03-28 14:13:07 UTC
Created attachment 164063 [details] [review]
Bug 36135: Fix QA issues and add improvements

This patch:

- Fixes issues reported by qa-tool
- Adds info about how many holds were modified
- Add title and barcode columns to table of
modified holds
- Adds div with class "page-section" to
"Modify holds" table

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
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
Created attachment 177518 [details] [review]
Bug 36135: Report errors to a modal
Comment 38 Lucas Gass (lukeg) 2025-02-04 20:58:58 UTC
Created attachment 177519 [details] [review]
Bug 36135: Fix breadcrumbs/Bootstrap issues
Comment 39 Lucas Gass (lukeg) 2025-02-12 18:24:49 UTC
Created attachment 177959 [details] [review]
Bug 36135: Fix some markup errors
Comment 40 Lucas Gass (lukeg) 2025-02-12 18:28:50 UTC
Created attachment 177960 [details] [review]
Bug 36135: Add new permission batch_modify_holds

This patch adds new permission "batch_modify_holds".

To test:
1. Apply this patch.
2. Run updatedatabase.pl.
3. Check that new permission was added to Patrons->Set permissions.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 41 Lucas Gass (lukeg) 2025-02-12 18:28:54 UTC
Created attachment 177961 [details] [review]
Bug 36135: Add links to the batch modify holds tool

This patch adds links to new tool batch modify holds tool.

To test:
1. Apply this patch.
2. Add patron permission "batch_modify_holds".
3. Login as chosen patron.
4. Navigate to Tools page.
=> Note there's a link "Batch modify holds".
5. Remove permission from user.
=> Link to new tool is not displayed.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 42 Lucas Gass (lukeg) 2025-02-12 18:28:57 UTC
Created attachment 177962 [details] [review]
Bug 36135: Add new tool to batch modify holds

We sometimes need to modify holds due library closing down etc.
For this we need tool to batch modify holds which allows us
to filter holds and modify them.

Tool allows users to select holds with following parameters:
 - expiration date
    -> if patron_expiration_date is set, it is used as main search filter
    -> otherwise expirationdate is used
 - library
 - found status
 - suspended status
 - suspend until date
 - hold note

Users are able to modify following  with tool:
 - expiration date (not expiration date set by patron)
 - pick up library
 - suspend status (only if hold has no found status)
 - suspend until date (only if suspend status is set)
 - hold note

To test:
1. Apply this patch.
2. Navigate to new tool Tools->Batch modify holds.
3. Fill in some criteria to search holds with.
=> Confirm that holds found match your search criteria.
4. Set e.g. new expiration date for all or just selected holds.
=> Confirm that holds were modified as they should have.
=> Confirm this also from database.

To test suspending holds with found status:
1. Find holds with found status.
2. Attempt to modify their suspend status.
=> Alert box with text "One or more holds have found status
and can't be suspended." should be displayed.

Play around, test different search compinations and modifications.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 43 Lucas Gass (lukeg) 2025-02-12 18:29:01 UTC
Created attachment 177963 [details] [review]
Bug 36135: Allow using SQL results in batch modify holds tool

This patch adds ability to use SQL results for batch holds
tool if report contains column reserve_id.

To test:
1. Create SQL report which contains column for reserve_id.
=> e.g "SELECT reserve_id FROM reserves limit 10".
2. Run report.
3. Over report select "Batch operations" should be displayed
and under that there should be option "Batch modify holds".
4. Click "Batch modify holds" option.
=> Confirm that holds displayed in "Batch mofidy holds" tool
are same as found with your report.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 44 Lucas Gass (lukeg) 2025-02-12 18:29:04 UTC
Created attachment 177964 [details] [review]
Bug 36135: (follow-up) Adjust for CSRF

Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 45 Lucas Gass (lukeg) 2025-02-12 18:29:07 UTC
Created attachment 177965 [details] [review]
Bug 36135: Add several new features

This patch adds following new features to batch
modify holds tool.

- Do not automatically select login library as search param
- Add biblios title and items barcode to found holds table
- Add count of found holds at top of the table
- Add ability to do not change pick up library
- Display those hold rows with some found status in different
color

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 46 Lucas Gass (lukeg) 2025-02-12 18:29:11 UTC
Created attachment 177966 [details] [review]
Bug 36135: Fix QA issues and add improvements

This patch:

- Fixes issues reported by qa-tool
- Adds info about how many holds were modified
- Add title and barcode columns to table of
modified holds
- Adds div with class "page-section" to
"Modify holds" table

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 47 Lucas Gass (lukeg) 2025-02-12 18:29:14 UTC
Created attachment 177967 [details] [review]
Bug 36135: Report errors to a modal
Comment 48 Lucas Gass (lukeg) 2025-02-12 18:29:17 UTC
Created attachment 177968 [details] [review]
Bug 36135: Fix breadcrumbs/Bootstrap issues
Comment 49 Lucas Gass (lukeg) 2025-02-12 18:29:20 UTC
Created attachment 177969 [details] [review]
Bug 36135: Fix some markup errors
Comment 50 Lucas Gass (lukeg) 2025-02-12 18:30:24 UTC
Rebased using auto_rebase.pl
Comment 51 Lucas Gass (lukeg) 2025-02-12 21:16:05 UTC
Created attachment 177975 [details] [review]
Bug 36135: API the found_holds table
Comment 52 Lucas Gass (lukeg) 2025-02-12 21:18:29 UTC
Created attachment 177976 [details] [review]
Bug 36135: Embed items
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
Created attachment 177981 [details] [review]
Bug 36135: Add ability to fetch patron info via API holds endpoint
Comment 55 Emmi Takkinen 2025-02-13 07:34:49 UTC
Created attachment 177982 [details] [review]
Bug 36135: Add more API functionality to display holds to modify
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
Created attachment 178154 [details] [review]
Bug 36135: Add new permission batch_modify_holds

This patch adds new permission "batch_modify_holds".

To test:
1. Apply this patch.
2. Run updatedatabase.pl.
3. Check that new permission was added to Patrons->Set permissions.

Sponsored-by: Koha-Suomi Oy
Comment 61 Emmi Takkinen 2025-02-18 06:31:57 UTC
Created attachment 178155 [details] [review]
Bug 36135: Add links to the batch modify holds tool

This patch adds links to new tool batch modify holds tool.

To test:
1. Apply this patch.
2. Add patron permission "batch_modify_holds".
3. Login as chosen patron.
4. Navigate to Tools page.
=> Note there's a link "Batch modify holds".
5. Remove permission from user.
=> Link to new tool is not displayed.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 62 Emmi Takkinen 2025-02-18 06:32:29 UTC
Created attachment 178156 [details] [review]
Bug 36135: Add new tool to batch modify holds

We sometimes need to modify holds due library closing down etc.
For this we need tool to batch modify holds which allows us
to filter holds and modify them.

Tool allows users to select holds with following parameters:
 - expiration date
    -> if patron_expiration_date is set, it is used as main search filter
    -> otherwise expirationdate is used
 - library
 - found status
 - suspended status
 - suspend until date
 - hold note

Users are able to modify following  with tool:
 - expiration date (not expiration date set by patron)
 - pick up library
 - suspend status (only if hold has no found status)
 - suspend until date (only if suspend status is set)
 - hold note

To test:
1. Apply this patch.
2. Navigate to new tool Tools->Batch modify holds.
3. Fill in some criteria to search holds with.
=> Confirm that holds found match your search criteria.
4. Set e.g. new expiration date for all or just selected holds.
=> Confirm that holds were modified as they should have.
=> Confirm this also from database.

To test suspending holds with found status:
1. Find holds with found status.
2. Attempt to modify their suspend status.
=> Alert box with text "One or more holds have found status
and can't be suspended." should be displayed.

Play around, test different search compinations and modifications.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 63 Emmi Takkinen 2025-02-18 06:32:57 UTC
Created attachment 178157 [details] [review]
Bug 36135: Allow using SQL results in batch modify  holds tool

This patch adds ability to use SQL results for batch holds
tool if report contains column reserve_id.

To test:
1. Create SQL report which contains column for reserve_id.
=> e.g "SELECT reserve_id FROM reserves limit 10".
2. Run report.
3. Over report select "Batch operations" should be displayed
and under that there should be option "Batch modify holds".
4. Click "Batch modify holds" option.
=> Confirm that holds displayed in "Batch mofidy holds" tool
are same as found with your report.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 64 Emmi Takkinen 2025-02-18 06:33:21 UTC
Created attachment 178158 [details] [review]
Bug 36135: (follow-up) Adjust for CSRF

Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 65 Emmi Takkinen 2025-02-18 06:33:56 UTC
Created attachment 178159 [details] [review]
Bug 36135: Add several new features

This patch adds following new features to batch
modify holds tool.

- Do not automatically select login library as search param
- Add biblios title and items barcode to found holds table
- Add count of found holds at top of the table
- Add ability to do not change pick up library
- Display those hold rows with some found status in different
color

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 66 Emmi Takkinen 2025-02-18 06:34:25 UTC
Created attachment 178160 [details] [review]
Bug 36135: Fix QA issues and add improvements

This patch:

- Fixes issues reported by qa-tool
- Adds info about how many holds were modified
- Add title and barcode columns to table of
modified holds
- Adds div with class "page-section" to
"Modify holds" table

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 67 Emmi Takkinen 2025-02-18 06:34:50 UTC
Created attachment 178161 [details] [review]
Bug 36135: Fix breadcrumbs/Bootstrap issues
Comment 68 Emmi Takkinen 2025-02-18 06:35:11 UTC
Created attachment 178162 [details] [review]
Bug 36135: Fix some markup errors
Comment 69 Emmi Takkinen 2025-02-18 06:35:31 UTC
Created attachment 178163 [details] [review]
Bug 36135: Embed items
Comment 70 Emmi Takkinen 2025-02-18 06:35:50 UTC
Created attachment 178164 [details] [review]
Bug 36135: Add ability to fetch patron info via API holds endpoint
Comment 71 Emmi Takkinen 2025-02-18 06:36:14 UTC
Created attachment 178165 [details] [review]
Bug 36135: Use API and datatables to display holds to modify
Comment 72 Emmi Takkinen 2025-02-18 06:36:32 UTC
Created attachment 178166 [details] [review]
Bug 36135: Save state of selected checkboxes and fix modifying suspend
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
Created attachment 178954 [details] [review]
Bug 36135: Batch clear hold notes and use select2 in multiple filters

This patch adds ability to clear hold note from all selected
holds. It also adds select2 to libraries and holds status
filters.
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
Created attachment 179028 [details] [review]
Bug 36135: Add new permission batch_modify_holds

This patch adds new permission "batch_modify_holds".

To test:
1. Apply this patch.
2. Run updatedatabase.pl.
3. Check that new permission was added to Patrons->Set permissions.

Sponsored-by: Koha-Suomi Oy
Comment 84 Emmi Takkinen 2025-03-07 07:47:13 UTC
Created attachment 179029 [details] [review]
Bug 36135: Add links to the batch modify holds tool

This patch adds links to new tool batch modify holds tool.

To test:
1. Apply this patch.
2. Add patron permission "batch_modify_holds".
3. Login as chosen patron.
4. Navigate to Tools page.
=> Note there's a link "Batch modify holds".
5. Remove permission from user.
=> Link to new tool is not displayed.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 85 Emmi Takkinen 2025-03-07 07:47:49 UTC
Created attachment 179030 [details] [review]
Bug 36135: Add new tool to batch modify holds

We sometimes need to modify holds due library closing down etc.
For this we need tool to batch modify holds which allows us
to filter holds and modify them.

Tool allows users to select holds with following parameters:
 - expiration date
    -> if patron_expiration_date is set, it is used as main search filter
    -> otherwise expirationdate is used
 - library
 - found status
 - suspended status
 - suspend until date
 - hold note

Users are able to modify following  with tool:
 - expiration date (not expiration date set by patron)
 - pick up library
 - suspend status (only if hold has no found status)
 - suspend until date (only if suspend status is set)
 - hold note

To test:
1. Apply this patch.
2. Navigate to new tool Tools->Batch modify holds.
3. Fill in some criteria to search holds with.
=> Confirm that holds found match your search criteria.
4. Set e.g. new expiration date for all or just selected holds.
=> Confirm that holds were modified as they should have.
=> Confirm this also from database.

To test suspending holds with found status:
1. Find holds with found status.
2. Attempt to modify their suspend status.
=> Alert box with text "One or more holds have found status
and can't be suspended." should be displayed.

Play around, test different search compinations and modifications.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 86 Emmi Takkinen 2025-03-07 07:48:20 UTC
Created attachment 179031 [details] [review]
Bug 36135: Allow using SQL results in batch modify  holds tool

This patch adds ability to use SQL results for batch holds
tool if report contains column reserve_id.

To test:
1. Create SQL report which contains column for reserve_id.
=> e.g "SELECT reserve_id FROM reserves limit 10".
2. Run report.
3. Over report select "Batch operations" should be displayed
and under that there should be option "Batch modify holds".
4. Click "Batch modify holds" option.
=> Confirm that holds displayed in "Batch mofidy holds" tool
are same as found with your report.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 87 Emmi Takkinen 2025-03-07 07:48:56 UTC
Created attachment 179032 [details] [review]
Bug 36135: (follow-up) Adjust for CSRF

Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 88 Emmi Takkinen 2025-03-07 07:49:25 UTC
Created attachment 179033 [details] [review]
Bug 36135: Add several new features

This patch adds following new features to batch
modify holds tool.

- Do not automatically select login library as search param
- Add biblios title and items barcode to found holds table
- Add count of found holds at top of the table
- Add ability to do not change pick up library
- Display those hold rows with some found status in different
color

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 89 Emmi Takkinen 2025-03-07 07:49:55 UTC
Created attachment 179034 [details] [review]
Bug 36135: Fix QA issues and add improvements

This patch:

- Fixes issues reported by qa-tool
- Adds info about how many holds were modified
- Add title and barcode columns to table of
modified holds
- Adds div with class "page-section" to
"Modify holds" table

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 90 Emmi Takkinen 2025-03-07 07:50:25 UTC
Created attachment 179035 [details] [review]
Bug 36135: Report errors to a modal
Comment 91 Emmi Takkinen 2025-03-07 07:50:46 UTC
Created attachment 179036 [details] [review]
Bug 36135: Fix breadcrumbs/Bootstrap issues
Comment 92 Emmi Takkinen 2025-03-07 07:51:07 UTC
Created attachment 179037 [details] [review]
Bug 36135: Fix some markup errors
Comment 93 Emmi Takkinen 2025-03-07 07:51:33 UTC
Created attachment 179038 [details] [review]
Bug 36135: Embed items
Comment 94 Emmi Takkinen 2025-03-07 07:52:01 UTC
Created attachment 179039 [details] [review]
Bug 36135: Add ability to fetch patron info via API holds endpoint
Comment 95 Emmi Takkinen 2025-03-07 07:52:28 UTC
Created attachment 179040 [details] [review]
Bug 36135: Use API and datatables to display holds to modify
Comment 96 Emmi Takkinen 2025-03-07 07:52:53 UTC
Created attachment 179041 [details] [review]
Bug 36135: Save state of selected checkboxes and fix modifying suspend
Comment 97 Emmi Takkinen 2025-03-07 07:53:15 UTC
Created attachment 179042 [details] [review]
Bug 36135: Batch clear hold notes and use select2 in multiple filters
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
Created attachment 179081 [details] [review]
Bug 36135: (QA follow-up): fix typo in tool permissions

The flagsrequired for the new page was incorrectly set to 'batch_modify_holds.tt'
Comment 100 Lucas Gass (lukeg) 2025-03-07 14:50:06 UTC
Created attachment 179082 [details] [review]
Bug 36135: (follow-up) Fix link style on guided report page