Apologies if there's a duplicate bug. It would be great to add a parameter to delete_items.pl to delete the record if the last item getting deleted is the only one that remains.
Sorry, what steps do you take to get to delete_items.pl?
It's a CLI script in misc/cronjobs :)
Would this be optional or be coded to always happen? We would definitely want to choose when to delete the record or not.
Hi Angela, I believe that this was filed with the idea to make it an option for the CLI script, so you'd add it or not as needed, similar to how you can check the checkbox in the GUI or leave it off.
Created attachment 174480 [details] [review] Bug 35654: Add option to delete_items.pl cronjob to delete empty bibs This patch adds a new option to the delete_items cronjob When --del_bibs is set, bib records will be deleted if the last item is deleted. The following plan assumes default ktd data. To test: 1. Apply patch 2. Check the holdings for bib #115 and see 3 items http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=115 2. Run the cron with the following flags perl ./misc/cronjobs/delete_items.pl --where "biblionumber=115 and homebranch ='MPL'" --verbose --del_bibs --commit 3. Notice in the output the count of current items on the bib record: Where statement: where biblionumber=115 and homebranch='MPL' Deleting empty bib records! Deleting item '237' (of 3) Deleting item '238' (of 2) 4. Confirm the two Midway items were deleted from bib #115, and the bib still exists with one item 4. Run the cron again but change the the --where flag to delete all the items from bib #115 perl ./misc/cronjobs/delete_items.pl --where "biblionumber=115" --verbose --del_bibs --commit 5. Notice in the output the bib is deleted after the last item is deleted Where statement: where biblionumber=115 Deleting empty bib records! Deleting item '239' (of 1) Last item deleted - deleting bib '115' 8. Confirm the bib #115 was deleted 9. Add some more items and test with some other --where parameters 10. Test with and without --commit and --del_bibs, test the --help flag Sponsored-by: CLAMS
Created attachment 174529 [details] [review] Bug 35654: Add option to delete_items.pl cronjob to delete empty bibs This patch adds a new option to the delete_items cronjob When --del_bibs is set, bib records will be deleted if the last item is deleted. The following plan assumes default ktd data. To test: 1. Apply patch 2. Check the holdings for bib #115 and see 3 items http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=115 2. Run the cron with the following flags perl ./misc/cronjobs/delete_items.pl --where "biblionumber=115 and homebranch ='MPL'" --verbose --del_bibs --commit 3. Notice in the output the count of current items on the bib record: Where statement: where biblionumber=115 and homebranch='MPL' Deleting empty bib records! Deleting item '237' (of 3) Deleting item '238' (of 2) 4. Confirm the two Midway items were deleted from bib #115, and the bib still exists with one item 4. Run the cron again but change the the --where flag to delete all the items from bib #115 perl ./misc/cronjobs/delete_items.pl --where "biblionumber=115" --verbose --del_bibs --commit 5. Notice in the output the bib is deleted after the last item is deleted Where statement: where biblionumber=115 Deleting empty bib records! Deleting item '239' (of 1) Last item deleted - deleting bib '115' 8. Confirm the bib #115 was deleted 9. Add some more items and test with some other --where parameters 10. Test with and without --commit and --del_bibs, test the --help flag Sponsored-by: CLAMS Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Created attachment 175193 [details] [review] Bug 35654: (follow-up) Simplify variable use and only delete when commit is set This patch leans up some things: - version option does nothing, so removed - warn when not setting commit and show conditional messages - remove use of Koha::Biblios and use item object methods
Created attachment 175194 [details] [review] Bug 35654: Simplify and update delete_items.pl code Rather than defining hashes and storing our options there, we can just use variables directly, improves readability IMO
Created attachment 175195 [details] [review] Bug 35654: Use Koha::Items instead of direct SQL
Created attachment 175208 [details] [review] Bug 35654: (follow-up) Remove unneeded code and comment This patch removes old code that was used for direct SQL
Created attachment 175209 [details] [review] Bug 35654: (follow-up) Fix $holdings_count Fix the off by one error in the holdings_count to accurately print the number of remaining items.
Is "Signed off" still the right status for this bug? It looks like 5 follow-ups have been added since it was signed off, some of which contain refactoring that seems out of scope to me... I should be able to QA this if Nick isn't planning on it, but I'd want another signoff and ideally some commit cleanup first (or clarity on why it isn't needed).
Thanks for looking at this Emily! I think I should have probably changed the status back to needs sign off when I added the last two patches. I'm not sure how to best clean up the commits because some are from me and some from Nick. The commit messages describe the changes they are making and make sense to me, but I understand how they could be confusing and it would be clearer to look at this as a whole if they were squashed.
Created attachment 176936 [details] [review] Bug 35654: Add option to delete_items.pl cronjob to delete empty bibs This patch adds a new option to the delete_items cronjob When --del_bibs is set, bib records will be deleted if the last item is deleted. The following plan assumes default ktd data. To test: 1. Apply patch 2. Check the holdings for bib #115 and see 3 items http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=115 2. Run the cron with the following flags perl ./misc/cronjobs/delete_items.pl --where "biblionumber=115 and homebranch ='MPL'" --verbose --del_bibs --commit 3. Notice in the output the count of current items on the bib record: Where statement: where biblionumber=115 and homebranch='MPL' Deleting empty bib records! Deleting item '237' (of 3) Deleting item '238' (of 2) 4. Confirm the two Midway items were deleted from bib #115, and the bib still exists with one item 4. Run the cron again but change the the --where flag to delete all the items from bib #115 perl ./misc/cronjobs/delete_items.pl --where "biblionumber=115" --verbose --del_bibs --commit 5. Notice in the output the bib is deleted after the last item is deleted Where statement: where biblionumber=115 Deleting empty bib records! Deleting item '239' (of 1) Last item deleted - deleting bib '115' 8. Confirm the bib #115 was deleted 9. Add some more items and test with some other --where parameters 10. Test with and without --commit and --del_bibs, test the --help flag Sponsored-by: CLAMS
I squashed everything into one commit and removed Andrew's sign off line. Thanks for your help Nick!
Created attachment 176937 [details] [review] Bug 35654: Add option to delete_items.pl cronjob to delete empty bibs This patch adds a new option to the delete_items cronjob When --del_bibs is set, bib records will be deleted if the last item is deleted. The following plan assumes default ktd data. To test: 1. Apply patch 2. Check the holdings for bib #115 and see 3 items http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=115 2. Run the cron with the following flags perl ./misc/cronjobs/delete_items.pl --where "biblionumber=115 and homebranch ='MPL'" --verbose --del_bibs --commit 3. Notice in the output the count of current items on the bib record: Where statement: where biblionumber=115 and homebranch='MPL' Deleting empty bib records! Deleting item '237' (of 3) Deleting item '238' (of 2) 4. Confirm the two Midway items were deleted from bib #115, and the bib still exists with one item 4. Run the cron again but change the the --where flag to delete all the items from bib #115 perl ./misc/cronjobs/delete_items.pl --where "biblionumber=115" --verbose --del_bibs --commit 5. Notice in the output the bib is deleted after the last item is deleted Where statement: where biblionumber=115 Deleting empty bib records! Deleting item '239' (of 1) Last item deleted - deleting bib '115' 8. Confirm the bib #115 was deleted 9. Add some more items and test with some other --where parameters 10. Test with and without --commit and --del_bibs, test the --help flag Sponsored-by: CLAMS Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Created attachment 177868 [details] [review] Bug 35654: Add option to delete_items.pl cronjob to delete empty bibs This patch adds a new option to the delete_items cronjob When --del_bibs is set, bib records will be deleted if the last item is deleted. The following plan assumes default ktd data. To test: 1. Apply patch 2. Check the holdings for bib #115 and see 3 items http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=115 2. Run the cron with the following flags perl ./misc/cronjobs/delete_items.pl --where "biblionumber=115 and homebranch ='MPL'" --verbose --del_bibs --commit 3. Notice in the output the count of current items on the bib record: Where statement: where biblionumber=115 and homebranch='MPL' Deleting empty bib records! Deleting item '237' (of 3) Deleting item '238' (of 2) 4. Confirm the two Midway items were deleted from bib #115, and the bib still exists with one item 4. Run the cron again but change the the --where flag to delete all the items from bib #115 perl ./misc/cronjobs/delete_items.pl --where "biblionumber=115" --verbose --del_bibs --commit 5. Notice in the output the bib is deleted after the last item is deleted Where statement: where biblionumber=115 Deleting empty bib records! Deleting item '239' (of 1) Last item deleted - deleting bib '115' 8. Confirm the bib #115 was deleted 9. Add some more items and test with some other --where parameters 10. Test with and without --commit and --del_bibs, test the --help flag Sponsored-by: CLAMS Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Patch rebased using the auto-rebase script.
FAIL misc/cronjobs/delete_items.pl FAIL critic # Variables::ProhibitConditionalDeclarations: Got 1 violation(s).
Created attachment 179675 [details] [review] Bug 35654: Add option to delete_items.pl cronjob to delete empty bibs This patch adds a new option to the delete_items cronjob When --del_bibs is set, bib records will be deleted if the last item is deleted. The following plan assumes default ktd data. To test: 1. Apply patch 2. Check the holdings for bib #115 and see 3 items http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=115 2. Run the cron with the following flags perl ./misc/cronjobs/delete_items.pl --where "biblionumber=115 and homebranch ='MPL'" --verbose --del_bibs --commit 3. Notice in the output the count of current items on the bib record: Where statement: where biblionumber=115 and homebranch='MPL' Deleting empty bib records! Deleting item '237' (of 3) Deleting item '238' (of 2) 4. Confirm the two Midway items were deleted from bib #115, and the bib still exists with one item 4. Run the cron again but change the the --where flag to delete all the items from bib #115 perl ./misc/cronjobs/delete_items.pl --where "biblionumber=115" --verbose --del_bibs --commit 5. Notice in the output the bib is deleted after the last item is deleted Where statement: where biblionumber=115 Deleting empty bib records! Deleting item '239' (of 1) Last item deleted - deleting bib '115' 8. Confirm the bib #115 was deleted 9. Add some more items and test with some other --where parameters 10. Test with and without --commit and --del_bibs, test the --help flag Sponsored-by: CLAMS Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Thanks Marcel, I fixed that QA violation