Bug 19008 - Add more options to cleanup database script
Summary: Add more options to cleanup database script
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Fridolin SOMERS
QA Contact: Testopia
URL:
Keywords: Manual
: 13667 (view as bug list)
Depends on:
Blocks: 18081 24152
  Show dependency treegraph
 
Reported: 2017-07-31 14:27 UTC by Fridolin SOMERS
Modified: 2020-05-16 15:49 UTC (History)
15 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
The cleanup_database.pl cronjob now also includes options for deleting: - entries from the statistics table - deleted bibliographic records and items (deletedbiblio, deletedbiblioitems, deletedbiblio_metadata, deleteditems) - deleted patrons (deleted_patrons) - returend checkouts (old_issues) - filled and cancelled holds (old_reserves) - finished transfers between libraries (branchtransfers)
Version(s) released in:
20.05.00, 19.11.05, 19.05.10


Attachments
Bug 19008 - More database cleanups - statistics (3.21 KB, patch)
2017-07-31 14:57 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - deleted catalog (4.16 KB, patch)
2017-07-31 14:58 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - deleted patrons (3.40 KB, patch)
2017-07-31 14:58 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - old issues (3.35 KB, patch)
2017-07-31 14:58 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - old reserves (3.40 KB, patch)
2017-07-31 14:58 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - item transfers (3.40 KB, patch)
2017-07-31 14:59 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - statistics (3.26 KB, patch)
2017-10-24 18:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - deleted catalog (4.21 KB, patch)
2017-10-24 18:44 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - deleted patrons (3.45 KB, patch)
2017-10-24 18:44 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - old issues (3.40 KB, patch)
2017-10-24 18:44 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - old reserves (3.46 KB, patch)
2017-10-24 18:44 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - item transfers (3.46 KB, patch)
2017-10-24 18:44 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - statistics (3.32 KB, patch)
2018-02-16 13:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - deleted catalog (4.27 KB, patch)
2018-02-16 13:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - deleted patrons (3.51 KB, patch)
2018-02-16 13:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - old issues (3.46 KB, patch)
2018-02-16 13:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - old reserves (3.51 KB, patch)
2018-02-16 13:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - item transfers (3.51 KB, patch)
2018-02-16 13:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - statistics (3.35 KB, patch)
2019-08-02 12:41 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - deleted catalog (4.24 KB, patch)
2019-08-02 12:41 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - deleted patrons (3.49 KB, patch)
2019-08-02 12:41 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - old issues (3.46 KB, patch)
2019-08-02 12:41 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - old reserves (3.52 KB, patch)
2019-08-02 12:41 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - item transfers (3.52 KB, patch)
2019-08-02 12:41 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 19008 - More database cleanups - QA followup (2.06 KB, patch)
2019-08-02 12:42 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 19008: More database cleanups - statistics (3.42 KB, patch)
2020-02-04 11:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19008: More database cleanups - deleted catalog (4.31 KB, patch)
2020-02-04 11:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19008: More database cleanups - deleted patrons (3.56 KB, patch)
2020-02-04 11:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19008: More database cleanups - old issues (3.53 KB, patch)
2020-02-04 11:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19008: More database cleanups - old reserves (3.59 KB, patch)
2020-02-04 11:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19008: More database cleanups - item transfers (3.59 KB, patch)
2020-02-04 11:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19008: More database cleanups - QA followup (2.13 KB, patch)
2020-02-04 11:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19008: Replace issues with checkouts in print statement (925 bytes, patch)
2020-02-04 11:11 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 Fridolin SOMERS 2017-07-31 14:27:34 UTC
misc/cronjobs/cleanup_database.pl provides some database cleanup.
Yet some tables that could need cleanup are not in this script.
Comment 1 Fridolin SOMERS 2017-07-31 14:57:58 UTC Comment hidden (obsolete)
Comment 2 Fridolin SOMERS 2017-07-31 14:58:14 UTC Comment hidden (obsolete)
Comment 3 Fridolin SOMERS 2017-07-31 14:58:29 UTC Comment hidden (obsolete)
Comment 4 Fridolin SOMERS 2017-07-31 14:58:42 UTC Comment hidden (obsolete)
Comment 5 Fridolin SOMERS 2017-07-31 14:58:54 UTC Comment hidden (obsolete)
Comment 6 Fridolin SOMERS 2017-07-31 14:59:06 UTC Comment hidden (obsolete)
Comment 7 Simon Pouchol 2017-10-20 14:00:49 UTC
Works as intended for the tables statistics, deleted catalogs, branch transfers and deleted patrons (there is a typo in your test plan for this one : "- Run cleanup : misc/cronjobs/cleanup_database.pl -v --delete-patrons 30" is missing a "d" at --deleted-patrons).

