Patron card batches can be deleted manually, but it would also be helpful if the batches could be cleaned up automatically after X days.
*** Bug 24265 has been marked as a duplicate of this bug. ***
Created attachment 102934 [details] [review] Bug 15563: Add label batch deletion to cleanup_database To test: Apply patch 1- create a label batch with multiple items 2- update creator_batches to change timestamp on some items in batch 1 to more than 2 days ago 3- create a second label batch with multiple items 4- update creator_batches to change timestamp on all items in batch 2 to more than 2 days ago 5- create a patron card batch with multiple patrons 6- update creator_batches to change timestamp on some items to more than 2 days ago 7- run cron with --labels 2 8- confirm all labels with timestamp more than 2 days ago were deleted 9- confirm newer labels were not deleted 10- confirm batch 2 no longer shows in label creator 11- confirm no patrons card batches were changed
Deleting batches is complicated a bit by the table structure. There's no table for batches, it's just a column in the table that holds the individual items. So there's no timestamp for a given batch, just a bunch of timestamps for individual items. I've sidestepped that by deleting items rather than batches. If all items in a given batch are gone, the batch is gone. This does mean you could have a batch where items were added at different times and therefore part of the batch is deleted and part remains.
Created attachment 102948 [details] [review] Bug 15563: Add card creator batch deletion to cleanup_database To test: Apply patch 1- create a label batch with multiple items 2- update creator_batches to change timestamp on some items in batch 1 to more than 2 days ago 3- create a second label batch with multiple items 4- update creator_batches to change timestamp on all items in batch 2 to more than 2 days ago 5- create a patron card batch with multiple patrons 6- update creator_batches to change timestamp on some items to more than 2 days ago 7- run cron with --labels 2 8- confirm all labels with timestamp more than 2 days ago were deleted 9- confirm newer labels were not deleted 10- confirm batch 2 no longer shows in label creator 11- confirm no patrons card batches were changed 12- run cron with --cards 2 13- confirm all cards batches were deleted
Realized original bug also covered patron card creator. Added a separate option in cron to delete card creator patrons rather than label printed items.
Created attachment 103003 [details] [review] Bug 15563: Add label batch deletion to cleanup_database To test: Apply patch 1- create a label batch with multiple items 2- update creator_batches to change timestamp on some items in batch 1 to more than 2 days ago 3- create a second label batch with multiple items 4- update creator_batches to change timestamp on all items in batch 2 to more than 2 days ago 5- create a patron card batch with multiple patrons 6- update creator_batches to change timestamp on some items to more than 2 days ago 7- run cron with --labels 2 8- confirm all labels with timestamp more than 2 days ago were deleted 9- confirm newer labels were not deleted 10- confirm batch 2 no longer shows in label creator 11- confirm no patrons card batches were changed Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Created attachment 103004 [details] [review] Bug 15563: Add label and card creator batch deletion to cleanup_database To test: Apply patch 1- create a label batch with multiple items 2- update creator_batches to change timestamp on some items in batch 1 to more than 2 days ago 3- create a second label batch with multiple items 4- update creator_batches to change timestamp on all items in batch 2 to more than 2 days ago 5- create a patron card batch with multiple patrons 6- update creator_batches to change timestamp on some items to more than 2 days ago 7- run cron with --labels 2 8- confirm all labels with timestamp more than 2 days ago were deleted 9- confirm newer labels were not deleted 10- confirm batch 2 no longer shows in label creator 11- confirm no patrons card batches were changed 12- run cron with --cards 2 13- confirm all cards batches were deleted Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Created attachment 103011 [details] [review] Bug 15563: Add label batch deletion to cleanup_database To test: Apply patch 1- create a label batch with multiple items 2- update creator_batches to change timestamp on some items in batch 1 to more than 2 days ago 3- create a second label batch with multiple items 4- update creator_batches to change timestamp on all items in batch 2 to more than 2 days ago 5- create a patron card batch with multiple patrons 6- update creator_batches to change timestamp on some items to more than 2 days ago 7- run cron with --labels 2 8- confirm all labels with timestamp more than 2 days ago were deleted 9- confirm newer labels were not deleted 10- confirm batch 2 no longer shows in label creator 11- confirm no patrons card batches were changed Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Created attachment 103012 [details] [review] Bug 15563: Add label and card creator batch deletion to cleanup_database To test: Apply patch 1- create a label batch with multiple items 2- update creator_batches to change timestamp on some items in batch 1 to more than 2 days ago 3- create a second label batch with multiple items 4- update creator_batches to change timestamp on all items in batch 2 to more than 2 days ago 5- create a patron card batch with multiple patrons 6- update creator_batches to change timestamp on some items to more than 2 days ago 7- run cron with --labels 2 8- confirm all labels with timestamp more than 2 days ago were deleted 9- confirm newer labels were not deleted 10- confirm batch 2 no longer shows in label creator 11- confirm no patrons card batches were changed 12- run cron with --cards 2 13- confirm all cards batches were deleted Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Trivial change on that last update -- realized I'd forgotten to change the verbose wording when copy/pasting to make the card creator function so it still said "label items."
Created attachment 103492 [details] [review] Bug 15563: Add label batch deletion to cleanup_database To test: Apply patch 1- create a label batch with multiple items 2- update creator_batches to change timestamp on some items in batch 1 to more than 2 days ago 3- create a second label batch with multiple items 4- update creator_batches to change timestamp on all items in batch 2 to more than 2 days ago 5- create a patron card batch with multiple patrons 6- update creator_batches to change timestamp on some items to more than 2 days ago 7- run cron with --labels 2 8- confirm all labels with timestamp more than 2 days ago were deleted 9- confirm newer labels were not deleted 10- confirm batch 2 no longer shows in label creator 11- confirm no patrons card batches were changed Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Deb Stephenson <dstephen@dubuque.lib.ia.us>
Created attachment 103493 [details] [review] Bug 15563: Add label and card creator batch deletion to cleanup_database To test: Apply patch 1- create a label batch with multiple items 2- update creator_batches to change timestamp on some items in batch 1 to more than 2 days ago 3- create a second label batch with multiple items 4- update creator_batches to change timestamp on all items in batch 2 to more than 2 days ago 5- create a patron card batch with multiple patrons 6- update creator_batches to change timestamp on some items to more than 2 days ago 7- run cron with --labels 2 8- confirm all labels with timestamp more than 2 days ago were deleted 9- confirm newer labels were not deleted 10- confirm batch 2 no longer shows in label creator 11- confirm no patrons card batches were changed 12- run cron with --cards 2 13- confirm all cards batches were deleted Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Deb Stephenson <dstephen@dubuque.lib.ia.us>
Hi Andrew, (In reply to Andrew Fuerste-Henry from comment #3) > Deleting batches is complicated a bit by the table structure. There's no > table for batches, it's just a column in the table that holds the individual > items. So there's no timestamp for a given batch, just a bunch of timestamps > for individual items. I've sidestepped that by deleting items rather than > batches. If all items in a given batch are gone, the batch is gone. This > does mean you could have a batch where items were added at different times > and therefore part of the batch is deleted and part remains. I understand the issue - that's complicating things. May I make a suggestion for a slightly different approach? We could delete batches that have not been changed in x days instead, by using something like: SELECT batch_id from creator_batches WHERE MAX(timestamp) ... (older than x days) GROUP BY batch_id; And then DELETE all the entries with the batch_ids of our result set. What do you think? I am feeling a bit uneasy about the possibility that the batches will keep 'changing' in the GUI in number and what's in them. It would feel a bit more consistent to me to delete the whole thing or not. Otherwise I had a bit of trouble understanding the wording of the new options. Could you maybe try and clear that up a bit?
Created attachment 107115 [details] [review] Bug 15563: add label and card creator batch deletion to cleanup_database To test: For item label batches: 1- Create 3 item label batches of at least 2 items each 2- Perform the following updates via the database 3- In Batch 1, set all items' timestamps to 10+ days prior to today 4- In Batch 2, set 1 item's timestamp to 10+ days prior to today 5- In Batch 3, do not update any timestamps 6- Run cron with --labels 9 7- Confirm batch 1 is deleted, batches 2 and 3 are not Repeat with card creator batches 8- Create 3 card creator batches of at least 2 items each 9- Perform the following updates via the database 10- In Batch 1, set all cards' timestamps to 10+ days prior to today 11- In Batch 2, set 1 card's timestamp to 10+ days prior to today 12- In Batch 3, do not update any timestamps 13- Run cron with --cards 9 14- Confirm batch 1 is deleted, batches 2 and 3 are noti
Updated to delete entire batches based on max(timestamp), as suggested. This took a little bit of SQL sleight-of-hand as SQL doesn't like a subquery including the table you're updating. Katrin, do you have any specific feedback about wording? I changed it a bit, please let me know if it's still unclear.
Hi Andrew, the wording reads ok to me - could you please rebase?
Created attachment 107639 [details] [review] Bug 15563: add label and card creator batch deletion to cleanup_database To test: For item label batches: 1- Create 3 item label batches of at least 2 items each 2- Perform the following updates via the database 3- In Batch 1, set all items' timestamps to 10+ days prior to today 4- In Batch 2, set 1 item's timestamp to 10+ days prior to today 5- In Batch 3, do not update any timestamps 6- Run cron with --labels 9 7- Confirm batch 1 is deleted, batches 2 and 3 are not Repeat with card creator batches 8- Create 3 card creator batches of at least 2 items each 9- Perform the following updates via the database 10- In Batch 1, set all cards' timestamps to 10+ days prior to today 11- In Batch 2, set 1 card's timestamp to 10+ days prior to today 12- In Batch 3, do not update any timestamps 13- Run cron with --cards 9 14- Confirm batch 1 is deleted, batches 2 and 3 are noti
Created attachment 118076 [details] [review] Bug 15563: add option to cleanup_database to delete old label and card batches To test: For item label batches: 1- Create 3 item label batches of at least 2 items each 2- Perform the following updates via the database 3- In Batch 1, set all items' timestamps to 10+ days prior to today 4- In Batch 2, set 1 item's timestamp to 10+ days prior to today 5- In Batch 3, do not update any timestamps 6- Run cron with --labels 9 7- Confirm batch 1 is deleted, batches 2 and 3 are not Repeat with card creator batches 8- Create 3 card creator batches of at least 2 items each 9- Perform the following updates via the database 10- In Batch 1, set all cards' timestamps to 10+ days prior to today 11- In Batch 2, set 1 card's timestamp to 10+ days prior to today 12- In Batch 3, do not update any timestamps 13- Run cron with --cards 9 14- Confirm batch 1 is deleted, batches 2 and 3 are not
Updated patch to make sure it still applies and works
Comment on attachment 107639 [details] [review] Bug 15563: add label and card creator batch deletion to cleanup_database >From cb0cdb4c97783566339d080be1a6263bad4caf97 Mon Sep 17 00:00:00 2001 >From: Andrew Fuerste-Henry <andrew@bywatersolutions.com> >Date: Mon, 20 Jul 2020 15:09:30 +0000 >Subject: [PATCH] Bug 15563: add label and card creator batch deletion to > cleanup_database > >To test: >For item label batches: >1- Create 3 item label batches of at least 2 items each >2- Perform the following updates via the database >3- In Batch 1, set all items' timestamps to 10+ days prior to today >4- In Batch 2, set 1 item's timestamp to 10+ days prior to today >5- In Batch 3, do not update any timestamps >6- Run cron with --labels 9 >7- Confirm batch 1 is deleted, batches 2 and 3 are not > >Repeat with card creator batches >8- Create 3 card creator batches of at least 2 items each >9- Perform the following updates via the database >10- In Batch 1, set all cards' timestamps to 10+ days prior to today >11- In Batch 2, set 1 card's timestamp to 10+ days prior to today >12- In Batch 3, do not update any timestamps >13- Run cron with --cards 9 >14- Confirm batch 1 is deleted, batches 2 and 3 are noti >--- > misc/cronjobs/cleanup_database.pl | 47 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > >diff --git a/misc/cronjobs/cleanup_database.pl b/misc/cronjobs/cleanup_database.pl >index c2d8e9832c..d59fa3a368 100755 >--- a/misc/cronjobs/cleanup_database.pl >+++ b/misc/cronjobs/cleanup_database.pl >@@ -103,6 +103,9 @@ Usage: $0 [-h|--help] [--confirm] [--sessions] [--sessdays DAYS] [-v|--verbose] > --pseudo-transactions DAYS Purge the pseudonymized transactions that have been originally created more than DAYS days ago > DAYS is optional and can be replaced by: > --pseudo-transactions-from YYYY-MM-DD and/or --pseudo-transactions-to YYYY-MM-DD >+ --labels DAYS Purge item label batches last added to more than DAYS days ago. >+ --cards DAY Purge card creator batches last added to more than DAYS days ago. >+ > USAGE > exit $_[0]; > } >@@ -577,6 +580,50 @@ if (defined $pPseudoTransactions or $pPseudoTransactionsFrom or $pPseudoTransact > ? sprintf "Done with purging %d pseudonymized transactions.", $count > : sprintf "%d pseudonymized transactions would have been purged.", $count; > } >+ $sth = $dbh->prepare( >+ q{ >+ DELETE FROM branchtransfers >+ WHERE datearrived < DATE_SUB(CURDATE(), INTERVAL ? DAY) >+ } >+ ); >+ $sth->execute($pTransfers); >+ print "Done with purging transfers.\n" if $verbose; >+} >+ >+if ($labels) { >+ print "Purging item label batches last added to more than $labels days ago.\n" if $verbose; >+ $sth = $dbh->prepare( >+ q{ >+ DELETE from creator_batches >+ WHERE batch_id in >+ (SELECT batch_id >+ FROM (SELECT batch_id >+ FROM creator_batches >+ WHERE creator='labels' >+ GROUP BY batch_id >+ HAVING max(timestamp) < date_sub(curdate(),interval ? day)) a) >+ } >+ ); >+ $sth->execute($labels) or die $dbh->errstr; >+ print "Done with purging item label batches last added to more than $labels days ago.\n" if $verbose; >+} >+ >+if ($cards) { >+ print "Purging card creator batches last added to more than $cards days ago.\n" if $verbose; >+ $sth = $dbh->prepare( >+ q{ >+ DELETE from creator_batches >+ WHERE batch_id in >+ (SELECT batch_id >+ FROM (SELECT batch_id >+ FROM creator_batches >+ WHERE creator='patroncards' >+ GROUP BY batch_id >+ HAVING max(timestamp) < date_sub(curdate(),interval ? day)) a); >+ } >+ ); >+ $sth->execute($cards) or die $dbh->errstr; >+ print "Done with purging card creator batches last added to more than $cards days ago.\n" if $verbose; > } > > exit(0); >-- >2.11.0
Created attachment 118078 [details] [review] Bug 15563: add option to cleanup_database to delete old label and card batches To test: For item label batches: 1- Create 3 item label batches of at least 2 items each 2- Perform the following updates via the database 3- In Batch 1, set all items' timestamps to 10+ days prior to today 4- In Batch 2, set 1 item's timestamp to 10+ days prior to today 5- In Batch 3, do not update any timestamps 6- Run cron with --labels 9 7- Confirm batch 1 is deleted, batches 2 and 3 are not Repeat with card creator batches 8- Create 3 card creator batches of at least 2 items each 9- Perform the following updates via the database 10- In Batch 1, set all cards' timestamps to 10+ days prior to today 11- In Batch 2, set 1 card's timestamp to 10+ days prior to today 12- In Batch 3, do not update any timestamps 13- Run cron with --cards 9 14- Confirm batch 1 is deleted, batches 2 and 3 are not https://bugs.koha-community.org/show_bug.cgi?id=15569
Created attachment 118081 [details] [review] Bug 15563: add label and card creator batch deletion to cleanup_database To test: For item label batches: 1- Create 3 item label batches of at least 2 items each 2- Perform the following updates via the database 3- In Batch 1, set all items' timestamps to 10+ days prior to today 4- In Batch 2, set 1 item's timestamp to 10+ days prior to today 5- In Batch 3, do not update any timestamps 6- Run cron with --labels 9 7- Confirm batch 1 is deleted, batches 2 and 3 are not Repeat with card creator batches 8- Create 3 card creator batches of at least 2 items each 9- Perform the following updates via the database 10- In Batch 1, set all cards' timestamps to 10+ days prior to today 11- In Batch 2, set 1 card's timestamp to 10+ days prior to today 12- In Batch 3, do not update any timestamps 13- Run cron with --cards 9 14- Confirm batch 1 is deleted, batches 2 and 3 are noti Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Created attachment 118082 [details] [review] Bug 15563: add option to cleanup_database to delete old label and card batches To test: For item label batches: 1- Create 3 item label batches of at least 2 items each 2- Perform the following updates via the database 3- In Batch 1, set all items' timestamps to 10+ days prior to today 4- In Batch 2, set 1 item's timestamp to 10+ days prior to today 5- In Batch 3, do not update any timestamps 6- Run cron with --labels 9 7- Confirm batch 1 is deleted, batches 2 and 3 are not Repeat with card creator batches 8- Create 3 card creator batches of at least 2 items each 9- Perform the following updates via the database 10- In Batch 1, set all cards' timestamps to 10+ days prior to today 11- In Batch 2, set 1 card's timestamp to 10+ days prior to today 12- In Batch 3, do not update any timestamps 13- Run cron with --cards 9 14- Confirm batch 1 is deleted, batches 2 and 3 are not Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Andrew, I think maybe some old code for deleting branch transfers made it into your send patch. Can you double check that?
Created attachment 118345 [details] [review] Bug 15563: add label and card creator batch deletion to cleanup_database To test: For item label batches: 1- Create 3 item label batches of at least 2 items each 2- Perform the following updates via the database 3- In Batch 1, set all items' timestamps to 10+ days prior to today 4- In Batch 2, set 1 item's timestamp to 10+ days prior to today 5- In Batch 3, do not update any timestamps 6- Run cron with --labels 9 7- Confirm batch 1 is deleted, batches 2 and 3 are not Repeat with card creator batches 8- Create 3 card creator batches of at least 2 items each 9- Perform the following updates via the database 10- In Batch 1, set all cards' timestamps to 10+ days prior to today 11- In Batch 2, set 1 card's timestamp to 10+ days prior to today 12- In Batch 3, do not update any timestamps 13- Run cron with --cards 9 14- Confirm batch 1 is deleted, batches 2 and 3 are noti Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Created attachment 118762 [details] [review] Bug 15563: add label and card creator batch deletion to cleanup_database To test: For item label batches: 1- Create 3 item label batches of at least 2 items each 2- Perform the following updates via the database 3- In Batch 1, set all items' timestamps to 10+ days prior to today 4- In Batch 2, set 1 item's timestamp to 10+ days prior to today 5- In Batch 3, do not update any timestamps 6- Run cron with --labels 9 7- Confirm batch 1 is deleted, batches 2 and 3 are not Repeat with card creator batches 8- Create 3 card creator batches of at least 2 items each 9- Perform the following updates via the database 10- In Batch 1, set all cards' timestamps to 10+ days prior to today 11- In Batch 2, set 1 card's timestamp to 10+ days prior to today 12- In Batch 3, do not update any timestamps 13- Run cron with --cards 9 14- Confirm batch 1 is deleted, batches 2 and 3 are noti Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118763 [details] [review] Bug 15563: (QA follow-up) Add missing character The script wasn't compiling; very simple mistake to make. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118764 [details] [review] Bug 15563: add label and card creator batch deletion to cleanup_database To test: For item label batches: 1- Create 3 item label batches of at least 2 items each 2- Perform the following updates via the database 3- In Batch 1, set all items' timestamps to 10+ days prior to today 4- In Batch 2, set 1 item's timestamp to 10+ days prior to today 5- In Batch 3, do not update any timestamps 6- Run cron with --labels 9 7- Confirm batch 1 is deleted, batches 2 and 3 are not Repeat with card creator batches 8- Create 3 card creator batches of at least 2 items each 9- Perform the following updates via the database 10- In Batch 1, set all cards' timestamps to 10+ days prior to today 11- In Batch 2, set 1 card's timestamp to 10+ days prior to today 12- In Batch 3, do not update any timestamps 13- Run cron with --cards 9 14- Confirm batch 1 is deleted, batches 2 and 3 are noti Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com> Signed-off-by: Deb Stephenson <dstephen@dubuque.lib.ia.us> Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I squashed a rebase error fix into the patch.. there was a simple missing }. Works as expected, no regressions found. Passing QA
The "confirm" flag needs to be taken into account. Also it would be good if you could use the filter_by_last_update methods inherited from Koha::Objects.
Created attachment 120082 [details] [review] Bug 15563: (follow-up) Combine methods , add count, obey confirm This patch moves the SQL to a subroutine and uses the Label delete method to remove the batches We now return a count and do not delete if --confirm flag is not passed Repeate previous test plan, confirm works as expected
(In reply to Jonathan Druart from comment #30) > The "confirm" flag needs to be taken into account. Also it would be good if > you could use the filter_by_last_update methods inherited from Koha::Objects. This would require adding Objects for the batches, correct? I think that should be its own bug
Created attachment 120221 [details] [review] Bug 15563: (follow-up) Combine methods, add count, obey confirm This patch moves the SQL to a subroutine and uses the Label delete method to remove the batches We now return a count and do not delete if --confirm flag is not passed Repeate previous test plan, confirm works as expected JD amended patch: perltidy
Pushed to master for 21.05, thanks to everybody involved!
Enhancement not pushed to 20.11.x