Bug 26831

Summary:	Enable librarians to control when unaccepted private list share invites are removed by the cleanup_database.pl cronjob
Product:	Koha	Reporter:	Alex Buckley <alexbuckley>
Component:	Command-line Utilities	Assignee:	Alex Buckley <alexbuckley>
Status:	RESOLVED FIXED	QA Contact:	Kyle M Hall (khall) <kyle>
Severity:	enhancement
Priority:	P5 - low	CC:	caroline.cyr-la-rose, david, fridolin.somers, kyle, lucas, magnus, martin.renvoize, robin
Version:	Main
Hardware:	All
OS:	All
See Also:	https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24968
Change sponsored?:	Sponsored	Patch complexity:	---
Documentation contact:	Caroline Cyr La Rose	Documentation submission:	https://gitlab.com/koha-community/koha-manual/-/merge_requests/929
Text to go in the release notes:	The new `PurgeListShareInvitesOlderThan` system preference enables librarians to control when unaccepted private list share invites are removed from the database. Unaccepted private list share invites will now be removed based on the following prioritised options: - Priority 1. Use DAYS value when the cleanup_database.pl cronjob is run with a --list-invites DAYS parameter specified. - Priority 2. Use the PurgeListShareInvitesOlderThan system preference value. - Priority 3. Use a default of 14 days, if the cleanup_database.pl cronjob is run with a --list-invites parameter missing the DAYS value, AND the PurgeListShareInvitesOlderThan system preference is empty. - Priority 4. Don't remove any unaccepted private list share invites if the cleanup_database.pl cronjob is run without the --list-invites parameter and the PurgeListShareInvitesOlderThan syspref is empty.	Version(s) released in:	24.05.00,23.11.03
Circulation function:
Bug Depends on:	13701
Bug Blocks:	35893
Attachments:	Bug 26831: Add a system preference to define the number of days used for purging unaccepted list share invites in cleanup_database.pl Bug 26831: Add new system preference PurgeListShareInvitesOlderThan Bug 26831: Use new PurgeListShareInvitesOlderThan syspref Bug 26831: Attempting to clarify priotising of options Bug 26831: Attempting to clarify prioritising of options Bug 26831: Use new PurgeListShareInvitesOlderThan syspref Bug 26831: Attempting to clarify prioritising of options Bug 26831: Add new system preference PurgeListShareInvitesOlderThan Bug 26831: Use new PurgeListShareInvitesOlderThan syspref Bug 26831: Attempting to clarify prioritising of options Bug 26831: Add new system preference PurgeListShareInvitesOlderThan Bug 26831: Use new PurgeListShareInvitesOlderThan syspref Bug 26831: Attempting to clarify prioritising of options Bug 26831: (follow-up) Update system preference description Bug 26831: Add new system preference PurgeListShareInvitesOlderThan Bug 26831: Use new PurgeListShareInvitesOlderThan syspref Bug 26831: Attempting to clarify prioritising of options Bug 26831: (follow-up) Update system preference description Bug 26831: (follow-up) Tidy code for qa script Bug 26831: (follow-up) Tidy cleanup_database.pl

Description Alex Buckley 2020-10-26 22:57:10 UTC

Currently the cleanup_database.pl cronjob accepts a 'list-invites' parameter. This is the age (in days) at which list share invites are purged from the database.

If this parameter is not passed to the cleanup_database.pl cronjob then it defaults to 14.

It would be nice to replace this cronjob parameter - which only users with server access can alter - with a system preference.  

This system preference would have a default value of 14 for new and upgrading Koha installs.

Comment 1 Alex Buckley 2020-10-28 10:35:22 UTC

Created attachment 112634 [details] [review]
Bug 26831: Add a system preference to define the number of days used for purging unaccepted list share invites in cleanup_database.pl

The PurgeUnacceptedListShareInvitesOlderThan system preference value
is used whenever the cleanup_database.pl cron job is called without the
--list-invites parameter.

If neither the system preference or the --list-invites parameter has a
value then the cronjob errors out.

Test plan:
1. Apply patch
2. Update database:
sudo koha-shell <instancename>
cd installer/data/mysql
./updatedatabase.pl

3. Confirm the default value of the
PurgeUnacceptedListShareInvitesOlderThan system preference is 14

4. Run the cleanup_database.pl script without the --list-invites parameter:
sudo koha-shell <instancename>
cd misc/cronjobs
./cleanup_database.pl -v

5. Confirm the following text is included in the output:
"Purging unaccepted list share invites older than 14 days."