AS for the tables old_issues and old_reserves, I couldn't get the test done on mysql, the count returns "ERROR 1054 (42S22): Unknown column 'datetime' in 'field list'" for both tables. The cleanup seems to be working, but I just can't verify it in mysql.
Comment 8 M. Tompsett 2017-10-24 18:43:10 UTC
(In reply to Simon Pouchol from comment #7)
> Works as intended for the tables statistics, deleted catalogs, branch
> transfers and deleted patrons (there is a typo in your test plan for this
> one : "- Run cleanup : misc/cronjobs/cleanup_database.pl -v --delete-patrons
> 30" is missing a "d" at --deleted-patrons).

I am tweaking the test plan with this correction.


> AS for the tables old_issues and old_reserves, I couldn't get the test done
> on mysql, the count returns "ERROR 1054 (42S22): Unknown column 'datetime'
> in 'field list'" for both tables. The cleanup seems to be working, but I
> just can't verify it in mysql.

All these were test plan typos, likely caused by cut-and-paste.
Reading the code, they should have been timestamp.
I am tweaking the test plan to match.

The code reads well. Passes Koha QA Test tool.

And while I had no data to actually test the purge, the code ran perfectly fine. I'm signing off.
Comment 9 M. Tompsett 2017-10-24 18:43:59 UTC Comment hidden (obsolete)
Comment 10 M. Tompsett 2017-10-24 18:44:02 UTC Comment hidden (obsolete)
Comment 11 M. Tompsett 2017-10-24 18:44:05 UTC Comment hidden (obsolete)
Comment 12 M. Tompsett 2017-10-24 18:44:07 UTC Comment hidden (obsolete)
Comment 13 M. Tompsett 2017-10-24 18:44:10 UTC Comment hidden (obsolete)
Comment 14 M. Tompsett 2017-10-24 18:44:13 UTC Comment hidden (obsolete)
Comment 15 Kyle M Hall 2018-02-16 13:56:19 UTC Comment hidden (obsolete)
Comment 16 Kyle M Hall 2018-02-16 13:56:26 UTC Comment hidden (obsolete)
Comment 17 Kyle M Hall 2018-02-16 13:56:29 UTC Comment hidden (obsolete)
Comment 18 Kyle M Hall 2018-02-16 13:56:32 UTC Comment hidden (obsolete)
Comment 19 Kyle M Hall 2018-02-16 13:56:35 UTC Comment hidden (obsolete)
Comment 20 Kyle M Hall 2018-02-16 13:56:38 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2018-02-16 20:53:25 UTC
Please improve the POD to explain what means "older than DAYS days" for each option you added.
Comment 22 Katrin Fischer 2018-02-16 21:39:26 UTC
Should we add notes about the consequences? I think old_issues might be used in some reports and deleting this data will remove information like patron history etc. Maybe it should come with a warning and some hints.
Comment 23 Fridolin SOMERS 2018-02-22 05:58:25 UTC
(In reply to Jonathan Druart from comment #21)
> Please improve the POD to explain what means "older than DAYS days" for each
> option you added.

Sorry but I don't see how it could be explained.
Look at sessdays arg :
   --sessdays DAYS    purge only sessions older than DAYS days.
Comment 24 Fridolin SOMERS 2018-02-22 05:59:26 UTC
(In reply to Katrin Fischer from comment #22)
> Should we add notes about the consequences? I think old_issues might be used
> in some reports and deleting this data will remove information like patron
> history etc. Maybe it should come with a warning and some hints.

Indeed, I will add a warn for statistics saying it is used in reports and for old_issues that is used in reading history.
Comment 25 Jonathan Druart 2018-02-22 12:53:54 UTC
(In reply to Fridolin SOMERS from comment #23)
> (In reply to Jonathan Druart from comment #21)
> > Please improve the POD to explain what means "older than DAYS days" for each
> > option you added.
> 
> Sorry but I don't see how it could be explained.
> Look at sessdays arg :
>    --sessdays DAYS    purge only sessions older than DAYS days.

Yes existing POD is bad, that is not a valid argument :)

+   --statistics DAYS       Purge entries from statistics older than DAYS days.

=> this one may be ok

+   --deleted-catalog  DAYS Purge deleted catalog older than DAYS

=> This can be improved: Remove deleted catalogue records that have been removed before DAYS

+   --deleted-patrons DAYS  Purge deleted patrons older than DAYS days.

=> So if they are too old? Let remove all the retired!

+   --old-issues DAYS       Purge old issues older than DAYS days.

=> Then we are going to "purge the checkins that have been checked out" [...]

+   --old-reserves DAYS     Purge old reserves older than DAYS days.

=> I am expecting "holds" and "filled" in the description

+   --transfers DAYS        Purge arrived items transfers older than DAYS days.

=> Hum, any native English speakers around?

Should we keep "issues" and "reserves" in the parameter?
At least not in the POD.
Comment 26 Nick Clemens 2018-02-22 13:01:00 UTC
(In reply to Jonathan Druart from comment #25)

Maybe like:

Purge statistics entries more than DAYS days old

Purge catalog records deleted more then DAYS days ago

Purge patrons deleted more than DAYS days ago

Purge checkouts (old_issues) returned more than DAYS days ago

Purge transfers completed more than DAYS day ago
Comment 27 Katrin Fischer 2018-02-22 22:20:50 UTC Comment hidden (obsolete)
Comment 28 Magnus Enger 2019-08-01 09:53:45 UTC Comment hidden (obsolete)
Comment 29 Magnus Enger 2019-08-02 12:41:17 UTC Comment hidden (obsolete)
Comment 30 Magnus Enger 2019-08-02 12:41:26 UTC Comment hidden (obsolete)
Comment 31 Magnus Enger 2019-08-02 12:41:36 UTC Comment hidden (obsolete)
Comment 32 Magnus Enger 2019-08-02 12:41:43 UTC Comment hidden (obsolete)
Comment 33 Magnus Enger 2019-08-02 12:41:50 UTC Comment hidden (obsolete)
Comment 34 Magnus Enger 2019-08-02 12:41:58 UTC Comment hidden (obsolete)
Comment 35 Magnus Enger 2019-08-02 12:42:05 UTC Comment hidden (obsolete)
Comment 36 Magnus Enger 2019-08-02 12:44:06 UTC Comment hidden (obsolete)
Comment 37 Fridolin SOMERS 2019-08-05 09:58:04 UTC Comment hidden (obsolete)
Comment 38 Magnus Enger 2020-01-08 13:06:37 UTC Comment hidden (obsolete)
Comment 39 Magnus Enger 2020-01-08 13:09:29 UTC
This is clearly a step in the right direction, the cleanups added by these patches are sorely needed. 

But I think the actions of the cleanup-script should be governed by sysprefs, not command line switches. There are several reasons for this: 

- On a package install with several instances this means we can have one line in the cron-setup to trigger the script, with koha-foreach, but all the sites can have their individual configurations

- Libraries that do not have hands on access to their Koha server can change the setup without going through their provider

- And most importantly, I think: Libraries can tell what they are cleaning up and anonymizing, by looking at the sysprefs, instead of having to figure out what the command line switches are and do. (I *think* it is a requirement of the GDPR that libraries should be able to tell what they are doing with their data. Having a few sysprefs to look at seems way better than having to ask a provider what the command line switches of the cronjob are set to.)

But priority number 1 should be getting these patches in in the first place, then we can do the enhancement on another bug.
Comment 40 Jonathan Druart 2020-02-04 11:10:55 UTC
Created attachment 98373 [details] [review]
Bug 19008: More database cleanups - statistics

misc/cronjobs/cleanup_database.pl provides some database cleanup.
Yet some tables that could need cleanup are not in this script.

This patch adds cleanup for statistics table.

Test plan :
- Count statistics : select count(*),date(datetime) from statistics group by date(datetime);
- Run cleanup : misc/cronjobs/cleanup_database.pl -v --statistics 30
- Recount statistics

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 41 Jonathan Druart 2020-02-04 11:10:59 UTC
Created attachment 98374 [details] [review]
Bug 19008: More database cleanups - deleted catalog

misc/cronjobs/cleanup_database.pl provides some database cleanup.
Yet some tables that could need cleanup are not in this script.

This patch adds cleanup for deleted catalog tables.
Note that deletedbiblio_metadata is managed by foreign key on biblionumber.

Test plan :
- Count :
select count(*),year(timestamp) from deleteditems group by year(timestamp);
select count(*),year(timestamp) from deletedbiblio group by year(timestamp);
select count(*),year(timestamp) from deletedbiblioitems group by year(timestamp);
- Run cleanup : misc/cronjobs/cleanup_database.pl -v --deleted-catalog 30
- Recount

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 42 Jonathan Druart 2020-02-04 11:11:03 UTC
Created attachment 98375 [details] [review]
Bug 19008: More database cleanups - deleted patrons

misc/cronjobs/cleanup_database.pl provides some database cleanup.
Yet some tables that could need cleanup are not in this script.

This patch adds cleanup for deletedborrowers table.

Test plan :
- Count : select count(*),date(updated_on) from deletedborrowers group by date(updated_on);
- Run cleanup : misc/cronjobs/cleanup_database.pl -v --deleted-patrons 30
- Recount

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 43 Jonathan Druart 2020-02-04 11:11:07 UTC
Created attachment 98376 [details] [review]
Bug 19008: More database cleanups - old issues

misc/cronjobs/cleanup_database.pl provides some database cleanup.
Yet some tables that could need cleanup are not in this script.

This patch adds cleanup for old_issues table.

Test plan :
- Count : select count(*),date(timestamp) from old_issues group by date(timestamp);
- Run cleanup : misc/cronjobs/cleanup_database.pl -v --old-issues 30
- Recount

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 44 Jonathan Druart 2020-02-04 11:11:12 UTC
Created attachment 98377 [details] [review]
Bug 19008: More database cleanups - old reserves

misc/cronjobs/cleanup_database.pl provides some database cleanup.
Yet some tables that could need cleanup are not in this script.

This patch adds cleanup for old_reserves table.

Test plan :
- Count : select count(*),date(timestamp) from old_reserves group by date(timestamp);
- Run cleanup : misc/cronjobs/cleanup_database.pl -v --old-reserves 30
- Recount

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 45 Jonathan Druart 2020-02-04 11:11:15 UTC
Created attachment 98378 [details] [review]
Bug 19008: More database cleanups - item transfers

misc/cronjobs/cleanup_database.pl provides some database cleanup.
Yet some tables that could need cleanup are not in this script.

This patch adds cleanup for branchtransfers table.

Test plan :
- Count : select count(*),datearrived from branchtransfers group by datearrived;
- Run cleanup : misc/cronjobs/cleanup_database.pl -v --transfers 30
- Recount

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 46 Jonathan Druart 2020-02-04 11:11:20 UTC
Created attachment 98379 [details] [review]
Bug 19008: More database cleanups - QA followup

This patch tries to change the POD as suggested in the QA process.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 47 Jonathan Druart 2020-02-04 11:11:24 UTC
Created attachment 98380 [details] [review]
Bug 19008: Replace issues with checkouts in print statement

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 48 Martin Renvoize 2020-02-04 13:39:07 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 49 Joy Nelson 2020-02-10 23:58:02 UTC
Enhancement not backported to 19.11.x
Comment 50 Jonathan Druart 2020-02-14 10:51:38 UTC
*** Bug 13667 has been marked as a duplicate of this bug. ***
Comment 51 Fridolin SOMERS 2020-04-02 13:14:52 UTC
(In reply to Joy Nelson from comment #49)
> Enhancement not backported to 19.11.x

Hi, can we reconsider this ? Its needed for GDPR cleanups like old_reserves.
Comment 52 Joy Nelson 2020-04-02 18:49:28 UTC
Requested this be pushed to 19.11.x for GDPR. 
Backported to 19.11.x for 19.11.05
Comment 53 Lucas Gass 2020-04-09 20:58:01 UTC
assuming this is wanted in 19.05.x as well.

backported to 19.05.x for 19.05.10
Comment 54 Hayley Mapley 2020-04-14 23:14:31 UTC
Enhancement not backported to 18.11.x series - let me know if you'd like me to reconsider