Bug 21549 - Lock expired patron accounts after x days
Summary: Lock expired patron accounts after x days
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Marcel de Rooy
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords: Manual
Depends on:
Blocks: 28000
  Show dependency tree / graph
 
Reported: 2018-10-11 12:27 UTC by Marcel de Rooy
Modified: 2021-12-13 21:12 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This report adds pref LockExpiredDelay. When you enter a value there, the cleanup_database.pl cron job will lock the accounts of patrons that expired at least the specified number of days. This follows the same pattern as existing code that already allows you to anonymize or delete locked patrons.
Version(s) released in:
21.05.00
Circulation function:

Attachments
Bug 21549: Lock expired patron accounts (3.69 KB, patch)
2020-08-10 13:45 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 21549: Database revision for new preference (3.64 KB, patch)
2020-08-10 13:46 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 21549: Lock expired patron accounts (3.69 KB, patch)
2020-10-12 08:04 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 21549: Database revision for new preference (3.64 KB, patch)
2020-10-12 08:04 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 21549: Lock expired patron accounts (3.70 KB, patch)
2020-11-21 22:29 UTC, Michal Denar 		Details | Diff | Splinter Review
Bug 21549: Database revision for new preference (3.69 KB, patch)
2020-11-21 22:29 UTC, Michal Denar 		Details | Diff | Splinter Review
Bug 21549: Lock expired patron accounts (3.74 KB, patch)
2021-03-19 08:20 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 21549: Database revision for new preference (3.73 KB, patch)
2021-03-19 08:20 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 21549: (follow-up) Switch to filter_by (3.36 KB, patch)
2021-03-19 08:20 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 21549: (follow-up) Adjust preference description (1.18 KB, patch)
2021-03-19 08:20 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 21549: Lock expired patron accounts (3.76 KB, patch)
2021-03-19 12:04 UTC, Nick Clemens (kidclamp) 		Details | Diff | Splinter Review
Bug 21549: Database revision for new preference (3.74 KB, patch)
2021-03-19 12:04 UTC, Nick Clemens (kidclamp) 		Details | Diff | Splinter Review
Bug 21549: (follow-up) Switch to filter_by (3.37 KB, patch)
2021-03-19 12:04 UTC, Nick Clemens (kidclamp) 		Details | Diff | Splinter Review
Bug 21549: (follow-up) Adjust preference description (1.20 KB, patch)
2021-03-19 12:04 UTC, Nick Clemens (kidclamp) 		Details | Diff | Splinter Review
Bug 21549: (QA follow-up) Get pref at start of run and rename variable (1.91 KB, patch)
2021-03-19 12:04 UTC, Nick Clemens (kidclamp) 		Details | Diff | Splinter Review
Bug 21549: Lock expired patron accounts (3.76 KB, patch)
2021-04-12 14:23 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 21549: Database revision for new preference (3.73 KB, patch)
2021-04-12 14:23 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 21549: (follow-up) Switch to filter_by (3.38 KB, patch)
2021-04-12 14:23 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 21549: (follow-up) Adjust preference description (1.20 KB, patch)
2021-04-12 14:23 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 21549: (QA follow-up) Get pref at start of run and rename variable (1.92 KB, patch)
2021-04-12 14:23 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 21549: Use filter_by_last_update from filter_by_dateexpiry (4.33 KB, patch)
2021-04-12 14:23 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 21549: Rename filter_by_dateexpiry with filter_by_expiration_date (3.27 KB, patch)
2021-04-12 14:23 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 21549: Make filter_by_last_update returns a Koha::Objects-based object (1.48 KB, patch)
2021-04-12 14:24 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 21549: Use filter_by_last_update from filter_by_dateexpiry (4.35 KB, patch)
2021-04-13 11:12 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 21549: Rename filter_by_dateexpiry with filter_by_expiration_date (3.36 KB, patch)
2021-04-13 11:12 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 21549: Make filter_by_last_update returns a Koha::Objects-based object (1.57 KB, patch)
2021-04-13 11:12 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Show Obsolete (18) 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 Marcel de Rooy 2018-10-11 12:27:03 UTC 
See also bug 21336.
We could do the same for expired patron accounts. We would need an additional pref to define the number of days to wait before locking the account. After that they run in the same workflow as the unsubscribe requests in 21336 (lock, anonymize, delete).

Would it be useful?
Comment 1 Marcel de Rooy 2018-10-15 07:37:49 UTC 
Essentially, we need something like:

Koha::Patrons->search_expired({ days => $days })->search({ login_attempts => { '!=' => -1 } })->lock({ remove => 1, verbose => $verbose });
Comment 2 Marcel de Rooy 2020-08-10 13:45:56 UTC 
Created attachment 108000 [details] [review]
Bug 21549: Lock expired patron accounts

Adding a search on locked patrons to the search_expired in cron script.
This prevents relocking.

