Description
Nick Clemens (kidclamp)
2022-10-06 13:22:25 UTC
We would love to see this capability! It would be ideal if moving all holds on the bib at once were built in. This would be helpful for bibs with many (over 50) holds on them. This would be a fantastic solution to a problem we've been having with re-orders of older titles, where we get the newer edition as a replacement. Our current workaround creates some pretty yucky metadata. (In reply to Felicity Brown from comment #2) > This would be a fantastic solution to a problem we've been having with > re-orders of older titles, where we get the newer edition as a replacement. > Our current workaround creates some pretty yucky metadata. Same for us. This function would be fantastic for our library. Currently, our processes to move holds is time consuming and prone to errors. This would be great! One thing to specifically note is that the date that the hold was placed on the original item would be preserved. This was the order of the holds queue could be preserved and sorted in the the holds on the new item. This sounds good! I totally agree that currently the transfer of the holds takes time and the possibility of errors is high. I think these are the things that can go wrong now while doing the hold transfer and should be considered when building this function: - It should be taken into account whether the hold has been suspended or whether it is a itemlevel hold or a non-urgent hold. - In addition, you must choose the right pick-up location for the hold, change the hold date and priority of the hold. I agree. We are facing the same situation. A new action on the holds table on request.pl 'Move hold' would be very helpful Created attachment 179226 [details] [review] Bug 31698: Add alter_hold_targets permission Created attachment 179235 [details] [review] Bug 31698: Add new move_hold_item subroutine to Hold.pm Created attachment 179236 [details] [review] Bug 31698: Implement move_hold_item in request.pl Created attachment 179279 [details] [review] Bug 31698: Make move_hold_item use CanItemBeReserved and add logging Created attachment 179287 [details] [review] Bug 31698: Template and javascript changes to request.tt Created attachment 179290 [details] [review] Bug 31698: Report errors and failures back to request.tt Created attachment 179291 [details] [review] Bug 31698: Deal with priority Created attachment 179292 [details] [review] Bug 31698: Improve priority Created attachment 179295 [details] [review] Bug 31698: Flip the logic for priority Created attachment 179704 [details] [review] Bug 31698: Add missing filters Created attachment 179711 [details] [review] Bug 31698: Use _ShiftPriority to deal with priority Created attachment 179713 [details] [review] Bug 31698: Call _FixPriority on orginal biblio to clean-up priorty Created attachment 181222 [details] [review] Bug 31698: Add alter_hold_targets permission Created attachment 181223 [details] [review] Bug 31698: Add new move_hold_item subroutine to Hold.pm Created attachment 181224 [details] [review] Bug 31698: Template and javascript changes to request.tt Created attachment 181225 [details] [review] Bug 31698: Report errors and failures back to request.tt Created attachment 181226 [details] [review] Bug 31698: Add missing filters Created attachment 181228 [details] [review] Bug 31698: Add alter_hold_targets permission Created attachment 181229 [details] [review] Bug 31698: Add new move_hold_item subroutine to Hold.pm and request.pl Created attachment 181230 [details] [review] Bug 31698: Template and javascript changes to request.tt Created attachment 181231 [details] [review] Bug 31698: Report errors and failures back to request.tt Created attachment 181232 [details] [review] Bug 31698: Add support for moving record level holds Created attachment 181233 [details] [review] Bug 31698: Combine item and record level subroutines Created attachment 181234 [details] [review] Bug 31698: Add ability to move holds to new items or new records Created attachment 181929 [details] [review] Bug 31698: Fix modals for item level moves Created attachment 181930 [details] [review] Bug 31698: Fix modals for biblio level moves Created attachment 181944 [details] [review] Bug 31698: Add alter_hold_targets permission Created attachment 181945 [details] [review] Bug 31698: Add ability to move holds to new items or new records Created attachment 181946 [details] [review] Bug 31698: Fix modals for item level moves Created attachment 181947 [details] [review] Bug 31698: Fix modals for biblio level moves Created attachment 181948 [details] [review] Bug 31698: Add unit tests Created attachment 181951 [details] [review] Bug 31698: Add alter_hold_targets permission Created attachment 181952 [details] [review] Bug 31698: Add ability to move holds to new items or new records Created attachment 181953 [details] [review] Bug 31698: Fix modals for item level moves Created attachment 181954 [details] [review] Bug 31698: Fix modals for biblio level moves Created attachment 181955 [details] [review] Bug 31698: Add unit tests Created attachment 181956 [details] [review] Bug 31698: Disallow moving waiting or intransit holds Created attachment 181957 [details] [review] Bug 31698: Add alter_hold_targets permission Created attachment 181958 [details] [review] Bug 31698: Add ability to move holds to new items or new records Test plan: 1. APPLY PATCH, updatedatabase, restart_all 2. Have two staff members. 1. Superlibrarian, 2. Staff with new permission to `alter_hold_targets` 3. Now place some holds on different record, you'll want some item level holds and some record level holds. 4. After placing some holds go to a record with some holds on it. 5. Go to the holds tab (reserve/request.p) 6. You should notice a disabled 'Move selected (0)' button, next to 'Cancel selected (0)' 7. Select some holds to move, ensure that the number updates accordingly. ( Move selected (2) ) 8. When you click the button you should see 2 options, 'Item level holds to a different item' and 'Record level holds to a different record' 9. Clicking on either should bring up a modal with information about the holds you selected to move. 10. If you have item level holds and you select 'Item level holds to a different item' those holds should appear pre-checked. 11. If you have item level holds and you select 'Record level holds to a different record' those holds checkboxes should be disabled by default, you should see the 'Problem' as 'Cannot move a waiting or record level hold' 12. If you are moving a item level hold you should see a searchbar labeled 'Enter the item barcode of new hold target:'. 13. Enter a full or partial barcode and click search 14. You should see your item, if it exists. 15. Check 'Move all selected item level holds to this item', once you do so the yellow 'Move selected holds' button should be enabled. 16. Click 'Move selected holds', the hold should be moved. 17. You should see a message like " Moved the following holds: Hold ID: 1854 - Moved from The Arden edition of the works of William Shakespeare. to The Arden edition of the works of William Shakespeare." 19. Do steps 12 - 17 again, but with a record level hold. 20. Now make or have many holds on a specific target record to test priority. Make note of the current priority 21. Now move a hold on to that record, testing both item and record level holds. 22. Now on the target record review the priority. The holds originally on the record should not have changed. The hold you move should now have the lowest priority on the record. 23. Check the original record, if a hold was moved all other priority should have been adjusted correctly. FOr example, if the hold you moved was 3 of 4 then the others should have adjusted priority of 1 2 3 now. 24. Turn on HoldsLog if it is disabled. 25. Move some holds, both item and record ones, and review the logs after each one. There should be a correct MODIFY entry for each move. 26. Make an item with a hold on it as damaged and turn on AllowHoldsOnDamagedItems. Try to move the hold, you should get an error telling you that the target item is damaged. 27. Now login as the non-superlibrarian patron from step 2. If they have the `alter_hold_targets` permission they should see the 'Move selected' button. 28. Revoke the `alter_hold_targets` permission. The button should not appear now. Created attachment 181959 [details] [review] Bug 31698: Fix modals for item level moves Created attachment 181960 [details] [review] Bug 31698: Fix modals for biblio level moves Created attachment 181961 [details] [review] Bug 31698: Add unit tests Created attachment 181962 [details] [review] Bug 31698: Disallow moving waiting or intransit holds Created attachment 182639 [details] [review] Bug 31698: Disallow moving waiting or intransit holds Signed-off-by: kristi <kkrueger@cuyahogalibrary.org> Re: Step 25, I do see in the hold log a modify entry. For example, if I created a hold on bib 20, and I move it to bib 8, the log shows the create entry with 'biblionumber'=20 and the modify entry with 'biblionumber=8 for the same object number. Re: Step 26, I think this was supposed to be set AllowHoldsOnDamagedItems to "don't allow" If this is correct, then the error message shows as expected (see attached screenshot). If it is indeed supposed to be a turn on, that is Allow for this system preference, there is no error message. One final thing, sometimes on moving record level holds, it shows the checkbox/"Move all selected record level holds..." message twice (see attached RecordLevel_ListTwice.PNG) I didn't think this was big enough not to sign off, but I wanted to call it out. Created attachment 182640 [details]
Error Message for Don't allow holds on damaged items
Created attachment 182641 [details]
Checkbox for record level moves sometimes shows twice
Bug 23269 is going to conflict with this one, I'll need to rebase this on top of that one. (In reply to Lucas Gass (lukeg) from comment #54) > Bug 23269 is going to conflict with this one, I'll need to rebase this on > top of that one. Well, maybe not. Bug 23269 has some problems and I don't want to wait. Created attachment 184750 [details] [review] Bug 31698: Ensure searches for biblio records return only unique bibs > Re: Step 26, I think this was supposed to be set AllowHoldsOnDamagedItems to > "don't allow" If this is correct, then the error message shows as expected > (see attached screenshot). If it is indeed supposed to be a turn on, that is > Allow for this system preference, there is no error message. Yes, this is a mistake in the test plan. > One final thing, sometimes on moving record level holds, it shows the > checkbox/"Move all selected record level holds..." message twice (see > attached RecordLevel_ListTwice.PNG) I didn't think this was big enough not > to sign off, but I wanted to call it out. I have added a patch that should fix this problem. Thanks for testing! Created attachment 185050 [details] [review] Bug 31698: Adjust number of tests Created attachment 185064 [details] [review] Bug 31698: Add alter_hold_targets permission Created attachment 185065 [details] [review] Bug 31698: Add ability to move holds to new items or new records Test plan: 1. APPLY PATCH, updatedatabase, restart_all 2. Have two staff members. 1. Superlibrarian, 2. Staff with new permission to `alter_hold_targets` 3. Now place some holds on different record, you'll want some item level holds and some record level holds. 4. After placing some holds go to a record with some holds on it. 5. Go to the holds tab (reserve/request.p) 6. You should notice a disabled 'Move selected (0)' button, next to 'Cancel selected (0)' 7. Select some holds to move, ensure that the number updates accordingly. ( Move selected (2) ) 8. When you click the button you should see 2 options, 'Item level holds to a different item' and 'Record level holds to a different record' 9. Clicking on either should bring up a modal with information about the holds you selected to move. 10. If you have item level holds and you select 'Item level holds to a different item' those holds should appear pre-checked. 11. If you have item level holds and you select 'Record level holds to a different record' those holds checkboxes should be disabled by default, you should see the 'Problem' as 'Cannot move a waiting or record level hold' 12. If you are moving a item level hold you should see a searchbar labeled 'Enter the item barcode of new hold target:'. 13. Enter a full or partial barcode and click search 14. You should see your item, if it exists. 15. Check 'Move all selected item level holds to this item', once you do so the yellow 'Move selected holds' button should be enabled. 16. Click 'Move selected holds', the hold should be moved. 17. You should see a message like " Moved the following holds: Hold ID: 1854 - Moved from The Arden edition of the works of William Shakespeare. to The Arden edition of the works of William Shakespeare." 19. Do steps 12 - 17 again, but with a record level hold. 20. Now make or have many holds on a specific target record to test priority. Make note of the current priority 21. Now move a hold on to that record, testing both item and record level holds. 22. Now on the target record review the priority. The holds originally on the record should not have changed. The hold you move should now have the lowest priority on the record. 23. Check the original record, if a hold was moved all other priority should have been adjusted correctly. FOr example, if the hold you moved was 3 of 4 then the others should have adjusted priority of 1 2 3 now. 24. Turn on HoldsLog if it is disabled. 25. Move some holds, both item and record ones, and review the logs after each one. There should be a correct MODIFY entry for each move. 26. Make an item with a hold on it as damaged and turn on AllowHoldsOnDamagedItems. Try to move the hold, you should get an error telling you that the target item is damaged. 27. Now login as the non-superlibrarian patron from step 2. If they have the `alter_hold_targets` permission they should see the 'Move selected' button. 28. Revoke the `alter_hold_targets` permission. The button should not appear now. Created attachment 185066 [details] [review] Bug 31698: Fix modals for item level moves Created attachment 185067 [details] [review] Bug 31698: Fix modals for biblio level moves Created attachment 185068 [details] [review] Bug 31698: Add unit tests Created attachment 185069 [details] [review] Bug 31698: Disallow moving waiting or intransit holds Signed-off-by: kristi <kkrueger@cuyahogalibrary.org> Created attachment 185070 [details] [review] Bug 31698: Ensure searches for biblio records return only unique bibs Hey all, nice feature! From my testing everything appears to be working nicely, but I have some comments: A) Step 6: The 'Move selected (0)' button is not disabled. It's styled differently to the 'Cancel selected (0)' button. It still expands options when clicked even if no holds are selected. My suggestion would be to style it the same as 'Cancel selected (0)' and not expand options if no holds are selected. Ideally using the 'disabled' CSS class but may be out of scope (?). B) Step 13: If you enter a partial barcode like '76', 20 hits are returned (I believe only limited by the default paging of 20 from the REST API). I don't know if this needs addressing or not, but I wonder if an exact match for barcode here makes more sense. C) Step 15: Clicking 'Move all selected item level holds to this item' does enable the 'Move selected holds' button and the checkbox column under the 'Review holds to move' is ticked by default. If you untick the latter, and keep the top 'Move all...' checkbox ticked, then click 'Move selected holds', the page redirects as if something happened, but no holds were moved (because none were selected, in the modal). My suggestion would be to have this button disabled if no 'holds to move' are ticked. D) Step 17: After moving the hold(s), it's nice that a message is shown 'Moved from X to Y', but the URL becomes '/cgi-bin/koha/reserve/request.pl'. This causes clicks on any of the links on the left side menu to result on a 404 (or 500) errors. My suggestion would be to have this URL '/cgi-bin/koha/reserve/request.pl?biblionumber=X' with the original record's biblionumber (the 'moved from' record). E) Step 19: Moving record level holds, the modal says 'Enter the biblionumber of new hold target', but if you enter 'xyz' and then search, the error shown is 'No item found with barcode: xyz'. This error message should refer to the biblionumber e.g. 'Biblionumber 'xyz' not found.' I think these are all very minor, with the exception perhaps of D) that I'd consider a bit more of a blocker. Created attachment 185121 [details] [review] Bug 31698: (QA follow-up): Query biblios endpoint when searching biblios Instead of filtering response on the client. Lucas I added a follow-up that implements your last patch differently, querying the biblios endpoint directly instead of using the items endpoint and then filtering on the client side. I've tested it myself to ensure it works, of course, but if my patch introduces issues or it's not worth it please feel free to obsolete! Created attachment 185379 [details] [review] Bug 31698: Style move selected button and disable when nothing is selected Created attachment 185380 [details] [review] Bug 31698: Make searches return only exact matches for barcode/biblionumber Created attachment 185607 [details] [review] Bug 31698: Add alter_hold_targets permission Created attachment 185608 [details] [review] Bug 31698: Add ability to move holds to new items or new records Test plan: 1. APPLY PATCH, updatedatabase, restart_all 2. Have two staff members. 1. Superlibrarian, 2. Staff with new permission to `alter_hold_targets` 3. Now place some holds on different record, you'll want some item level holds and some record level holds. 4. After placing some holds go to a record with some holds on it. 5. Go to the holds tab (reserve/request.p) 6. You should notice a disabled 'Move selected (0)' button, next to 'Cancel selected (0)' 7. Select some holds to move, ensure that the number updates accordingly. ( Move selected (2) ) 8. When you click the button you should see 2 options, 'Item level holds to a different item' and 'Record level holds to a different record' 9. Clicking on either should bring up a modal with information about the holds you selected to move. 10. If you have item level holds and you select 'Item level holds to a different item' those holds should appear pre-checked. 11. If you have item level holds and you select 'Record level holds to a different record' those holds checkboxes should be disabled by default, you should see the 'Problem' as 'Cannot move a waiting or record level hold' 12. If you are moving a item level hold you should see a searchbar labeled 'Enter the item barcode of new hold target:'. 13. Enter a full or partial barcode and click search 14. You should see your item, if it exists. 15. Check 'Move all selected item level holds to this item', once you do so the yellow 'Move selected holds' button should be enabled. 16. Click 'Move selected holds', the hold should be moved. 17. You should see a message like " Moved the following holds: Hold ID: 1854 - Moved from The Arden edition of the works of William Shakespeare. to The Arden edition of the works of William Shakespeare." 19. Do steps 12 - 17 again, but with a record level hold. 20. Now make or have many holds on a specific target record to test priority. Make note of the current priority 21. Now move a hold on to that record, testing both item and record level holds. 22. Now on the target record review the priority. The holds originally on the record should not have changed. The hold you move should now have the lowest priority on the record. 23. Check the original record, if a hold was moved all other priority should have been adjusted correctly. FOr example, if the hold you moved was 3 of 4 then the others should have adjusted priority of 1 2 3 now. 24. Turn on HoldsLog if it is disabled. 25. Move some holds, both item and record ones, and review the logs after each one. There should be a correct MODIFY entry for each move. 26. Make an item with a hold on it as damaged and turn on AllowHoldsOnDamagedItems. Try to move the hold, you should get an error telling you that the target item is damaged. 27. Now login as the non-superlibrarian patron from step 2. If they have the `alter_hold_targets` permission they should see the 'Move selected' button. 28. Revoke the `alter_hold_targets` permission. The button should not appear now. Created attachment 185609 [details] [review] Bug 31698: Fix modals for item level moves Created attachment 185610 [details] [review] Bug 31698: Fix modals for biblio level moves Created attachment 185611 [details] [review] Bug 31698: Add unit tests Created attachment 185612 [details] [review] Bug 31698: Disallow moving waiting or intransit holds Signed-off-by: kristi <kkrueger@cuyahogalibrary.org> Created attachment 185613 [details] [review] Bug 31698: Ensure searches for biblio records return only unique bibs Created attachment 185614 [details] [review] Bug 31698: (QA follow-up): Query biblios endpoint when searching biblios Instead of filtering response on the client. Created attachment 185615 [details] [review] Bug 31698: Style move selected button and disable when nothing is selected Created attachment 185616 [details] [review] Bug 31698: Make searches return only exact matches for barcode/biblionumber Created attachment 185753 [details] [review] Bug 31698: Add alter_hold_targets permission Created attachment 185754 [details] [review] Bug 31698: Add ability to move holds to new items or new records Test plan: 1. APPLY PATCH, updatedatabase, restart_all 2. Have two staff members. 1. Superlibrarian, 2. Staff with new permission to `alter_hold_targets` 3. Now place some holds on different record, you'll want some item level holds and some record level holds. 4. After placing some holds go to a record with some holds on it. 5. Go to the holds tab (reserve/request.p) 6. You should notice a disabled 'Move selected (0)' button, next to 'Cancel selected (0)' 7. Select some holds to move, ensure that the number updates accordingly. ( Move selected (2) ) 8. When you click the button you should see 2 options, 'Item level holds to a different item' and 'Record level holds to a different record' 9. Clicking on either should bring up a modal with information about the holds you selected to move. 10. If you have item level holds and you select 'Item level holds to a different item' those holds should appear pre-checked. 11. If you have item level holds and you select 'Record level holds to a different record' those holds checkboxes should be disabled by default, you should see the 'Problem' as 'Cannot move a waiting or record level hold' 12. If you are moving a item level hold you should see a searchbar labeled 'Enter the item barcode of new hold target:'. 13. Enter a full or partial barcode and click search 14. You should see your item, if it exists. 15. Check 'Move all selected item level holds to this item', once you do so the yellow 'Move selected holds' button should be enabled. 16. Click 'Move selected holds', the hold should be moved. 17. You should see a message like " Moved the following holds: Hold ID: 1854 - Moved from The Arden edition of the works of William Shakespeare. to The Arden edition of the works of William Shakespeare." 19. Do steps 12 - 17 again, but with a record level hold. 20. Now make or have many holds on a specific target record to test priority. Make note of the current priority 21. Now move a hold on to that record, testing both item and record level holds. 22. Now on the target record review the priority. The holds originally on the record should not have changed. The hold you move should now have the lowest priority on the record. 23. Check the original record, if a hold was moved all other priority should have been adjusted correctly. FOr example, if the hold you moved was 3 of 4 then the others should have adjusted priority of 1 2 3 now. 24. Turn on HoldsLog if it is disabled. 25. Move some holds, both item and record ones, and review the logs after each one. There should be a correct MODIFY entry for each move. 26. Make an item with a hold on it as damaged and turn on AllowHoldsOnDamagedItems. Try to move the hold, you should get an error telling you that the target item is damaged. 27. Now login as the non-superlibrarian patron from step 2. If they have the `alter_hold_targets` permission they should see the 'Move selected' button. 28. Revoke the `alter_hold_targets` permission. The button should not appear now. Created attachment 185755 [details] [review] Bug 31698: Fix modals for item level moves Created attachment 185756 [details] [review] Bug 31698: Fix modals for biblio level moves Created attachment 185757 [details] [review] Bug 31698: Add unit tests Created attachment 185758 [details] [review] Bug 31698: Disallow moving waiting or intransit holds Signed-off-by: kristi <kkrueger@cuyahogalibrary.org> Created attachment 185759 [details] [review] Bug 31698: Ensure searches for biblio records return only unique bibs Created attachment 185760 [details] [review] Bug 31698: (QA follow-up): Query biblios endpoint when searching biblios Instead of filtering response on the client. Created attachment 185761 [details] [review] Bug 31698: Style move selected button and disable when nothing is selected Created attachment 185762 [details] [review] Bug 31698: Make searches return only exact matches for barcode/biblionumber Created attachment 185763 [details] [review] Bug 31698: Move request.tt JS to hold.js Created attachment 185764 [details] [review] Bug 31698: Report errors and failures back to request.tt Created attachment 185765 [details] [review] Bug 31698: Add alter_hold_targets permission Created attachment 185766 [details] [review] Bug 31698: Add ability to move holds to new items or new records Test plan: 1. APPLY PATCH, updatedatabase, restart_all 2. Have two staff members. 1. Superlibrarian, 2. Staff with new permission to `alter_hold_targets` 3. Now place some holds on different record, you'll want some item level holds and some record level holds. 4. After placing some holds go to a record with some holds on it. 5. Go to the holds tab (reserve/request.p) 6. You should notice a disabled 'Move selected (0)' button, next to 'Cancel selected (0)' 7. Select some holds to move, ensure that the number updates accordingly. ( Move selected (2) ) 8. When you click the button you should see 2 options, 'Item level holds to a different item' and 'Record level holds to a different record' 9. Clicking on either should bring up a modal with information about the holds you selected to move. 10. If you have item level holds and you select 'Item level holds to a different item' those holds should appear pre-checked. 11. If you have item level holds and you select 'Record level holds to a different record' those holds checkboxes should be disabled by default, you should see the 'Problem' as 'Cannot move a waiting or record level hold' 12. If you are moving a item level hold you should see a searchbar labeled 'Enter the item barcode of new hold target:'. 13. Enter a full or partial barcode and click search 14. You should see your item, if it exists. 15. Check 'Move all selected item level holds to this item', once you do so the yellow 'Move selected holds' button should be enabled. 16. Click 'Move selected holds', the hold should be moved. 17. You should see a message like " Moved the following holds: Hold ID: 1854 - Moved from The Arden edition of the works of William Shakespeare. to The Arden edition of the works of William Shakespeare." 19. Do steps 12 - 17 again, but with a record level hold. 20. Now make or have many holds on a specific target record to test priority. Make note of the current priority 21. Now move a hold on to that record, testing both item and record level holds. 22. Now on the target record review the priority. The holds originally on the record should not have changed. The hold you move should now have the lowest priority on the record. 23. Check the original record, if a hold was moved all other priority should have been adjusted correctly. FOr example, if the hold you moved was 3 of 4 then the others should have adjusted priority of 1 2 3 now. 24. Turn on HoldsLog if it is disabled. 25. Move some holds, both item and record ones, and review the logs after each one. There should be a correct MODIFY entry for each move. 26. Make an item with a hold on it as damaged and turn on AllowHoldsOnDamagedItems. Try to move the hold, you should get an error telling you that the target item is damaged. 27. Now login as the non-superlibrarian patron from step 2. If they have the `alter_hold_targets` permission they should see the 'Move selected' button. 28. Revoke the `alter_hold_targets` permission. The button should not appear now. Created attachment 185767 [details] [review] Bug 31698: Fix modals for item level moves Created attachment 185768 [details] [review] Bug 31698: Fix modals for biblio level moves Created attachment 185769 [details] [review] Bug 31698: Add unit tests Created attachment 185770 [details] [review] Bug 31698: Disallow moving waiting or intransit holds Signed-off-by: kristi <kkrueger@cuyahogalibrary.org> Created attachment 185771 [details] [review] Bug 31698: Ensure searches for biblio records return only unique bibs Created attachment 185772 [details] [review] Bug 31698: (QA follow-up): Query biblios endpoint when searching biblios Instead of filtering response on the client. Created attachment 185773 [details] [review] Bug 31698: Style move selected button and disable when nothing is selected Created attachment 185774 [details] [review] Bug 31698: Make searches return only exact matches for barcode/biblionumber Created attachment 185775 [details] [review] Bug 31698: Report errors and failures back to request.tt Created attachment 185776 [details] [review] Bug 31698: Move request.tt JS to hold.js Created attachment 185834 [details] [review] Bug 31698: Move reamining JS to hold.js Created attachment 186085 [details] [review] Bug 31698: Add alter_hold_targets permission Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186086 [details] [review] Bug 31698: Add ability to move holds to new items or new records Test plan: 1. APPLY PATCH, updatedatabase, restart_all 2. Have two staff members. 1. Superlibrarian, 2. Staff with new permission to `alter_hold_targets` 3. Now place some holds on different record, you'll want some item level holds and some record level holds. 4. After placing some holds go to a record with some holds on it. 5. Go to the holds tab (reserve/request.p) 6. You should notice a disabled 'Move selected (0)' button, next to 'Cancel selected (0)' 7. Select some holds to move, ensure that the number updates accordingly. ( Move selected (2) ) 8. When you click the button you should see 2 options, 'Item level holds to a different item' and 'Record level holds to a different record' 9. Clicking on either should bring up a modal with information about the holds you selected to move. 10. If you have item level holds and you select 'Item level holds to a different item' those holds should appear pre-checked. 11. If you have item level holds and you select 'Record level holds to a different record' those holds checkboxes should be disabled by default, you should see the 'Problem' as 'Cannot move a waiting or record level hold' 12. If you are moving a item level hold you should see a searchbar labeled 'Enter the item barcode of new hold target:'. 13. Enter a full or partial barcode and click search 14. You should see your item, if it exists. 15. Check 'Move all selected item level holds to this item', once you do so the yellow 'Move selected holds' button should be enabled. 16. Click 'Move selected holds', the hold should be moved. 17. You should see a message like " Moved the following holds: Hold ID: 1854 - Moved from The Arden edition of the works of William Shakespeare. to The Arden edition of the works of William Shakespeare." 19. Do steps 12 - 17 again, but with a record level hold. 20. Now make or have many holds on a specific target record to test priority. Make note of the current priority 21. Now move a hold on to that record, testing both item and record level holds. 22. Now on the target record review the priority. The holds originally on the record should not have changed. The hold you move should now have the lowest priority on the record. 23. Check the original record, if a hold was moved all other priority should have been adjusted correctly. FOr example, if the hold you moved was 3 of 4 then the others should have adjusted priority of 1 2 3 now. 24. Turn on HoldsLog if it is disabled. 25. Move some holds, both item and record ones, and review the logs after each one. There should be a correct MODIFY entry for each move. 26. Make an item with a hold on it as damaged and turn on AllowHoldsOnDamagedItems. Try to move the hold, you should get an error telling you that the target item is damaged. 27. Now login as the non-superlibrarian patron from step 2. If they have the `alter_hold_targets` permission they should see the 'Move selected' button. 28. Revoke the `alter_hold_targets` permission. The button should not appear now. Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186087 [details] [review] Bug 31698: Fix modals for item level moves Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186088 [details] [review] Bug 31698: Fix modals for biblio level moves Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186089 [details] [review] Bug 31698: Add unit tests Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186090 [details] [review] Bug 31698: Disallow moving waiting or intransit holds Signed-off-by: kristi <kkrueger@cuyahogalibrary.org> Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186091 [details] [review] Bug 31698: Ensure searches for biblio records return only unique bibs Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186092 [details] [review] Bug 31698: (QA follow-up): Query biblios endpoint when searching biblios Instead of filtering response on the client. Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186093 [details] [review] Bug 31698: Style move selected button and disable when nothing is selected Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186094 [details] [review] Bug 31698: Make searches return only exact matches for barcode/biblionumber Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186095 [details] [review] Bug 31698: Report errors and failures back to request.tt Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186096 [details] [review] Bug 31698: Move request.tt JS to hold.js Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186097 [details] [review] Bug 31698: Move reamining JS to hold.js Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186098 [details] [review] Bug 31698: (QA follow-up): Prevent display 'NaN' UI error message If the input is not a number, display an appropriate error message. Additionally, don't make an API call unless the input is a number. Test by doing the following: 1) Step 19: Moving record level holds, the modal says 'Enter the biblionumber of new hold target', but if you enter 'xyz' and then search, the error shown is 'No item found with barcode: NaN'. This patch fixes that. Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Hi Lucas: 1) I've added a follow-up. Please check if you agree. 2) Passing QA as all my observations were addressed (thank you!) and QA script is happy, however 3) Can you please ensure that all the new code in holds.js is translatable e.g.: $("#itemResultMessage").html(` <div class="alert alert-warning">No item found with barcode: ${externalID}.</div> ); Created attachment 186105 [details] [review] Bug 31698: Make strings in holds.js translatable (In reply to Pedro Amorim from comment #120) > Hi Lucas: > 1) I've added a follow-up. Please check if you agree. > 2) Passing QA as all my observations were addressed (thank you!) and QA > script is happy, however > 3) Can you please ensure that all the new code in holds.js is translatable > e.g.: > > $("#itemResultMessage").html(` > <div class="alert alert-warning">No item found with barcode: > ${externalID}.</div> > ); I agree with your follow-up, thanks! I have also supplied a follow to make the string translatable. Created attachment 186115 [details] [review] Bug 31698: Make strings in holds.js translatable Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> Created attachment 186247 [details] [review] Bug 31698: Add alter_hold_targets to t/Koha/Auth/Permissions.t Nice work everyone! Pushed to main for 25.11 This is great news! So happy to see this. Any chance for backports on this? |