Bug 15157 - Cronjob to automatically restrict patrons with pending/unpaid charges
Summary: Cronjob to automatically restrict patrons with pending/unpaid charges
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Marcel de Rooy
URL:
Keywords: release-notes-needed
Depends on: 15156
Blocks:
  Show dependency treegraph
 
Reported: 2015-11-09 11:02 UTC by Olli-Antti Kivilahti
Modified: 2023-11-11 09:24 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00


Attachments
Bug 15157 - Cronjob to automatically debar Borrowers/Patrons with pending/unpaid fines/accountlines (2.69 KB, patch)
2015-11-09 11:19 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 15157 - Cronjob to automatically debar Borrowers/Patrons with pending/unpaid fines/accountlines (2.69 KB, patch)
2015-11-09 11:41 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 15157 - Cronjob to automatically debar Borrowers/Patrons with pending/unpaid fines/accountlines (3.35 KB, patch)
2015-11-09 12:49 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 15157: Update script (1.69 KB, patch)
2020-08-27 10:06 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 15157 - Cronjob to automatically debar Borrowers/Patrons with pending/unpaid fines/accountlines (3.35 KB, patch)
2021-09-15 14:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15157: Update script (1.69 KB, patch)
2021-09-15 14:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15157: Update for reworked bug 15156 (1.59 KB, patch)
2021-09-15 14:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15157: Cronjob to debar Patrons with unpaid accountlines (3.37 KB, patch)
2022-01-21 16:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15157: Update script (1.69 KB, patch)
2022-01-21 16:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15157: Modernise and Update for bug 15156 (6.68 KB, patch)
2022-01-21 16:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15157: (follow-up) Add ability to set lower limit (1.49 KB, patch)
2022-01-21 16:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 15157: Add my $amount and fix options (1.20 KB, patch)
2022-02-02 13:34 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 15157: Cronjob to debar Patrons with unpaid accountlines (3.43 KB, patch)
2023-11-02 21:28 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 15157: Update script (1.75 KB, patch)
2023-11-02 21:28 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 15157: Modernise and Update for bug 15156 (6.73 KB, patch)
2023-11-02 21:28 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 15157: (follow-up) Add ability to set lower limit (1.54 KB, patch)
2023-11-02 21:28 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 15157: Add my $amount and fix options (1.25 KB, patch)
2023-11-02 21:28 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 15157: Cronjob to debar Patrons with unpaid accountlines (4.90 KB, patch)
2023-11-03 10:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15157: (follow-up) Add ability to set lower limit (1.64 KB, patch)
2023-11-03 10:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15157: Add my $amount and fix options (1.38 KB, patch)
2023-11-03 10:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15157: (QA follow-up) Some more changes (3.38 KB, patch)
2023-11-03 10:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Olli-Antti Kivilahti 2015-11-09 11:02:32 UTC
If your library wants to debar all Borrower who haven't paid their fines by the end of the year, this script will do that trick :)
Comment 1 Olli-Antti Kivilahti 2015-11-09 11:19:47 UTC Comment hidden (obsolete)
Comment 2 Olli-Antti Kivilahti 2015-11-09 11:20:15 UTC
Failing QA because of no integration tests
Comment 3 Olli-Antti Kivilahti 2015-11-09 11:41:35 UTC Comment hidden (obsolete)
Comment 4 Olli-Antti Kivilahti 2015-11-09 12:49:23 UTC
Created attachment 44646 [details] [review]
Bug 15157 - Cronjob to automatically debar Borrowers/Patrons with pending/unpaid fines/accountlines

If your library wants to debar all Borrower who haven't paid their fines by the
end of the year, this script will do that trick :)

You can give the message from a file if the cronjob runner doesn't deal with
quotes,
or as a command-line parameter.
Comment 5 Emmi Takkinen 2020-08-27 10:06:24 UTC
Created attachment 109220 [details] [review]
Bug 15157: Update script

This patch updates debarrBorrowersWithFines.pl script to match
changes made in bug 15156.

To test:
1. Have patron(s) with unpaid fines
2. Run e.g debarrBorrowersWithFines.pl --confirm -m "This is a description of you bad deeds"
(test other options too)
3. Confirm patron(s) with fines has been debarred with the explanation

