19008 – Add more options to cleanup database script

Bug 19008 - Add more options to cleanup database script

Summary: Add more options to cleanup database script

Status:	CLOSED FIXED

Alias:	None

Product:	Koha
Classification:	Unclassified
Component:	Command-line Utilities (show other bugs)
Version:	Main
Hardware:	All All

Importance:	P5 - low enhancement
Assignee:	Fridolin Somers
QA Contact:	Testopia

URL:
Keywords:

Duplicates (1):	13667 (view as bug list)
Depends on:
Blocks:	18081 27965 32440 32441 24152 32439
	Show dependency tree / graph

Reported:	2017-07-31 14:27 UTC by Fridolin Somers
Modified:	2022-12-09 18:00 UTC (History)
CC List:	14 users (show)

See Also:	13667 23453
Change sponsored?:	---
Patch complexity:	Small patch
Documentation contact:
Documentation submission:
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
Circulation function:

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, Mark Tompsett	Details \| Diff \| Splinter Review
Bug 19008 - More database cleanups - deleted catalog (4.21 KB, patch) 2017-10-24 18:44 UTC, Mark Tompsett	Details \| Diff \| Splinter Review
Bug 19008 - More database cleanups - deleted patrons (3.45 KB, patch) 2017-10-24 18:44 UTC, Mark Tompsett	Details \| Diff \| Splinter Review
Bug 19008 - More database cleanups - old issues (3.40 KB, patch) 2017-10-24 18:44 UTC, Mark Tompsett	Details \| Diff \| Splinter Review
Bug 19008 - More database cleanups - old reserves (3.46 KB, patch) 2017-10-24 18:44 UTC, Mark Tompsett	Details \| Diff \| Splinter Review
Bug 19008 - More database cleanups - item transfers (3.46 KB, patch) 2017-10-24 18:44 UTC, Mark Tompsett	Details \| Diff \| Splinter Review
Bug 19008 - More database cleanups - statistics (3.32 KB, patch) 2018-02-16 13:56 UTC, Kyle M Hall (khall)	Details \| Diff \| Splinter Review
Bug 19008 - More database cleanups - deleted catalog (4.27 KB, patch) 2018-02-16 13:56 UTC, Kyle M Hall (khall)	Details \| Diff \| Splinter Review
Bug 19008 - More database cleanups - deleted patrons (3.51 KB, patch) 2018-02-16 13:56 UTC, Kyle M Hall (khall)	Details \| Diff \| Splinter Review
Bug 19008 - More database cleanups - old issues (3.46 KB, patch) 2018-02-16 13:56 UTC, Kyle M Hall (khall)	Details \| Diff \| Splinter Review
Bug 19008 - More database cleanups - old reserves (3.51 KB, patch) 2018-02-16 13:56 UTC, Kyle M Hall (khall)	Details \| Diff \| Splinter Review
Bug 19008 - More database cleanups - item transfers (3.51 KB, patch) 2018-02-16 13:56 UTC, Kyle M Hall (khall)	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
Show Obsolete (25) View All Add an attachment (proposed patch, testcase, etc.)

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)

