Bug 40744 - Don't give noisy warning when PatronSelfRegistration is turned off
Summary: Don't give noisy warning when PatronSelfRegistration is turned off
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 37845
Blocks:
  Show dependency treegraph
 
Reported: 2025-09-03 07:22 UTC by Katrin Fischer
Modified: 2026-01-24 20:38 UTC (History)
5 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 40744: Ignore --del-exp-selfreg if pref is not set (1.15 KB, patch)
2026-01-22 15:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 40744: Ignore --del-exp-selfreg if pref is not set (1.15 KB, patch)
2026-01-22 15:58 UTC, Ayoub Glizi-Vicioso
Details | Diff | Splinter Review
BZ40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay (1.15 KB, patch)
2026-01-22 16:07 UTC, Ayoub Glizi-Vicioso
Details | Diff | Splinter Review
BZ40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay (1.23 KB, patch)
2026-01-22 16:12 UTC, Ayoub Glizi-Vicioso
Details | Diff | Splinter Review
BZ40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay (1.15 KB, patch)
2026-01-22 16:21 UTC, Ayoub Glizi-Vicioso
Details | Diff | Splinter Review
Bug 40744: Ignore --del-exp-selfreg if pref is not set (1.15 KB, patch)
2026-01-22 19:31 UTC, Ayoub Glizi-Vicioso
Details | Diff | Splinter Review
Bug 40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay (1.15 KB, patch)
2026-01-22 19:31 UTC, Ayoub Glizi-Vicioso
Details | Diff | Splinter Review
Bug 40744: Ignore --del-exp-selfreg if pref is not set (1.15 KB, patch)
2026-01-22 19:42 UTC, Ayoub Glizi-Vicioso
Details | Diff | Splinter Review
Bug 40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay (1.15 KB, patch)
2026-01-22 19:42 UTC, Ayoub Glizi-Vicioso
Details | Diff | Splinter Review
Bug 40744: Ignore --del-exp-selfreg if pref is not set (1.15 KB, patch)
2026-01-23 13:42 UTC, Ayoub Glizi-Vicioso
Details | Diff | Splinter Review
Bug 40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay (3.47 KB, patch)
2026-01-23 13:42 UTC, Ayoub Glizi-Vicioso
Details | Diff | Splinter Review
Bug 40744: Ignore --del-exp-selfreg if pref is not set (1.20 KB, patch)
2026-01-24 20:34 UTC, David Nind
Details | Diff | Splinter Review
Bug 40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay (3.52 KB, patch)
2026-01-24 20:35 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2025-09-03 07:22:13 UTC
We have several instances on one server. Some are using patron self registration, others do not. Now when cleanup_database.pl is run, we get a warning in the logs:

Self-registrations cannot be removed, system preference PatronSelfRegistrationDefaultCategory is not set 

This is unnecessary, as for the libraries in question PatronSelfRegistration is OFF and PatronSelfRegistrationExpireTemporaryAccountsDelay is set to 0.

The pref description says:
NOTE: This system preference requires the misc/cronjobs/cleanup_database.pl cronjob. Ask your system administrator to schedule it.
No patrons will be deleted if you set the pref to zero or make it empty. 

So we are telling the script, it's OK, we don't want any deleted, and still get the noisy warning. This is unnecessary. 

The warning should only be given when PatronSelfRegistrationExpireTemporaryAccountsDelay is not empty or 0 and the PatronSelfRegistrationDefaultCategory  is empty at the same time. We might also want to check for PatronSelfRegistration.
Comment 1 Caroline Cyr La Rose 2026-01-21 16:12:37 UTC
We have the same since upgrading our clients from 24.05 to 25.05. Very annoying!

We have the same cleanup cron for everyone on the server. We don't really want to start separating crons for the ones who have selfreg, vs those who don't.

misc/cronjobs/cleanup_database.pl [...] --del-exp-selfreg --del-unv-selfreg 30 [...] -confirm  

This is an example of a client whose Self registration is not activated:

MariaDB [***]> select variable, value from systempreferences where variable in ('PatronSelfRegistration', 'PatronSelfRegistrationDefaultCategory', 'PatronSelfRegistrationExpireTemporaryAccountsDelay');
+----------------------------------------------------+-------+
| variable                                           | value |
+----------------------------------------------------+-------+
| PatronSelfRegistration                             | 0     |
| PatronSelfRegistrationDefaultCategory              |       |
| PatronSelfRegistrationExpireTemporaryAccountsDelay | 0     |
+----------------------------------------------------+-------+
3 rows in set (0.000 sec)

Message we get

Self-registrations cannot be removed, system preference PatronSelfRegistrationDefaultCategory is not set
Comment 2 Caroline Cyr La Rose 2026-01-21 16:54:55 UTC
Probably due to the change in bug 37845
Comment 3 Jonathan Druart 2026-01-22 15:15:22 UTC
Created attachment 191869 [details] [review]
Bug 40744: Ignore --del-exp-selfreg if pref is not set

