Description
Alex Buckley
2022-09-23 01:51:23 UTC
Created attachment 140878 [details] [review] Bug 31611: Clearly highlight items that cannot be modified/deleted in the Batch item tools Test plan: 1. Apply patches 2. Check out an item with a different home branch than the library you're logged in as 3. Go to: Tools > 'Batch item deletion' 4. Into the barcode list area paste the checked-out item's barcode and the barcode of a non-checked out item (with the same home library as the branch you're logged in as) and submit the form 5. Observe in the table that loads the row containing the checked out item is highlighted yellow, and it has a red cross in the first column. 6. Hover over the red cross to see the reason why you cannot delete the item appears 7. Observe the non checked out item has a checkbox and is not highlighted yellow 8. Click 'Select all' and 'Clear all' links and notice the display of the checked out item remains unchanged 9. Change 'IndependentBranches' syspref = 'yes' 10. Log into the staff client as a patron with only the '(catalogue)' and '(tools)' permissions selected 11. Go to: Tools > 'Batch item modification' 12. Enter the checked-out item's barcode and a non-checked out item's barcode and submit the form 13. Observe the table row for the checked out item is again highlighted yellow, with a red cross. 14. Hover over the red cross and observe the text 'Cannot edit' is shown 15. Observe the non-checked out item is not highlighted yellow and does have a checkbox Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 140879 [details] [review] Bug 31611: Clearly highlight biblios that cannot be deleted in the Batch record deletion tool Test plan: 1. Apply patch 2. Check out an item 3. Go to: Tools > 'Batch record deletion' 4. Into the 'Enter a list of record numbers' area paste in the biblionumber belonging to the checked-out item and that of a biblio without checked out items 5. Submit the form 6. Observe the row of the biblio with checked-out items is highlighted yellow, with a red cross in the first column 7. Hover over the red cross to see the reason why the biblio cannot be deleted 8. Observe the row of the biblio without checked-out items is not highlighted yellow, and does have a checkbox in the first column 9. Click 'Delete selected records' 10. Confirm the biblio without checked-out items is successfully deleted Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Ready for testing Created attachment 140880 [details]
Batch record deletion tool with the second patch applied clearly highlighting which record cannot be deleted
This is a really nice usability enhancement. Why did you remove "Clear on loan"? 2 small things: 1) QA test tools failure: FAIL koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc FAIL forbidden patterns forbidden pattern: tab char (line 283) 2) The removed bit Jonathan noted, maybe a rebase issue? - <a id="selectallbutton" href="#"><i class="fa fa-check"></i> Select all</a> | <a id="clearallbutton" href="#"><i class="fa fa-remove"></i> Clear all</a> | <a id="clearonloanbutton" href="#">Clear on loan</a> + <a id="selectallbutton" href="#"><i class="fa fa-check"></i> Select all</a> | <a id="clearallbutton" href="#"><i class="fa fa-remove"></i> Clear all</a> - $("#clearonloanbutton").click(function () { - $("#itemst input[name='itemnumber'][data-is-onloan='1']").each(function () { - $(this).prop('checked', false); - }); - return false; - }); Hm. testing this, I don't see the 'select on loan' filter without the patch either... So maybe this was just removing dead code? I think bringing back the option to remove all records from the list that have items checked out could still be useful to bring back. So maybe we should keep the code and file a new bug? Alex, could you have a look? (In reply to Katrin Fischer from comment #8) > Hm. testing this, I don't see the 'select on loan' filter without the patch > either... So maybe this was just removing dead code? > > I think bringing back the option to remove all records from the list that > have items checked out could still be useful to bring back. So maybe we > should keep the code and file a new bug? > > Alex, could you have a look? Hi Katrin, I am back from leave now, and will take a look at this. Thanks, Alex Created attachment 144888 [details] [review] Bug 31611: Clearly highlight items that cannot be modified/deleted in the Batch item tools Test plan: 1. Apply patches 2. Check out an item with a different home branch than the library you're logged in as 3. Go to: 'Cataloguing' > 'Batch item deletion' 4. Into the barcode list area paste the checked-out item's barcode and the barcode of a non-checked out item (with the same home library as the branch you're logged in as) and submit the form 5. Observe in the table that loads the row containing the checked out item is highlighted yellow, and it has a red cross in the first column. 6. Hover over the red cross to see the reason why you cannot delete the item appears 7. Observe the non checked out item has a checkbox and is not highlighted yellow 8. Click 'Select all' and 'Clear all' links and notice the display of the checked out item remains unchanged 9. Change 'IndependentBranches' syspref = 'yes' 10. Log into the staff client as a patron with only the '(catalogue)', '(editcatalogue)', and '(tools)' permissions selected 11. Go to: 'Cataloguing' > 'Batch item modification' 12. Enter the checked-out item's barcode and a non-checked out item's barcode and submit the form 13. Observe the table row for the checked out item is again highlighted yellow, with a red cross. 14. Hover over the red cross and observe the text 'Cannot edit' is shown 15. Observe the non-checked out item is not highlighted yellow and does have a checkbox Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 144889 [details] [review] Bug 31611: Clearly highlight biblios that cannot be deleted in the Batch record deletion tool Test plan: 1. Apply patch 2. Check out an item 3. Go to: Tools > 'Batch record deletion' 4. Into the 'Enter a list of record numbers' area paste in the biblionumber belonging to the checked-out item and that of a biblio without checked out items 5. Submit the form 6. Observe the row of the biblio with checked-out items is highlighted yellow, with a red cross in the first column 7. Hover over the red cross to see the reason why the biblio cannot be deleted 8. Observe the row of the biblio without checked-out items is not highlighted yellow, and does have a checkbox in the first column 9. Click 'Delete selected records' 10. Confirm the biblio without checked-out items is successfully deleted Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 144890 [details] [review] Bug 31611: Clearly highlight biblios that cannot be deleted in the Batch record deletion tool Test plan: 1. Apply patch 2. Check out an item 3. Go to: 'Cataloguing' > 'Batch record deletion' 4. Into the 'Enter a list of record numbers' area paste in the biblionumber belonging to the checked-out item and that of a biblio without checked out items 5. Submit the form 6. Observe the row of the biblio with checked-out items is highlighted yellow, with a red cross in the first column 7. Hover over the red cross to see the reason why the biblio cannot be deleted 8. Observe the row of the biblio without checked-out items is not highlighted yellow, and does have a checkbox in the first column 9. Click 'Delete selected records' 10. Confirm the biblio without checked-out items is successfully deleted Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Hi Jonathan and Katrin, Thank you for your responses. Jonathan: I originally removed the 'Clear on loan' link from the batch item modification and deletion pages for the reason outlined in the description: This patchset makes items on loan display a red cross, rather than a disabled checkbox. So giving users the ability to clear on loan items when they could not be selected in most cases seemed redundant. However, when the 'IndependentBranches' syspref is set to 'No' then on-loan items can be displayed with a selectable checkbox in the Batch item modification page so I have now re-added the 'Clear on loan' link to both batch item tool pages. --- Katrin: I've done some testing: The 'Clear on loan' link only appears on the batch item modification and deletion pages on master/main (and I have re-added it in my patches) - loaded from the html_helpers.inc. It does not appear on the batch record deletion page - as that uses links from batch_delete_records.tt ( https://github.com/Koha-Community/Koha/blob/master/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_delete_records.tt#L142 ). I do notice, however, the 'Clear on loan' link doesn't appear to work on the batch item modification page on master/main - so I'll make a new bug report for that. --- I've removed the tab char that QA test tools reported in comment #7, and have updated the test plans on both patches now batch item/record tools exist in the 'Cataloguing' module. Ready for testing again! Many thanks, Alex (In reply to Alex Buckley from comment #13) > Hi Jonathan and Katrin, > > Thank you for your responses. > > Jonathan: I originally removed the 'Clear on loan' link from the batch item > modification and deletion pages for the reason outlined in the description: > > This patchset makes items on loan display a red cross, rather than a > disabled checkbox. So giving users the ability to clear on loan items when > they could not be selected in most cases seemed redundant. > > However, when the 'IndependentBranches' syspref is set to 'No' then on-loan > items can be displayed with a selectable checkbox in the Batch item > modification page so I have now re-added the 'Clear on loan' link to both > batch item tool pages. > > --- > > Katrin: I've done some testing: The 'Clear on loan' link only appears on the > batch item modification and deletion pages on master/main (and I have > re-added it in my patches) - loaded from the html_helpers.inc. > > It does not appear on the batch record deletion page - as that uses links > from batch_delete_records.tt ( > https://github.com/Koha-Community/Koha/blob/master/koha-tmpl/intranet-tmpl/ > prog/en/modules/tools/batch_delete_records.tt#L142 ). > > I do notice, however, the 'Clear on loan' link doesn't appear to work on the > batch item modification page on master/main - so I'll make a new bug report > for that. > > --- > > I've removed the tab char that QA test tools reported in comment #7, and > have updated the test plans on both patches now batch item/record tools > exist in the 'Cataloguing' module. > > Ready for testing again! > > Many thanks, > Alex I've created a new bug report for the 'Clear on loan' link not working on the 'Batch item modification' page: bug 32550 Created attachment 144897 [details] [review] Bug 31611: Clearly highlight items that cannot be modified/deleted in the Batch item tools Test plan: 1. Apply patches 2. Check out an item with a different home branch than the library you're logged in as 3. Go to: 'Cataloguing' > 'Batch item deletion' 4. Into the barcode list area paste the checked-out item's barcode and the barcode of a non-checked out item (with the same home library as the branch you're logged in as) and submit the form 5. Observe in the table that loads the row containing the checked out item is highlighted yellow, and it has a red cross in the first column. 6. Hover over the red cross to see the reason why you cannot delete the item appears 7. Observe the non checked out item has a checkbox and is not highlighted yellow 8. Click 'Select all' and 'Clear all' links and notice the display of the checked out item remains unchanged 9. Change 'IndependentBranches' syspref = 'yes' 10. Log into the staff client as a patron with only the '(catalogue)', '(editcatalogue)', and '(tools)' permissions selected 11. Go to: 'Cataloguing' > 'Batch item modification' 12. Enter the checked-out item's barcode and a non-checked out item's barcode and submit the form 13. Observe the table row for the checked out item is again highlighted yellow, with a red cross. 14. Hover over the red cross and observe the text 'Cannot edit' is shown 15. Observe the non-checked out item is not highlighted yellow and does have a checkbox Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Created attachment 144898 [details] [review] Bug 31611: Clearly highlight biblios that cannot be deleted in the Batch record deletion tool Test plan: 1. Apply patch 2. Check out an item 3. Go to: 'Cataloguing' > 'Batch record deletion' 4. Into the 'Enter a list of record numbers' area paste in the biblionumber belonging to the checked-out item and that of a biblio without checked out items 5. Submit the form 6. Observe the row of the biblio with checked-out items is highlighted yellow, with a red cross in the first column 7. Hover over the red cross to see the reason why the biblio cannot be deleted 8. Observe the row of the biblio without checked-out items is not highlighted yellow, and does have a checkbox in the first column 9. Click 'Delete selected records' 10. Confirm the biblio without checked-out items is successfully deleted Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Moved release notes from the version(s) released in field to the text to go in the release notes field, and edited slightly... + // Highlight in yellow item rows that cannot be deleted + $(".error").parents('tr').find('td').css('background-color', '#ffff99'); Would normally expect this via css file, but similar actions are found already in the batch delete too. Created attachment 147286 [details] [review] Bug 31611: Clearly highlight items that cannot be modified/deleted in the Batch item tools Test plan: 1. Apply patches 2. Check out an item with a different home branch than the library you're logged in as 3. Go to: 'Cataloguing' > 'Batch item deletion' 4. Into the barcode list area paste the checked-out item's barcode and the barcode of a non-checked out item (with the same home library as the branch you're logged in as) and submit the form 5. Observe in the table that loads the row containing the checked out item is highlighted yellow, and it has a red cross in the first column. 6. Hover over the red cross to see the reason why you cannot delete the item appears 7. Observe the non checked out item has a checkbox and is not highlighted yellow 8. Click 'Select all' and 'Clear all' links and notice the display of the checked out item remains unchanged 9. Change 'IndependentBranches' syspref = 'yes' 10. Log into the staff client as a patron with only the '(catalogue)', '(editcatalogue)', and '(tools)' permissions selected 11. Go to: 'Cataloguing' > 'Batch item modification' 12. Enter the checked-out item's barcode and a non-checked out item's barcode and submit the form 13. Observe the table row for the checked out item is again highlighted yellow, with a red cross. 14. Hover over the red cross and observe the text 'Cannot edit' is shown 15. Observe the non-checked out item is not highlighted yellow and does have a checkbox Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 147287 [details] [review] Bug 31611: Clearly highlight biblios that cannot be deleted in the Batch record deletion tool Test plan: 1. Apply patch 2. Check out an item 3. Go to: 'Cataloguing' > 'Batch record deletion' 4. Into the 'Enter a list of record numbers' area paste in the biblionumber belonging to the checked-out item and that of a biblio without checked out items 5. Submit the form 6. Observe the row of the biblio with checked-out items is highlighted yellow, with a red cross in the first column 7. Hover over the red cross to see the reason why the biblio cannot be deleted 8. Observe the row of the biblio without checked-out items is not highlighted yellow, and does have a checkbox in the first column 9. Click 'Delete selected records' 10. Confirm the biblio without checked-out items is successfully deleted Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master for 23.05. Nice work everyone, thanks! Enhancement - not backporting to 22.11.x Nice work everyone! *** Bug 34089 has been marked as a duplicate of this bug. *** |