Sponsored-by: Koha-Suomi Oy
Comment 6 Katrin Fischer 2020-09-13 10:28:39 UTC
The bug this is depending on is in "Failed QA" - moving this one to BLOCKED.
Comment 7 Martin Renvoize 2021-09-15 14:28:59 UTC
Created attachment 124894 [details] [review]
Bug 15157 - Cronjob to automatically debar Borrowers/Patrons with pending/unpaid fines/accountlines

If your library wants to debar all Borrower who haven't paid their fines by the
end of the year, this script will do that trick :)

You can give the message from a file if the cronjob runner doesn't deal with
quotes,
or as a command-line parameter.
Comment 8 Martin Renvoize 2021-09-15 14:29:03 UTC
Created attachment 124895 [details] [review]
Bug 15157: Update script

This patch updates debarrBorrowersWithFines.pl script to match
changes made in bug 15156.

To test:
1. Have patron(s) with unpaid fines
2. Run e.g debarrBorrowersWithFines.pl --confirm -m "This is a description of you bad deeds"
(test other options too)
3. Confirm patron(s) with fines has been debarred with the explanation

Sponsored-by: Koha-Suomi Oy
Comment 9 Martin Renvoize 2021-09-15 14:29:07 UTC
Created attachment 124896 [details] [review]
Bug 15157: Update for reworked bug 15156

This patch updates the script to use filter_by_amount_owed.

We could add a series of options to the script to allow more fine
grained control.
Comment 10 Martin Renvoize 2021-09-15 14:30:30 UTC
Updated to use the Signed off alternative posted to bug 15156.

What is 'MAD' for the --confirm option.. that confused me whilst testing.

Otherwise, this generally works as expected.. I'd love to see the filter options introduced in bug 15156 utilised in this script allowing for debarment threshold amounts for example.
Comment 11 David Cook 2021-09-16 06:47:58 UTC
This should be wrapped in a system preference, so that you can enable the cronjob for a server, but only implement it for selective Koha instances on that server.
Comment 12 Fridolin Somers 2021-12-11 06:04:28 UTC
I think we should rename the script debarrBorrowersWithFines.pl.
Essentially we use 'patron' now instead of 'borrowers', like misc/import_patrons.pl.
So maybe debarr_patrons_with_fines.pl
Comment 13 Katrin Fischer 2021-12-12 13:37:17 UTC
1) I think debar_patrons_with_fines.pl would be great. Note r vs rr here. :)

2) I am not sure about this requirement here:
+    print "\nYour --message is too short. A proper message to your end-users must be longer than 20 characters.\n";

Depending on language this might be a strange requirement. For example Chinese/Japanese tend to have much shorter texts because of the use of signs.

3) I think David is right about having a system preference for this to make it easier to use on multi-instance systems, but I wonder if this should be a hard requirement. One advantage of doing it right from the beginning is that it's much easier than introducing later when you might break/change peoples workflows. 

But I am not sure if a lot of instances would use that in the same way, it looks more like a 'once a year' type of thing then a very regular cronjob. So for me this would not be a blocker right now.
Comment 14 Katrin Fischer 2021-12-12 13:52:51 UTC
I see that this is still assigned to Olli, but has follow-ups from Emmi and Martin. Is someone currently working on this?
Comment 15 Martin Renvoize 2022-01-21 16:28:24 UTC
Created attachment 129701 [details] [review]
Bug 15157: Cronjob to debar Patrons with unpaid accountlines

If your library wants to debar all Borrower who haven't paid
their fines by the end of the year, this script will do that
trick :)

You can give the message from a file if the cronjob runner
doesn't deal with quotes, or as a command-line parameter.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2022-01-21 16:28:27 UTC
Created attachment 129702 [details] [review]
Bug 15157: Update script

This patch updates debarrBorrowersWithFines.pl script to match
changes made in bug 15156.

To test:
1. Have patron(s) with unpaid fines
2. Run e.g debarrBorrowersWithFines.pl --confirm -m "This is a description of you bad deeds"
(test other options too)
3. Confirm patron(s) with fines has been debarred with the explanation

Sponsored-by: Koha-Suomi Oy
Comment 17 Martin Renvoize 2022-01-21 16:28:31 UTC
Created attachment 129703 [details] [review]
Bug 15157: Modernise and Update for bug 15156

This patch updates the script to use filter_by_amount_owed, renames
it to debar_patrons_with_fines.pl and moves it to the cronjobs
directory whilst also adding a copyright notice and POD.