If PatronSelfRegistration we should ignore (i.e. do nothing) if
--del-exp-selfreg is passed to cleanup_database.pl

It prevents the script to warn that the pref is not set.
Comment 4 Jonathan Druart 2026-01-22 15:15:44 UTC
Would this patch work for you?
Comment 5 Katrin Fischer 2026-01-22 15:19:55 UTC
Thanks, I'll try to test as soon as I can!
Comment 6 Ayoub Glizi-Vicioso 2026-01-22 15:58:56 UTC
Created attachment 191874 [details] [review]
Bug 40744: Ignore --del-exp-selfreg if pref is not set

Skip self-registration cleanup when PatronSelfRegistrationExpireTemporaryAccountsDelay is 0 or unset as well.
Comment 7 Ayoub Glizi-Vicioso 2026-01-22 16:07:52 UTC Comment hidden (obsolete)
Comment 8 Ayoub Glizi-Vicioso 2026-01-22 16:12:17 UTC
Created attachment 191876 [details] [review]
BZ40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay
Comment 9 Ayoub Glizi-Vicioso 2026-01-22 16:21:44 UTC
Created attachment 191877 [details] [review]
BZ40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay

fix merge conflict.
Comment 10 Ayoub Glizi-Vicioso 2026-01-22 17:22:19 UTC Comment hidden (obsolete)
Comment 11 Ayoub Glizi-Vicioso 2026-01-22 17:31:30 UTC Comment hidden (obsolete)
Comment 12 Ayoub Glizi-Vicioso 2026-01-22 19:31:32 UTC
Created attachment 191886 [details] [review]
Bug 40744: Ignore --del-exp-selfreg if pref is not set

If PatronSelfRegistration we should ignore (i.e. do nothing) if
--del-exp-selfreg is passed to cleanup_database.pl

It prevents the script to warn that the pref is not set.
Comment 13 Ayoub Glizi-Vicioso 2026-01-22 19:31:35 UTC
Created attachment 191887 [details] [review]
Bug 40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay
Comment 14 Ayoub Glizi-Vicioso 2026-01-22 19:42:42 UTC Comment hidden (obsolete)
Comment 15 Ayoub Glizi-Vicioso 2026-01-22 19:42:44 UTC
Created attachment 191889 [details] [review]
Bug 40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay
Comment 16 Katrin Fischer 2026-01-23 10:42:56 UTC
Hi Ayoub, I see that you wrote the test plan in Bugzilla or when using git bz, but that way it's only here and not part of the patch file. Better to do it in the git commit step, so it's part of the file. See here: https://wiki.koha-community.org/wiki/Commit_messages

If I understand correctly your patch is a follow-up to Joubu's?
Comment 17 Katrin Fischer 2026-01-23 11:31:54 UTC
But also: thanks for writing the test plan! (should have said that first!)
Comment 18 Ayoub Glizi-Vicioso 2026-01-23 13:42:46 UTC
Created attachment 191933 [details] [review]
Bug 40744: Ignore --del-exp-selfreg if pref is not set

If PatronSelfRegistration we should ignore (i.e. do nothing) if
--del-exp-selfreg is passed to cleanup_database.pl

It prevents the script to warn that the pref is not set.
Comment 19 Ayoub Glizi-Vicioso 2026-01-23 13:42:48 UTC
Created attachment 191934 [details] [review]
Bug 40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay

Test plan

BEFORE APPLYING PATCH:

1- From the main page on the staff interface, go to Administration.
2- On the system preferences search bar, enter 'PatronSelfRegistration'.
3- Set 'PatronSelfRegistration' value to "Don't allow".
4- On the system preferences search bar, enter 'PatronSelfRegistrationExpireTemporaryAccountsDelay'.
5- Set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to '0'.
6- On command line, run cleanup_database.pl.
Note: the following message will show up:
'Self-registrations cannot be removed, system preference PatronSelfRegistrationDefaultCategory is not set'

7- Apply patch.

AFTER APPLYING PATCH:

8- Set 'PatronSelfRegistration' value to "Don't allow" and set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to '0'.
9- Save and run cleanup_database.pl : No message should show up.

10- Set 'PatronSelfRegistration' value to "Allow" and set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to '0'.
11- Save and run cleanup_database.pl : No message should show up.

12- Set 'PatronSelfRegistration' value to "Don't allow". and set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to '1'.
13- Save and run cleanup_database.pl : No message should show up.

14- Set 'PatronSelfRegistration' value to "Allow". and set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to '1'.
15- Save and run cleanup_database.pl
Note: this message should show up:
'Self-registrations cannot be removed, system preference PatronSelfRegistrationDefaultCategory is not set'

16- Set 'PatronSelfRegistration' value to "Don't allow". and set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to ''(leave empty).
17- Save and run cleanup_database.pl : No message should show up.