Test plan:
Prove Koha/Patrons.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2020-08-10 13:46:01 UTC 
Created attachment 108001 [details] [review]
Bug 21549: Database revision for new preference

Introducing LockExpiredDelay.

Test plan:
Run upgrade. Check syspref.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Marcel de Rooy 2020-10-12 08:04:07 UTC 
Created attachment 111480 [details] [review]
Bug 21549: Lock expired patron accounts

Adding a search on locked patrons to the search_expired in cron script.
This prevents relocking.

Test plan:
Prove Koha/Patrons.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2020-10-12 08:04:12 UTC 
Created attachment 111481 [details] [review]
Bug 21549: Database revision for new preference

Introducing LockExpiredDelay.

Test plan:
Run upgrade. Check syspref.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2020-10-12 08:06:28 UTC 
Rebased. Please test !
Comment 7 Michal Denar 2020-11-21 22:20:07 UTC 
Hi,
plaease rebase on master.

Thank you.
Comment 8 Michal Denar 2020-11-21 22:27:14 UTC 
Sorry, Patch wors for me now. My fault. Back to NSO.
Comment 9 Michal Denar 2020-11-21 22:29:09 UTC 
Created attachment 113914 [details] [review]
Bug 21549: Lock expired patron accounts

Adding a search on locked patrons to the search_expired in cron script.
This prevents relocking.

Test plan:
Prove Koha/Patrons.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 10 Michal Denar 2020-11-21 22:29:12 UTC 
Created attachment 113915 [details] [review]
Bug 21549: Database revision for new preference

Introducing LockExpiredDelay.