We could add a series of options to the script to allow more fine
grained control.
Comment 18 Martin Renvoize 2022-01-21 16:28:35 UTC
Created attachment 129704 [details] [review]
Bug 15157: (follow-up) Add ability to set lower limit
Comment 19 Martin Renvoize 2022-01-21 16:30:58 UTC
I started working through the feedback here, but ran out of tuits.

I think we should probably use system preferences instead of passing parameters in the script call as David suggests.. I'm also not sure at this point how cron vs background tasks are working.. this is really designed as an 'auto debar users' feature I think?

This wasn't ever something I intended to work on, nor have anyone using.. so I'm not really sure where it should be going.. I just picked it up to modernise the code and help things to move forwards.  Any feedback welcome.
Comment 20 Emmi Takkinen 2022-02-02 12:02:48 UTC
(In reply to Katrin Fischer from comment #13)

> 3) I think David is right about having a system preference for this to make
> it easier to use on multi-instance systems, but I wonder if this should be a
> hard requirement. One advantage of doing it right from the beginning is that
> it's much easier than introducing later when you might break/change peoples
> workflows. 
> 
> But I am not sure if a lot of instances would use that in the same way, it
> looks more like a 'once a year' type of thing then a very regular cronjob.
> So for me this would not be a blocker right now.

