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.
Corrected
(In reply to Emmi Takkinen from comment #1) > Corrected ... the title, we should extend expiration dates, not pick up dates.
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
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
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
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
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.
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)
Created attachment 163529 [details] [review] Bug 36135: (follow-up) Adjust for CSRF
Created attachment 163530 [details] [review] Bug 36135: (follow-up) Adjust for CSRF
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>
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>
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>
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>
Created attachment 163544 [details] [review] Bug 36135: (follow-up) Adjust for CSRF Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
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.
(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.
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
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"/>
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
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.
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
Thanks Emmi! This looks great.
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>
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>
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>
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>
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>
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>
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>
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
(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.
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.
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.
I would love to see this one pick up again. It's a popular request from our partner libraries.
Looks like the templates now need to be adjusted for Bootstrap 5 changes.
Created attachment 177518 [details] [review] Bug 36135: Report errors to a modal
Created attachment 177519 [details] [review] Bug 36135: Fix breadcrumbs/Bootstrap issues
Created attachment 177959 [details] [review] Bug 36135: Fix some markup errors
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>
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>
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>
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>
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>
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>
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>
Created attachment 177967 [details] [review] Bug 36135: Report errors to a modal
Created attachment 177968 [details] [review] Bug 36135: Fix breadcrumbs/Bootstrap issues
Created attachment 177969 [details] [review] Bug 36135: Fix some markup errors
Rebased using auto_rebase.pl
Created attachment 177975 [details] [review] Bug 36135: API the found_holds table
Created attachment 177976 [details] [review] Bug 36135: Embed items
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?
Created attachment 177981 [details] [review] Bug 36135: Add ability to fetch patron info via API holds endpoint
Created attachment 177982 [details] [review] Bug 36135: Add more API functionality to display holds to modify
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.
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.
(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
(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
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
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>
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>
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>
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>
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>
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>
Created attachment 178161 [details] [review] Bug 36135: Fix breadcrumbs/Bootstrap issues
Created attachment 178162 [details] [review] Bug 36135: Fix some markup errors
Created attachment 178163 [details] [review] Bug 36135: Embed items
Created attachment 178164 [details] [review] Bug 36135: Add ability to fetch patron info via API holds endpoint
Created attachment 178165 [details] [review] Bug 36135: Use API and datatables to display holds to modify
Created attachment 178166 [details] [review] Bug 36135: Save state of selected checkboxes and fix modifying suspend
All patches are now rebased and ready for testing.
There is still some issues with modifying suspend status.
Created attachment 178167 [details] [review] Bug 36135: Save state of selected checkboxes and fix modifying suspend
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.
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.
Sorry Emmi, patches don't apply to current main.
(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.
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
(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.
(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.
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
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>
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>
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>
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>
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>
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>
Created attachment 179035 [details] [review] Bug 36135: Report errors to a modal
Created attachment 179036 [details] [review] Bug 36135: Fix breadcrumbs/Bootstrap issues
Created attachment 179037 [details] [review] Bug 36135: Fix some markup errors
Created attachment 179038 [details] [review] Bug 36135: Embed items
Created attachment 179039 [details] [review] Bug 36135: Add ability to fetch patron info via API holds endpoint
Created attachment 179040 [details] [review] Bug 36135: Use API and datatables to display holds to modify
Created attachment 179041 [details] [review] Bug 36135: Save state of selected checkboxes and fix modifying suspend
Created attachment 179042 [details] [review] Bug 36135: Batch clear hold notes and use select2 in multiple filters
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).
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'
Created attachment 179082 [details] [review] Bug 36135: (follow-up) Fix link style on guided report page