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 :)
Created attachment 44643 [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 :)
Failing QA because of no integration tests
Created attachment 44645 [details] [review] Bug 15157 - Cronjob to automatically debar Borrowers/Patrons with pending/unpaid fines/accountlines
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.
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
The bug this is depending on is in "Failed QA" - moving this one to BLOCKED.
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.
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
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.
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.
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.
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
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.
I see that this is still assigned to Olli, but has follow-ups from Emmi and Martin. Is someone currently working on this?
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>
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
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.
Created attachment 129704 [details] [review] Bug 15157: (follow-up) Add ability to set lower limit
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.
(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
(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.
(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'...
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
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>
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>
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>
Created attachment 158291 [details] [review] Bug 15157: (follow-up) Add ability to set lower limit Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
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>
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.
Looking here
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>
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>
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>
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>
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.
(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 :)
Pushed to master for 23.11. Nice work everyone, thanks!
Enhancement not pushed to 23.05.x