6. Clear the aforementioned system preference. Run the cleanup_database.pl cron job with
the --list-invites parameter defined:
./cleanup_database.pl --list-invites 10 -v

7. Confirm the following text is included in the output:
"Purging unaccepted list share invites older than 10 days."

8. Input a value of 20 into the aforementioned system preference. Run the cron job with --list-invites defined:
./cleanup_database.pl --list-invites 10 -v

9. Confirm the following text is included in the output:
"Purging unaccepted list share invites older than 10 days."

10. Clear the aforementioned system preference value and run the cron job without the
--list-invites parameter:
./cleanup_database.pl -v

11. Confirm the following text is included in the output:
"You did not specify any cleanup work for the script to do"

Sponsored-by: Catalyst IT

Comment 2 Alex Buckley 2020-10-28 10:44:10 UTC

Hi Katrin,

I've done a bit of investigation and found bug 13287  , and wondered if our approach to the --list-invites parameter in the cleanup_database.pl cron job should change?

Looking at that bug, I saw instead of replacing the cron parameter with a system preference, they used both:

* If both parameter and syspref are set then prioritise the parameter value
* If only syspref is set then use syspref value
* If only parameter set then use parameter value
* If neither syspref nor parameter value is set then don't do anything

If we go with this approach, then at least existing installs can to continue to call the cleanup_database.pl cronjob, with the list-invites parameter, without a change of behaviour. What are your thoughts on this?

I thought it good to have a discussion about this, before I start writing the unit tests!

Thanks,
Alex

Comment 3 Katrin Fischer 2022-09-07 13:05:50 UTC

Hi Alex, that makes sense to me. 

IIUC: Replacing would probably mean that it would stop deleting until libraries set it up again, which could be easily missed for a while. Having both together would leave options and not interrupt on update. 

The prioritizing of options should be well documented.

Comment 4 Alex Buckley 2022-09-09 01:57:09 UTC

