Bug 31937

Summary: Prevent cleanup_database.pl from locking too many accounts
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: PatronsAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: gmcharlt, jonathan.druart, kyle, robin
Version: unspecified   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
22.11.00
Circulation function:
Bug Depends on: 21336    
Bug Blocks:    
Attachments: Bug 31937: Fix Patrons::search_unsubscribed - sharpen condition
k
Bug 31937: Fix Patrons::search_unsubscribed - sharpen condition
Bug 31937: Fix Patrons::search_unsubscribed - sharpen condition

Description Marcel de Rooy 2022-10-24 10:32:39 UTC Comment hidden (obsolete)
Comment 1 Marcel de Rooy 2022-10-24 13:16:27 UTC
Nothing better than being hit by your own development :)
We need to refine the condition of search_unsubscribed in cleanup_database.
I added a lot of refusals for another consent type (still customization), but they were picked up nicely by cleanup_database :)

So no more casualties expected after all..
Comment 2 Marcel de Rooy 2022-10-27 12:00:37 UTC
Created attachment 142712 [details] [review]
Bug 31937: Fix Patrons::search_unsubscribed - sharpen condition

The condition is fine as long as we only have GDPR consents.
But if you dont, you would have trouble. I know :)
Lets prevent that here, although the table still has an ENUM. So
double safety.

Test plan:
Run t/db_dependent/Koha/Patrons.t
Bonus: Run cleanup_database.pl and check for unusual number of
locked accounts:
     misc/cronjobs/cleanup_database.pl --confirm -v --sessions

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2022-10-27 12:05:06 UTC
Note for QA: I did explicitly leave this on bug, not enh. Since it does not hurt to backport it too.
Comment 4 Marcel de Rooy 2022-10-27 12:06:16 UTC
(In reply to Marcel de Rooy from comment #0)
> The first run of cleanupdatabase after the upgrade to 21.11 did indeed
> cleanup a lot ;)
> It seems that this statement
>     my $expired_patrons = Koha::Patrons->filter_by_expiration_date({ days =>
> $lock_days })->search({ login_attempts => { '!=' => -1 } });
> 
> works different than before. It must have selected patrons that had an empty
> dateexpiry in 20.11 ?

In order to be complete, this comment was not the cause. See comment1.
Comment 5 Katrin Fischer 2022-11-16 23:04:31 UTC
Created attachment 143979 [details] [review]
k
Comment 6 Katrin Fischer 2022-11-16 23:05:33 UTC
Created attachment 143980 [details] [review]
Bug 31937: Fix Patrons::search_unsubscribed - sharpen condition

The condition is fine as long as we only have GDPR consents.
But if you dont, you would have trouble. I know :)
Lets prevent that here, although the table still has an ENUM. So
double safety.

Test plan:
Run t/db_dependent/Koha/Patrons.t
Bonus: Run cleanup_database.pl and check for unusual number of
locked accounts:
     misc/cronjobs/cleanup_database.pl --confirm -v --sessions

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Marcel de Rooy 2022-11-17 06:32:25 UTC
(In reply to Katrin Fischer from comment #6)
> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Great. Thx
Comment 8 Jonathan Druart 2022-11-17 07:05:00 UTC
Created attachment 143982 [details] [review]
Bug 31937: Fix Patrons::search_unsubscribed - sharpen condition

The condition is fine as long as we only have GDPR consents.
But if you dont, you would have trouble. I know :)
Lets prevent that here, although the table still has an ENUM. So
double safety.

Test plan:
Run t/db_dependent/Koha/Patrons.t
Bonus: Run cleanup_database.pl and check for unusual number of
locked accounts:
     misc/cronjobs/cleanup_database.pl --confirm -v --sessions

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 9 Tomás Cohen Arazi (tcohen) 2022-11-17 16:42:57 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!