This is indeed 'once a year' cronjob. Some of our instances have a policy to set blocks to patrons whose fines are over accepted limit after New Year. Setting this as system preference and be triggered every time someone exceeds that limit would go against these policies. Patrons still need some respite to pay their fines :D
Comment 21 Emmi Takkinen 2022-02-02 13:19:03 UTC
(In reply to Martin Renvoize from comment #19)
> I started working through the feedback here, but ran out of tuits.
> 
> I think we should probably use system preferences instead of passing
> parameters in the script call as David suggests.. I'm also not sure at this
> point how cron vs background tasks are working.. this is really designed as
> an 'auto debar users' feature I think?
> 
> This wasn't ever something I intended to work on, nor have anyone using.. so
> I'm not really sure where it should be going.. I just picked it up to
> modernise the code and help things to move forwards.  Any feedback welcome.

Thanks for picking this up Martin! I noticed couple quick fixes, there's no my $amount declared and also --confirm option no longer needs string. And for some reason amount limit is not working.
Comment 22 Emmi Takkinen 2022-02-02 13:22:25 UTC
(In reply to Emmi Takkinen from comment #21)
> (In reply to Martin Renvoize from comment #19)
> > I started working through the feedback here, but ran out of tuits.
> > 
> > I think we should probably use system preferences instead of passing
> > parameters in the script call as David suggests.. I'm also not sure at this
> > point how cron vs background tasks are working.. this is really designed as
> > an 'auto debar users' feature I think?
> > 
> > This wasn't ever something I intended to work on, nor have anyone using.. so
> > I'm not really sure where it should be going.. I just picked it up to
> > modernise the code and help things to move forwards.  Any feedback welcome.
> 
> Thanks for picking this up Martin! I noticed couple quick fixes, there's no
> my $amount declared and also --confirm option no longer needs string. And
> for some reason amount limit is not working.

Oh, it needs to be 'a|amount:i'...
Comment 23 Emmi Takkinen 2022-02-02 13:34:39 UTC
Created attachment 130103 [details] [review]
Bug 15157: Add my $amount and fix options

Variable my $amount was missing. Also option
amount has to be declared as integer and
option confirm is no longer string.

Sponsored by: Koha-Suomi Oy
Comment 24 Lucas Gass 2023-11-02 21:28:03 UTC
Created attachment 158288 [details] [review]
Bug 15157: Cronjob to debar Patrons with unpaid accountlines

If your library wants to debar all Borrower who haven't paid
their fines by the end of the year, this script will do that
trick :)

You can give the message from a file if the cronjob runner
doesn't deal with quotes, or as a command-line parameter.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 25 Lucas Gass 2023-11-02 21:28:06 UTC
Created attachment 158289 [details] [review]
Bug 15157: Update script

This patch updates debarrBorrowersWithFines.pl script to match
changes made in bug 15156.

To test:
1. Have patron(s) with unpaid fines
2. Run e.g debarrBorrowersWithFines.pl --confirm -m "This is a description of you bad deeds"
(test other options too)
3. Confirm patron(s) with fines has been debarred with the explanation

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 26 Lucas Gass 2023-11-02 21:28:08 UTC
Created attachment 158290 [details] [review]
Bug 15157: Modernise and Update for bug 15156

This patch updates the script to use filter_by_amount_owed, renames
it to debar_patrons_with_fines.pl and moves it to the cronjobs
directory whilst also adding a copyright notice and POD.

We could add a series of options to the script to allow more fine
grained control.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 27 Lucas Gass 2023-11-02 21:28:10 UTC
Created attachment 158291 [details] [review]
Bug 15157: (follow-up) Add ability to set lower limit

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 28 Lucas Gass 2023-11-02 21:28:13 UTC
Created attachment 158292 [details] [review]
Bug 15157: Add my $amount and fix options

Variable my $amount was missing. Also option
amount has to be declared as integer and
option confirm is no longer string.

Sponsored by: Koha-Suomi Oy

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 29 Lucas Gass 2023-11-02 21:29:31 UTC
This works well so I am signing off. In the future it would be nice to have a verbose mode and the ability to limit by branch. But as is this is good and should not have been 2 years in NSO.
Comment 30 Marcel de Rooy 2023-11-03 08:54:51 UTC
Looking here
Comment 31 Marcel de Rooy 2023-11-03 10:06:02 UTC
Created attachment 158315 [details] [review]
Bug 15157: Cronjob to debar Patrons with unpaid accountlines

If your library wants to debar all Borrower who haven't paid
their fines by the end of the year, this script will do that
trick :)

You can give the message from a file if the cronjob runner
doesn't deal with quotes, or as a command-line parameter.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 15157: Update script

This patch updates debarrBorrowersWithFines.pl script to match
changes made in bug 15156.

To test:
1. Have patron(s) with unpaid fines
2. Run e.g debarrBorrowersWithFines.pl --confirm -m "This is a description of you bad deeds"
(test other options too)
3. Confirm patron(s) with fines has been debarred with the explanation

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 15157: Modernise and Update for bug 15156

This patch updates the script to use filter_by_amount_owed, renames
it to debar_patrons_with_fines.pl and moves it to the cronjobs
directory whilst also adding a copyright notice and POD.

We could add a series of options to the script to allow more fine
grained control.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

[EDIT] Run perltidy to resolve three lines.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 32 Marcel de Rooy 2023-11-03 10:06:05 UTC
Created attachment 158316 [details] [review]
Bug 15157: (follow-up) Add ability to set lower limit

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 33 Marcel de Rooy 2023-11-03 10:06:08 UTC
Created attachment 158317 [details] [review]
Bug 15157: Add my $amount and fix options

Variable my $amount was missing. Also option
amount has to be declared as integer and
option confirm is no longer string.

Sponsored by: Koha-Suomi Oy

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

[EDIT] Run perltidy to resolve one line.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 34 Marcel de Rooy 2023-11-03 10:06:10 UTC
Created attachment 158318 [details] [review]
Bug 15157: (QA follow-up) Some more changes

Renamed some vars. Removed unused $success.
Extended die statement.
Added Modern::Perl.

Added verbose parameter.
Added some logic around the confirm parameter. Making it possible
to run the script in 'dry' mode.
Print totals at the end.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 35 Marcel de Rooy 2023-11-03 10:07:06 UTC
QA Comment:
Squashed first 3 patches: Resolving: FAIL git manipulation The file has been added and deleted in the same patchset.
Tidied a bit more.
Follow-up: Renamed some vars. Removed unused $success. Extended die statement. Added Modern::Perl.

FIXED Serious omission: $confirm DOES NOT DO anything ! Blocker
TODO The expiration parameter is not tested in any way? Does not feel good. (No definitive blocker for me.)
TODO If we added a debarment and run the script again, we will add another debarment. Please add a check. (Doubt if this is a blocker.)
TODO We need a manual option like other scripts? No blocker for me.
DONE We need a verbose parameter as Lucas mentioned.
DONE If we do not confirm, I would expect to list patron info in verbose mode.
DONE I would add a count at the end. May be no blocker, but generally expected.

Please still add a follow-up or leave final decision to RM.
Comment 36 Marcel de Rooy 2023-11-03 10:07:51 UTC
(In reply to Marcel de Rooy from comment #35)
> FIXED Serious omission: $confirm DOES NOT DO anything ! Blocker
Well, not what I expected at least :)
Comment 37 Tomás Cohen Arazi 2023-11-08 14:42:13 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 38 Fridolin Somers 2023-11-11 01:09:08 UTC
Enhancement not pushed to 23.05.x