Bug 15563 - Add option to regularly delete patroncard and label batches to cleanup_database.pl cronjob
Summary: Add option to regularly delete patroncard and label batches to cleanup_databa...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Label/patron card printing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Andrew Fuerste-Henry
QA Contact: Testopia
URL:
Keywords:
: 24265 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-01-13 08:55 UTC by Katrin Fischer
Modified: 2022-06-06 20:29 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds new options to the cleanup_database cronjob to delete label and card creator batches that haven't been updated for a given number of days. Previously, all batches were kept indefinitely.
Version(s) released in:
21.05.00


Attachments
Bug 15563: Add label batch deletion to cleanup_database (2.80 KB, patch)
2020-04-14 15:33 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: Add card creator batch deletion to cleanup_database (2.99 KB, patch)
2020-04-14 21:15 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: Add label batch deletion to cleanup_database (2.86 KB, patch)
2020-04-15 13:49 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 15563: Add label and card creator batch deletion to cleanup_database (3.05 KB, patch)
2020-04-15 13:49 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 15563: Add label batch deletion to cleanup_database (2.86 KB, patch)
2020-04-15 14:09 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: Add label and card creator batch deletion to cleanup_database (3.07 KB, patch)
2020-04-15 14:09 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: Add label batch deletion to cleanup_database (2.92 KB, patch)
2020-04-22 16:19 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: Add label and card creator batch deletion to cleanup_database (3.13 KB, patch)
2020-04-22 16:19 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: add label and card creator batch deletion to cleanup_database (4.17 KB, patch)
2020-07-20 15:10 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: add label and card creator batch deletion to cleanup_database (3.83 KB, patch)
2020-07-31 12:41 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: add option to cleanup_database to delete old label and card batches (3.21 KB, patch)
2021-03-10 17:34 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: add option to cleanup_database to delete old label and card batches (2.00 KB, patch)
2021-03-10 18:03 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: add label and card creator batch deletion to cleanup_database (3.88 KB, patch)
2021-03-10 22:02 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: add option to cleanup_database to delete old label and card batches (3.26 KB, patch)
2021-03-10 22:02 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: add label and card creator batch deletion to cleanup_database (5.50 KB, patch)
2021-03-16 19:15 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 15563: add label and card creator batch deletion to cleanup_database (5.56 KB, patch)
2021-03-24 13:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15563: (QA follow-up) Add missing character (1.01 KB, patch)
2021-03-24 13:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15563: add label and card creator batch deletion to cleanup_database (5.54 KB, patch)
2021-03-24 13:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15563: (follow-up) Combine methods , add count, obey confirm (3.89 KB, patch)
2021-04-23 12:38 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15563: (follow-up) Combine methods, add count, obey confirm (3.95 KB, patch)
2021-04-27 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2016-01-13 08:55:59 UTC
Patron card batches can be deleted manually, but it would also be helpful if the batches could be cleaned up automatically after X days.
Comment 1 Katrin Fischer 2019-12-22 13:32:40 UTC
*** Bug 24265 has been marked as a duplicate of this bug. ***
Comment 2 Andrew Fuerste-Henry 2020-04-14 15:33:04 UTC
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
Comment 3 Andrew Fuerste-Henry 2020-04-14 15:36:19 UTC
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.
Comment 4 Andrew Fuerste-Henry 2020-04-14 21:15:14 UTC
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
Comment 5 Andrew Fuerste-Henry 2020-04-14 21:16:25 UTC
Realized original bug also covered patron card creator. Added a separate option in cron to delete card creator patrons rather than label printed items.
Comment 6 ByWater Sandboxes 2020-04-15 13:49:01 UTC
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>
Comment 7 ByWater Sandboxes 2020-04-15 13:49:03 UTC
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>
Comment 8 Andrew Fuerste-Henry 2020-04-15 14:09:41 UTC
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>
Comment 9 Andrew Fuerste-Henry 2020-04-15 14:09:44 UTC
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>
Comment 10 Andrew Fuerste-Henry 2020-04-15 14:12:33 UTC
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."
Comment 11 Andrew Fuerste-Henry 2020-04-22 16:19:50 UTC
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>
Comment 12 Andrew Fuerste-Henry 2020-04-22 16:19:53 UTC
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>
Comment 13 Katrin Fischer 2020-06-22 11:23:23 UTC
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?
Comment 14 Andrew Fuerste-Henry 2020-07-20 15:10:52 UTC
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
Comment 15 Andrew Fuerste-Henry 2020-07-20 15:13:15 UTC
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.
Comment 16 Katrin Fischer 2020-07-30 22:14:16 UTC
Hi Andrew, the wording reads ok to me - could you please rebase?
Comment 17 Andrew Fuerste-Henry 2020-07-31 12:41:46 UTC
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
Comment 18 Andrew Fuerste-Henry 2021-03-10 17:34:57 UTC
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
Comment 19 Andrew Fuerste-Henry 2021-03-10 17:36:10 UTC
Updated patch to make sure it still applies and works
Comment 20 Andrew Fuerste-Henry 2021-03-10 17:53:26 UTC
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
Comment 21 Andrew Fuerste-Henry 2021-03-10 18:03:33 UTC
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
Comment 22 Andrew Fuerste-Henry 2021-03-10 22:02:48 UTC
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>
Comment 23 Andrew Fuerste-Henry 2021-03-10 22:02:52 UTC
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>
Comment 24 Kyle M Hall 2021-03-11 10:56:29 UTC
Andrew, I think maybe some old code for deleting branch transfers made it into your send patch. Can you double check that?
Comment 25 Andrew Fuerste-Henry 2021-03-16 19:15:58 UTC
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>
Comment 26 Martin Renvoize 2021-03-24 13:54:18 UTC
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>
Comment 27 Martin Renvoize 2021-03-24 13:54:22 UTC
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>
Comment 28 Martin Renvoize 2021-03-24 13:56:41 UTC
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>
Comment 29 Martin Renvoize 2021-03-24 13:57:45 UTC
I squashed a rebase error fix into the patch.. there was a simple missing }.


Works as expected, no regressions found.

Passing QA
Comment 30 Jonathan Druart 2021-04-07 08:54:27 UTC
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.
Comment 31 Nick Clemens 2021-04-23 12:38:44 UTC
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
Comment 32 Nick Clemens 2021-04-23 12:39:32 UTC
(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
Comment 33 Jonathan Druart 2021-04-27 13:09:07 UTC
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
Comment 34 Jonathan Druart 2021-04-27 14:25:15 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 35 Fridolin Somers 2021-04-30 08:56:53 UTC
Enhancement not pushed to 20.11.x