Test plan:
Run upgrade. Check syspref.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 11 Marcel de Rooy 2020-11-23 07:04:25 UTC 
(In reply to Michal Denar from comment #10)
> Created attachment 113915 [details] [review] [review]
> Bug 21549: Database revision for new preference
> 
> Introducing LockExpiredDelay.
> 
> Test plan:
> Run upgrade. Check syspref.
> 
> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
> 
> Signed-off-by: Michal Denar <black23@gmail.com>

Thanks Michal
Comment 12 Nick Clemens (kidclamp) 2021-01-11 13:32:05 UTC 
-I think the method needs to be 'filter_by' -see bug 11983

-The pref description should include reliance on the cleanup database cron:
NOTE: This system preference requires the misc/cronjobs/cleanup_database.pl cronjob. Ask your system administrator to schedule it.
Comment 13 Marcel de Rooy 2021-03-19 08:20:24 UTC 
Created attachment 118493 [details] [review]
Bug 21549: Lock expired patron accounts

Adding a search on locked patrons to the search_expired in cron script.
This prevents relocking.

Test plan:
Prove Koha/Patrons.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 14 Marcel de Rooy 2021-03-19 08:20:27 UTC 
Created attachment 118494 [details] [review]
Bug 21549: Database revision for new preference

Introducing LockExpiredDelay.

Test plan:
Run upgrade. Check syspref.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 15 Marcel de Rooy 2021-03-19 08:20:31 UTC 
Created attachment 118495 [details] [review]
Bug 21549: (follow-up) Switch to filter_by

See QA remarks.

Test plan:
Run t/db_dependent/Koha/Patrons.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Marcel de Rooy 2021-03-19 08:20:34 UTC 
Created attachment 118496 [details] [review]
Bug 21549: (follow-up) Adjust preference description

See QA remarks.

Test plan:
Check the preference in staff client.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 17 Marcel de Rooy 2021-03-19 08:21:19 UTC 
(In reply to Nick Clemens from comment #12)
> -I think the method needs to be 'filter_by' -see bug 11983
> 
> -The pref description should include reliance on the cleanup database cron:
> NOTE: This system preference requires the misc/cronjobs/cleanup_database.pl
> cronjob. Ask your system administrator to schedule it.

Thx for QAing, Nick. The two follow-ups should do the job.
Comment 18 Nick Clemens (kidclamp) 2021-03-19 12:04:37 UTC 
Created attachment 118517 [details] [review]
Bug 21549: Lock expired patron accounts

Adding a search on locked patrons to the search_expired in cron script.
This prevents relocking.

Test plan:
Prove Koha/Patrons.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 19 Nick Clemens (kidclamp) 2021-03-19 12:04:40 UTC 
Created attachment 118518 [details] [review]
Bug 21549: Database revision for new preference

Introducing LockExpiredDelay.

Test plan:
Run upgrade. Check syspref.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 20 Nick Clemens (kidclamp) 2021-03-19 12:04:43 UTC 
Created attachment 118519 [details] [review]
Bug 21549: (follow-up) Switch to filter_by

See QA remarks.

Test plan:
Run t/db_dependent/Koha/Patrons.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 21 Nick Clemens (kidclamp) 2021-03-19 12:04:46 UTC 
Created attachment 118520 [details] [review]
Bug 21549: (follow-up) Adjust preference description

See QA remarks.

Test plan:
Check the preference in staff client.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 22 Nick Clemens (kidclamp) 2021-03-19 12:04:49 UTC 
Created attachment 118521 [details] [review]
Bug 21549: (QA follow-up) Get pref at start of run and rename variable

This patch ensures the script will lock patrons if the pref is set but no other actions
specified

It also changes the variable name to be a bit more explicit

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 23 Jonathan Druart 2021-04-07 11:09:59 UTC 
It seems that we should use filter_by_last_update (inherited from Koha::Objects) here.
Comment 24 Marcel de Rooy 2021-04-09 06:58:59 UTC 
(In reply to Jonathan Druart from comment #23)
> It seems that we should use filter_by_last_update (inherited from
> Koha::Objects) here.

I do not consider this as a valid reason for failing this patch. Note that a filter on dateexpiry is not a filter on "last update".
Perhaps this routine should be made more generic first?
Comment 25 Jonathan Druart 2021-04-09 13:53:18 UTC 
Well, I do consider it's a valid reason. Adding a method that is already inherited is not acceptable (duplication of code).

If you are not willing to do it I will have a look and investigate when possible.

Note that there is a difference between the 2 methods, < vs <=.
Comment 26 Jonathan Druart 2021-04-12 14:23:28 UTC 
Created attachment 119457 [details] [review]
Bug 21549: Lock expired patron accounts

Adding a search on locked patrons to the search_expired in cron script.
This prevents relocking.

Test plan:
Prove Koha/Patrons.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 27 Jonathan Druart 2021-04-12 14:23:32 UTC 
Created attachment 119458 [details] [review]
Bug 21549: Database revision for new preference

Introducing LockExpiredDelay.

Test plan:
Run upgrade. Check syspref.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 28 Jonathan Druart 2021-04-12 14:23:37 UTC 
Created attachment 119459 [details] [review]
Bug 21549: (follow-up) Switch to filter_by

See QA remarks.

Test plan:
Run t/db_dependent/Koha/Patrons.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 29 Jonathan Druart 2021-04-12 14:23:42 UTC 
Created attachment 119460 [details] [review]
Bug 21549: (follow-up) Adjust preference description

See QA remarks.

Test plan:
Check the preference in staff client.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 30 Jonathan Druart 2021-04-12 14:23:47 UTC 
Created attachment 119461 [details] [review]
Bug 21549: (QA follow-up) Get pref at start of run and rename variable

This patch ensures the script will lock patrons if the pref is set but no other actions
specified

It also changes the variable name to be a bit more explicit

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 31 Jonathan Druart 2021-04-12 14:23:52 UTC 
Created attachment 119462 [details] [review]
Bug 21549: Use filter_by_last_update from filter_by_dateexpiry

To not reengineer the patch this patch does not remove the new
subroutine but make it use the logic from
Koha::Objects->filter_by_last_update

It also adds a new parameter "days_inclusive"
Comment 32 Jonathan Druart 2021-04-12 14:23:57 UTC 
Created attachment 119463 [details] [review]
Bug 21549: Rename filter_by_dateexpiry with filter_by_expiration_date
Comment 33 Jonathan Druart 2021-04-12 14:24:02 UTC 
Created attachment 119464 [details] [review]
Bug 21549: Make filter_by_last_update returns a Koha::Objects-based object
Comment 34 Jonathan Druart 2021-04-12 14:24:43 UTC 
Do you agree with those 3 patches, Marcel?
Comment 35 Marcel de Rooy 2021-04-13 11:12:42 UTC 
Created attachment 119525 [details] [review]
Bug 21549: Use filter_by_last_update from filter_by_dateexpiry

To not reengineer the patch this patch does not remove the new
subroutine but make it use the logic from
Koha::Objects->filter_by_last_update

It also adds a new parameter "days_inclusive"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 36 Marcel de Rooy 2021-04-13 11:12:46 UTC 
Created attachment 119526 [details] [review]
Bug 21549: Rename filter_by_dateexpiry with filter_by_expiration_date

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 37 Marcel de Rooy 2021-04-13 11:12:50 UTC 
Created attachment 119527 [details] [review]
Bug 21549: Make filter_by_last_update returns a Koha::Objects-based object

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 38 Marcel de Rooy 2021-04-13 11:13:31 UTC 
(In reply to Jonathan Druart from comment #34)
> Do you agree with those 3 patches, Marcel?

Sure, thanks. Found a solution in the middle ;)
Comment 39 Jonathan Druart 2021-04-14 13:36:05 UTC 
Pushed to master for 21.05, thanks to everybody involved!
Comment 40 Fridolin Somers 2021-04-19 13:11:22 UTC 
Enhancement not pushed to 20.11.x