(In reply to Katrin Fischer from comment #3)
> Hi Alex, that makes sense to me. 
> 
> IIUC: Replacing would probably mean that it would stop deleting until
> libraries set it up again, which could be easily missed for a while. Having
> both together would leave options and not interrupt on update. 
> 
> The prioritizing of options should be well documented.

Hi Katrin, 

Thanks for that, and yes you're absolutely right that is what I am thinking, and agreed prioritizing of options should definitely be well documented to avoid confusion.

Thanks,
Alex

Comment 5 Alex Buckley 2022-09-19 00:42:54 UTC

Created attachment 140747 [details] [review]
Bug 26831: Add new system preference PurgeListShareInvitesOlderThan

Sponsored-by: Catalyst IT, New Zealand

Comment 6 Alex Buckley 2022-09-19 00:43:19 UTC

Created attachment 140748 [details] [review]
Bug 26831: Use new PurgeListShareInvitesOlderThan syspref

Enable librarians in the staff client to control when unaccepted list
invites will be removed from the database.

Test plan:
1. Apply patches

2. Update database
sudo koha-shell <instance>
cd installer/data/mysql
./updatedatabase.pl

3. Confirm the PurgeListShareInvitesOlderThan syspref = 14

4. Run cleanup_database.pl with --list-invites parameter defined:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl --list-invites 10 -v

5. Confirm the output contains:
"Purging unaccepted list share invites older than 10 days."

6. Set PurgeListShareInvitesOlderThan syspref = 15

7. Run cleanup_database.pl without the --list-invites parameter:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl -v

8. Confirm the output contains:
"Purging unaccepted list share invites older than 15 days."

9. Empty PurgeListShareInvitesOlderThan syspref

10. Run cleanup_database without the --list-invites parameter:
sudo koha-shell <instance.
cd misc/cronjobs
./cleanup_database.pl

11. Observe you get the following output (along with documentation on all
parameters of the script):
"You did not specify any cleanup work for the script to do."

12. Run the cleanup_database with no days defined for the --list-invites
parameter:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl --list-invites -v

13. Confirm the output contains:
"Purging unaccepted list share invites older than 14 days."

Sponsored-by: Catalyst IT, New Zealand

Comment 7 Alex Buckley 2022-09-19 00:43:59 UTC

Created attachment 140749 [details] [review]
Bug 26831: Attempting to clarify priotising of options

Sponsored-by: Catalyst IT, New Zealand

Comment 8 Alex Buckley 2022-09-19 00:46:51 UTC

Created attachment 140750 [details] [review]
Bug 26831: Attempting to clarify prioritising of options

Sponsored-by: Catalyst IT, New Zealand

Comment 9 Alex Buckley 2022-09-19 00:53:17 UTC

Created attachment 140751 [details] [review]
Bug 26831: Use new PurgeListShareInvitesOlderThan syspref

Enable librarians in the staff client to control when unaccepted list
invites will be removed from the database.

Test plan:
1. Apply patches

2. Update database
sudo koha-shell <instance>
cd installer/data/mysql
./updatedatabase.pl

3. Confirm the PurgeListShareInvitesOlderThan syspref = 14

4. Run cleanup_database.pl with --list-invites parameter defined:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl --list-invites 10 -v

5. Confirm the output contains:
"Purging unaccepted list share invites older than 10 days."

6. Set PurgeListShareInvitesOlderThan syspref = 15

7. Run cleanup_database.pl without the --list-invites parameter:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl -v

8. Confirm the output contains:
"Purging unaccepted list share invites older than 15 days."

9. Empty PurgeListShareInvitesOlderThan syspref

10. Run cleanup_database without the --list-invites parameter:
sudo koha-shell <instance.
cd misc/cronjobs
./cleanup_database.pl

11. Observe you get the following output (along with documentation on all
parameters of the script):
"You did not specify any cleanup work for the script to do."

12. Run the cleanup_database with no days defined for the --list-invites
parameter:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl --list-invites -v

13. Confirm the output contains:
"Purging unaccepted list share invites older than 14 days."

Sponsored-by: Catalyst IT, New Zealand

Comment 10 Alex Buckley 2022-09-19 00:53:55 UTC

Created attachment 140752 [details] [review]
Bug 26831: Attempting to clarify prioritising of options

Sponsored-by: Catalyst IT, New Zealand

Comment 11 Alex Buckley 2022-09-19 01:10:48 UTC

Ready for testing. 

Please feel free to update the release notes if you think you can improve by clarifying the prioritising of the options for controlling when unaccepted private list share invites are deleted.

Comment 12 David Nind 2023-09-02 20:23:11 UTC

The patches no longer apply 8-(..

git bz apply 26831

Bug 26831 - Enable librarians to control when unaccepted private list share invites are removed by the cleanup_database.pl cronjob

140747 - Bug 26831: Add new system preference PurgeListShareInvitesOlderThan
140751 - Bug 26831: Use new PurgeListShareInvitesOlderThan syspref
140752 - Bug 26831: Attempting to clarify prioritising of options

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 26831: Add new system preference PurgeListShareInvitesOlderThan
Using index info to reconstruct a base tree...
M	installer/data/mysql/mandatory/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Auto-merging installer/data/mysql/mandatory/sysprefs.sql
error: Failed to merge in the changes.
Patch failed at 0001 Bug 26831: Add new system preference PurgeListShareInvitesOlderThan

Comment 13 Alex Buckley 2023-10-23 17:51:08 UTC

Created attachment 157684 [details] [review]
Bug 26831: Add new system preference PurgeListShareInvitesOlderThan

Sponsored-by: Catalyst IT, New Zealand

Comment 14 Alex Buckley 2023-10-23 17:51:30 UTC

Created attachment 157685 [details] [review]
Bug 26831: Use new PurgeListShareInvitesOlderThan syspref

Enable librarians in the staff client to control when unaccepted list
invites will be removed from the database.

Test plan:
1. Apply patches

2. Update database
sudo koha-shell <instance>
cd installer/data/mysql
./updatedatabase.pl

3. Confirm the PurgeListShareInvitesOlderThan syspref = 14

4. Run cleanup_database.pl with --list-invites parameter defined:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl --list-invites 10 -v

5. Confirm the output contains:
"Purging unaccepted list share invites older than 10 days."

6. Set PurgeListShareInvitesOlderThan syspref = 15

7. Run cleanup_database.pl without the --list-invites parameter:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl -v

8. Confirm the output contains:
"Purging unaccepted list share invites older than 15 days."

9. Empty PurgeListShareInvitesOlderThan syspref

10. Run cleanup_database without the --list-invites parameter:
sudo koha-shell <instance.
cd misc/cronjobs
./cleanup_database.pl

11. Observe you get the following output (along with documentation on all
parameters of the script):
"You did not specify any cleanup work for the script to do."

12. Run the cleanup_database with no days defined for the --list-invites
parameter:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl --list-invites -v

13. Confirm the output contains:
"Purging unaccepted list share invites older than 14 days."

Sponsored-by: Catalyst IT, New Zealand

Comment 15 Alex Buckley 2023-10-23 17:51:47 UTC

Created attachment 157686 [details] [review]
Bug 26831: Attempting to clarify prioritising of options

Sponsored-by: Catalyst IT, New Zealand

Comment 16 Alex Buckley 2023-10-23 17:52:49 UTC

(In reply to David Nind from comment #12)
> The patches no longer apply 8-(..
> 
> git bz apply 26831
> 
> Bug 26831 - Enable librarians to control when unaccepted private list share
> invites are removed by the cleanup_database.pl cronjob
> 
> 140747 - Bug 26831: Add new system preference PurgeListShareInvitesOlderThan
> 140751 - Bug 26831: Use new PurgeListShareInvitesOlderThan syspref
> 140752 - Bug 26831: Attempting to clarify prioritising of options
> 
> Apply? [(y)es, (n)o, (i)nteractive] y
> Applying: Bug 26831: Add new system preference PurgeListShareInvitesOlderThan
> Using index info to reconstruct a base tree...
> M	installer/data/mysql/mandatory/sysprefs.sql
> M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
> Falling back to patching base and 3-way merge...
> Auto-merging
> koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
> CONFLICT (content): Merge conflict in
> koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
> Auto-merging installer/data/mysql/mandatory/sysprefs.sql
> error: Failed to merge in the changes.
> Patch failed at 0001 Bug 26831: Add new system preference
> PurgeListShareInvitesOlderThan

Hi David, 

Sorry for my delay, I've just back from a long holiday. I hope you are doing well :)

I've rebased these patches against master. Ready to test again now.

Alex

Comment 17 David Nind 2023-10-23 23:30:08 UTC

Created attachment 157711 [details] [review]
Bug 26831: Add new system preference PurgeListShareInvitesOlderThan

Sponsored-by: Catalyst IT, New Zealand
Signed-off-by: David Nind <david@davidnind.com>

Comment 18 David Nind 2023-10-23 23:30:11 UTC

Created attachment 157712 [details] [review]
Bug 26831: Use new PurgeListShareInvitesOlderThan syspref

Enable librarians in the staff client to control when unaccepted list
invites will be removed from the database.

Test plan:
1. Apply patches

2. Update database
sudo koha-shell <instance>
cd installer/data/mysql
./updatedatabase.pl

3. Confirm the PurgeListShareInvitesOlderThan syspref = 14

4. Run cleanup_database.pl with --list-invites parameter defined:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl --list-invites 10 -v

5. Confirm the output contains:
"Purging unaccepted list share invites older than 10 days."

6. Set PurgeListShareInvitesOlderThan syspref = 15

7. Run cleanup_database.pl without the --list-invites parameter:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl -v

8. Confirm the output contains:
"Purging unaccepted list share invites older than 15 days."

9. Empty PurgeListShareInvitesOlderThan syspref

10. Run cleanup_database without the --list-invites parameter:
sudo koha-shell <instance.
cd misc/cronjobs
./cleanup_database.pl

11. Observe you get the following output (along with documentation on all
parameters of the script):
"You did not specify any cleanup work for the script to do."

12. Run the cleanup_database with no days defined for the --list-invites
parameter:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl --list-invites -v

13. Confirm the output contains:
"Purging unaccepted list share invites older than 14 days."

Sponsored-by: Catalyst IT, New Zealand
Signed-off-by: David Nind <david@davidnind.com>

Comment 19 David Nind 2023-10-23 23:30:14 UTC

Created attachment 157713 [details] [review]
Bug 26831: Attempting to clarify prioritising of options

Sponsored-by: Catalyst IT, New Zealand
Signed-off-by: David Nind <david@davidnind.com>

Comment 20 David Nind 2023-10-23 23:30:16 UTC

Created attachment 157714 [details] [review]
Bug 26831: (follow-up) Update system preference description

Signed-off-by: David Nind <david@davidnind.com>

Comment 21 David Nind 2023-10-23 23:34:54 UTC

(In reply to Alex Buckley from comment #16)
> Hi David, 
> 
> Sorry for my delay, I've just back from a long holiday. I hope you are doing
> well :)
> 
> I've rebased these patches against master. Ready to test again now.
> 
> Alex

Hi Alex.

Welcome back! I hope you had a great holiday! (I'm doing well, thanks!)

I've now signed off.

I've also updated the system preference description, mainly for consistency with other system preferences (such as NOTE in capitals, script name in a code block) as well as some other wording changes. Feel free to remove if you prefer the original description.

David

Comment 22 Alex Buckley 2023-10-29 12:40:26 UTC

(In reply to David Nind from comment #21)
> (In reply to Alex Buckley from comment #16)
> > Hi David, 
> > 
> > Sorry for my delay, I've just back from a long holiday. I hope you are doing
> > well :)
> > 
> > I've rebased these patches against master. Ready to test again now.
> > 
> > Alex
> 
> Hi Alex.
> 
> Welcome back! I hope you had a great holiday! (I'm doing well, thanks!)
> 
> I've now signed off.
> 
> I've also updated the system preference description, mainly for consistency
> with other system preferences (such as NOTE in capitals, script name in a
> code block) as well as some other wording changes. Feel free to remove if
> you prefer the original description.
> 
> David

Hi David

Thanks so much, yes I did have a very nice holiday around Italy with my parents it was lovely. That's great to hear you're doing well :)

Thank you for testing and signing off, also appreciate your update to the system preference description that is much better now!

Kind regards,
Alex

Comment 23 Kyle M Hall (khall) 2023-11-17 17:27:57 UTC

Created attachment 159097 [details] [review]
Bug 26831: Add new system preference PurgeListShareInvitesOlderThan

Sponsored-by: Catalyst IT, New Zealand
Signed-off-by: David Nind <david@davidnind.com>

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

Comment 24 Kyle M Hall (khall) 2023-11-17 17:28:04 UTC

Created attachment 159098 [details] [review]
Bug 26831: Use new PurgeListShareInvitesOlderThan syspref

Enable librarians in the staff client to control when unaccepted list
invites will be removed from the database.

Test plan:
1. Apply patches

2. Update database
sudo koha-shell <instance>
cd installer/data/mysql
./updatedatabase.pl

3. Confirm the PurgeListShareInvitesOlderThan syspref = 14

4. Run cleanup_database.pl with --list-invites parameter defined:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl --list-invites 10 -v

5. Confirm the output contains:
"Purging unaccepted list share invites older than 10 days."

6. Set PurgeListShareInvitesOlderThan syspref = 15

7. Run cleanup_database.pl without the --list-invites parameter:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl -v

8. Confirm the output contains:
"Purging unaccepted list share invites older than 15 days."

9. Empty PurgeListShareInvitesOlderThan syspref

10. Run cleanup_database without the --list-invites parameter:
sudo koha-shell <instance.
cd misc/cronjobs
./cleanup_database.pl

11. Observe you get the following output (along with documentation on all
parameters of the script):
"You did not specify any cleanup work for the script to do."

12. Run the cleanup_database with no days defined for the --list-invites
parameter:
sudo koha-shell <instance>
cd misc/cronjobs
./cleanup_database.pl --list-invites -v

13. Confirm the output contains:
"Purging unaccepted list share invites older than 14 days."

Sponsored-by: Catalyst IT, New Zealand
Signed-off-by: David Nind <david@davidnind.com>

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

Comment 25 Kyle M Hall (khall) 2023-11-17 17:28:06 UTC

Created attachment 159099 [details] [review]
Bug 26831: Attempting to clarify prioritising of options

Sponsored-by: Catalyst IT, New Zealand
Signed-off-by: David Nind <david@davidnind.com>

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

Comment 26 Kyle M Hall (khall) 2023-11-17 17:28:09 UTC

Created attachment 159100 [details] [review]
Bug 26831: (follow-up) Update system preference description

Signed-off-by: David Nind <david@davidnind.com>

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

Comment 27 Kyle M Hall (khall) 2023-11-17 17:28:11 UTC

Created attachment 159101 [details] [review]
Bug 26831: (follow-up) Tidy code for qa script

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

Comment 28 Kyle M Hall (khall) 2023-11-17 17:28:13 UTC

Created attachment 159102 [details] [review]
Bug 26831: (follow-up) Tidy cleanup_database.pl

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

Comment 29 Katrin Fischer 2024-01-16 16:43:22 UTC

I really like seeing us move settings like this into the system preference/GUI. It makes it much easier to manage different settings for servers with multiple instances. Thanks!

Comment 30 Katrin Fischer 2024-01-17 10:40:16 UTC

Pushed for 24.05!

Well done everyone, thank you!

Comment 31 Fridolin Somers 2024-01-31 12:42:39 UTC

23.11.03 is still young, I choose to backport the enhancement :)

Pushed to 23.11.x for 23.11.03

Comment 32 Lucas Gass (lukeg) 2024-02-08 18:46:32 UTC

Enhancement, no backport for 23.05.x