18- Set 'PatronSelfRegistration' value to "Allow". and set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to ''(leave empty).
19- Save and run cleanup_database.pl : No message should show up.

** EXEMPLE OF THE COMMAND TO EXECUTE: make sure to enter the RIGHT PATH to the script file.

perl /path/to/cleanup_database.pl --sessions --sessdays 1 --zebraqueue 1 --mail 365 --merged --import 90 --z3950 --logs 180 --searchhistory 30 --list-invites 7 --all-restrictions --del-exp-selfreg --del-unv-selfreg 30 --temp-uploads --temp-uploads-days 14 --oauth-tokens --transfers 30  --jobs-days 1 --jobs-type all --confirm
Comment 20 Ayoub Glizi-Vicioso 2026-01-23 13:51:10 UTC
(In reply to Katrin Fischer from comment #16)
> Hi Ayoub, I see that you wrote the test plan in Bugzilla or when using git
> bz, but that way it's only here and not part of the patch file. Better to do
> it in the git commit step, so it's part of the file. See here:
> https://wiki.koha-community.org/wiki/Commit_messages
> 
> If I understand correctly your patch is a follow-up to Joubu's?

Hi Katrin,thanks for the note. I’ve adjusted the commit and added the test plan directly to the last commit message.
Comment 21 David Nind 2026-01-24 20:34:59 UTC
Created attachment 192003 [details] [review]
Bug 40744: Ignore --del-exp-selfreg if pref is not set

If PatronSelfRegistration we should ignore (i.e. do nothing) if
--del-exp-selfreg is passed to cleanup_database.pl

It prevents the script to warn that the pref is not set.

Signed-off-by: David Nind <david@davidnind.com>
Comment 22 David Nind 2026-01-24 20:35:01 UTC
Created attachment 192004 [details] [review]
Bug 40744: Handle PatronSelfRegistration as well as PatronSelfRegistrationExpireTemporaryAccountsDelay

Test plan

BEFORE APPLYING PATCH:

1- From the main page on the staff interface, go to Administration.
2- On the system preferences search bar, enter 'PatronSelfRegistration'.
3- Set 'PatronSelfRegistration' value to "Don't allow".
4- On the system preferences search bar, enter 'PatronSelfRegistrationExpireTemporaryAccountsDelay'.
5- Set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to '0'.
6- On command line, run cleanup_database.pl.
Note: the following message will show up:
'Self-registrations cannot be removed, system preference PatronSelfRegistrationDefaultCategory is not set'

7- Apply patch.

AFTER APPLYING PATCH:

8- Set 'PatronSelfRegistration' value to "Don't allow" and set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to '0'.
9- Save and run cleanup_database.pl : No message should show up.

10- Set 'PatronSelfRegistration' value to "Allow" and set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to '0'.
11- Save and run cleanup_database.pl : No message should show up.

12- Set 'PatronSelfRegistration' value to "Don't allow". and set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to '1'.
13- Save and run cleanup_database.pl : No message should show up.

14- Set 'PatronSelfRegistration' value to "Allow". and set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to '1'.
15- Save and run cleanup_database.pl
Note: this message should show up:
'Self-registrations cannot be removed, system preference PatronSelfRegistrationDefaultCategory is not set'

16- Set 'PatronSelfRegistration' value to "Don't allow". and set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to ''(leave empty).
17- Save and run cleanup_database.pl : No message should show up.

18- Set 'PatronSelfRegistration' value to "Allow". and set 'PatronSelfRegistrationExpireTemporaryAccountsDelay' value to ''(leave empty).
19- Save and run cleanup_database.pl : No message should show up.

** EXEMPLE OF THE COMMAND TO EXECUTE: make sure to enter the RIGHT PATH to the script file.

perl /path/to/cleanup_database.pl --sessions --sessdays 1 --zebraqueue 1 --mail 365 --merged --import 90 --z3950 --logs 180 --searchhistory 30 --list-invites 7 --all-restrictions --del-exp-selfreg --del-unv-selfreg 30 --temp-uploads --temp-uploads-days 14 --oauth-tokens --transfers 30  --jobs-days 1 --jobs-type all --confirm

Signed-off-by: David Nind <david@davidnind.com>
Comment 23 David Nind 2026-01-24 20:38:43 UTC
Testing notes (using KTD):

1. Note that the example cleanup_database.pl options to use are at the end of the test plan, from /kohadevbox/koha run:

perl misc/cronjobs/cleanup_database.pl --sessions --sessdays 1 --zebraqueue 1 --mail 365 --merged --import 90 --z3950 --logs 180 --searchhistory 30 --list-invites 7 --all-restrictions --del-exp-selfreg --del-unv-selfreg 30 --temp-uploads --temp-uploads-days 14 --oauth-tokens --transfers 30  --jobs-days 1 --jobs-type all --confirm

2. Step 15: Had to clear the default value in PatronSelfRegistrationDefaultCategory (Self registration) for the message to show.

Thanks for the test plan!