Created attachment 65351 [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

Comment 2 Fridolin Somers 2017-07-31 14:58:14 UTC Comment hidden (obsolete)

Created attachment 65352 [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

Comment 3 Fridolin Somers 2017-07-31 14:58:29 UTC Comment hidden (obsolete)

Created attachment 65353 [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 --delete-patrons 30
- Recount

Comment 4 Fridolin Somers 2017-07-31 14:58:42 UTC Comment hidden (obsolete)

Created attachment 65354 [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(datetime) from old_issues group by date(datetime);
- Run cleanup : misc/cronjobs/cleanup_database.pl -v --old-issues 30
- Recount

Comment 5 Fridolin Somers 2017-07-31 14:58:54 UTC Comment hidden (obsolete)

Created attachment 65355 [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(datetime) from old_reserves group by date(datetime);
- Run cleanup : misc/cronjobs/cleanup_database.pl -v --old-reserves 30
- Recount

Comment 6 Fridolin Somers 2017-07-31 14:59:06 UTC Comment hidden (obsolete)

Created attachment 65356 [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

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 Mark 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 Mark Tompsett 2017-10-24 18:43:59 UTC Comment hidden (obsolete)

Created attachment 68491 [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>

Comment 10 Mark Tompsett 2017-10-24 18:44:02 UTC Comment hidden (obsolete)

Created attachment 68492 [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>

Comment 11 Mark Tompsett 2017-10-24 18:44:05 UTC Comment hidden (obsolete)

Created attachment 68493 [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>

Comment 12 Mark Tompsett 2017-10-24 18:44:07 UTC Comment hidden (obsolete)

Created attachment 68494 [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>

Comment 13 Mark Tompsett 2017-10-24 18:44:10 UTC Comment hidden (obsolete)

Created attachment 68495 [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>

Comment 14 Mark Tompsett 2017-10-24 18:44:13 UTC Comment hidden (obsolete)

Created attachment 68496 [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>

Comment 15 Kyle M Hall (khall) 2018-02-16 13:56:19 UTC Comment hidden (obsolete)

Created attachment 71771 [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>

Comment 16 Kyle M Hall (khall) 2018-02-16 13:56:26 UTC Comment hidden (obsolete)

Created attachment 71772 [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>

Comment 17 Kyle M Hall (khall) 2018-02-16 13:56:29 UTC Comment hidden (obsolete)

Created attachment 71773 [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>

Comment 18 Kyle M Hall (khall) 2018-02-16 13:56:32 UTC Comment hidden (obsolete)

Created attachment 71774 [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>

Comment 19 Kyle M Hall (khall) 2018-02-16 13:56:35 UTC Comment hidden (obsolete)

Created attachment 71775 [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>

Comment 20 Kyle M Hall (khall) 2018-02-16 13:56:38 UTC Comment hidden (obsolete)

Created attachment 71776 [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>

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 (kidclamp) 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)

> Indeed, I will add a warn for statistics saying it is used in reports and
> for old_issues that is used in reading history.

+1

Comment 28 Magnus Enger 2019-08-01 09:53:45 UTC Comment hidden (obsolete)

(In reply to Nick Clemens from comment #26)
> (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

Fridolin: Any chance you can find the time to update the POD with these suggestions? We really need a way to clean up the data that these patches clean up. :-)

Comment 29 Magnus Enger 2019-08-02 12:41:17 UTC Comment hidden (obsolete)

Created attachment 91922 [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>

Comment 30 Magnus Enger 2019-08-02 12:41:26 UTC Comment hidden (obsolete)

Created attachment 91923 [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>

Comment 31 Magnus Enger 2019-08-02 12:41:36 UTC Comment hidden (obsolete)

Created attachment 91924 [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>

Comment 32 Magnus Enger 2019-08-02 12:41:43 UTC Comment hidden (obsolete)

Created attachment 91925 [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>

Comment 33 Magnus Enger 2019-08-02 12:41:50 UTC Comment hidden (obsolete)

Created attachment 91926 [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>

Comment 34 Magnus Enger 2019-08-02 12:41:58 UTC Comment hidden (obsolete)

Created attachment 91927 [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>

Comment 35 Magnus Enger 2019-08-02 12:42:05 UTC Comment hidden (obsolete)

Created attachment 91928 [details] [review]
Bug 19008 - More database cleanups - QA followup

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

Comment 36 Magnus Enger 2019-08-02 12:44:06 UTC Comment hidden (obsolete)

I have: 

- Resolved a pretty trivial conflict: 

$ git bz apply 19008
Bug 19008 - More database cleanups

71771 - Bug 19008 - More database cleanups - statistics
71772 - Bug 19008 - More database cleanups - deleted catalog
71773 - Bug 19008 - More database cleanups - deleted patrons
71774 - Bug 19008 - More database cleanups - old issues
71775 - Bug 19008 - More database cleanups - old reserves
71776 - Bug 19008 - More database cleanups - item transfers

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 19008 - More database cleanups - statistics
Using index info to reconstruct a base tree...
M	misc/cronjobs/cleanup_database.pl
Falling back to patching base and 3-way merge...
Auto-merging misc/cronjobs/cleanup_database.pl
CONFLICT (content): Merge conflict in misc/cronjobs/cleanup_database.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 19008 - More database cleanups - statistics
Use 'git am --show-current-patch' to see the failed patch

- Tried to add POD as suggested by the fine QA folks

Not sure if this should be Needs signoff or Signed off, setting it to the former.

Comment 37 Fridolin Somers 2019-08-05 09:58:04 UTC Comment hidden (obsolete)

@Magnus
Merci beaucoup ;)
= Thanks a lot

Comment 38 Magnus Enger 2020-01-08 13:06:37 UTC Comment hidden (obsolete)

So I did a small rebase and added a followup with some POD. Setting the status to "Signed off". Please change it back if my changes require another signoff.

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 (ashimema) 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 (lukeg) 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 Pelham 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