We have a library that marks expired borrowers as having lost their card. It would be nice to be able to upstream the cronjob they use to mark borrowers.lost = 1
Created attachment 144967 [details] [review] Bug 32560: Cronjob to mark expired borrowers as lost their card Test plan: 1. Edit a borrower to have an expiration date in the past - also make sure they have 'Lost card' set to 'No' 2. Run the set_expired_lost.pl script: sudo koha-shell <instance> cd misc/cronjobs ./set_expired_lost.pl 3. Notice you get output saying the number of borrowers are to be changed 4. View the borrower account from #1 and notice they have a 'Lost card' value of 'Yes' 5. Run the set_expired_lost.pl script again and observe no output is printed to the terminal - this is because all expired borrowers have already been marked as lost. Sponsored-by: Dalton McCaughey Library, Australia
Created attachment 145782 [details] [review] Bug 32560: Cronjob to mark expired borrowers as lost their card Test plan: 1. Edit a borrower to have an expiration date in the past - also make sure they have 'Lost card' set to 'No' 2. Run the set_expired_lost.pl script: sudo koha-shell <instance> cd misc/cronjobs ./set_expired_lost.pl 3. Notice you get output saying the number of borrowers are to be changed 4. View the borrower account from #1 and notice they have a 'Lost card' value of 'Yes' 5. Run the set_expired_lost.pl script again and observe no output is printed to the terminal - this is because all expired borrowers have already been marked as lost. Sponsored-by: Dalton McCaughey Library, Australia Signed-off-by: David Nind <david@davidnind.com>
Just a minor suggestion - rather than the output being "1 borrowers to be changed", maybe change it to "1 borrower(s) changed". The main reason is that when you run the script, the borrowers are changed.
Created attachment 145989 [details] [review] Bug 32560: (follow-up) Altered wording of output Sponsored-by: Dalton McCaughey Library, Australia
(In reply to David Nind from comment #3) > Just a minor suggestion - rather than the output being "1 borrowers to be > changed", maybe change it to "1 borrower(s) changed". > > The main reason is that when you run the script, the borrowers are changed. Hi David, Great suggestion, thank you, and thanks for testing too! I have attached a follow-up patch with that new wording. Cheers, Alex
Just setting this back to 'Needs signoff' so you can hopefully test and signoff the additional patch. Thanks so much
Created attachment 146031 [details] [review] Bug 32560: Cronjob to mark expired borrowers as lost their card Test plan: 1. Edit a borrower to have an expiration date in the past - also make sure they have 'Lost card' set to 'No' 2. Run the set_expired_lost.pl script: sudo koha-shell <instance> cd misc/cronjobs ./set_expired_lost.pl 3. Notice you get output saying the number of borrowers are to be changed 4. View the borrower account from #1 and notice they have a 'Lost card' value of 'Yes' 5. Run the set_expired_lost.pl script again and observe no output is printed to the terminal - this is because all expired borrowers have already been marked as lost. Sponsored-by: Dalton McCaughey Library, Australia Signed-off-by: David Nind <david@davidnind.com>
Created attachment 146032 [details] [review] Bug 32560: (follow-up) Altered wording of output Sponsored-by: Dalton McCaughey Library, Australia Signed-off-by: David Nind <david@davidnind.com>
I would love to see the raw SQL replaces with a Koha::Patrons search and update.
Created attachment 147814 [details] [review] Bug 32560: (follow-up) Replace raw SQL queries with Koha::Patrons search and update Also: - Add use Koha::Script -cron; - Add license - Add optional verbose flag - Add cronjob logging Test plan: 1. Set 'CronjobLog' syspref = 'Log' 2. Edit a borrower to have an expiration date in the past - also make sure they have 'Lost card' set to 'No' 3. Run the set_expired_lost.pl script: sudo koha-shell <instance> cd misc/cronjobs ./set_expired_lost.pl 4. Observe no output is displayed but viewing the borrower account from step #1 they now have a 'Lost card' value of 'Yes'. 5. Change the borrower account 'Lost card' back to 'No' 6. Run the set_expired_lost.pl script again, this time with the verbose flag: sudo koha-shell <instance> cd misc/cronjobs ./set_expired_lost.pl -v 7. Observe output is displayed to the commandline showing the number of borrowers that have been changed. Check the borrower account again and notice the 'Lost card' has been set back to 'Yes'. 8. Repeat step #6 and observe no output is printed to the terminal - this is because all expired borrowers have already been marked as lost. 9. Check there are log entries for set_expired_lost.pl cronjob in the action_logs table: sudo koha-mysql <instance> select * from action_logs where script='set_expired_lost.pl'; Sponsored-by: Dalton McCaughey Library, Australia
(In reply to Kyle M Hall from comment #9) > I would love to see the raw SQL replaces with a Koha::Patrons search and > update. Hi Kyle, I've just attached a follow-up patch with those changes and several other improvements. Thanks, Alex
Created attachment 154222 [details] [review] Bug 32560: Cronjob to mark expired borrowers as lost their card Test plan: 1. Edit a borrower to have an expiration date in the past - also make sure they have 'Lost card' set to 'No' 2. Run the set_expired_lost.pl script: sudo koha-shell <instance> cd misc/cronjobs ./set_expired_lost.pl 3. Notice you get output saying the number of borrowers are to be changed 4. View the borrower account from #1 and notice they have a 'Lost card' value of 'Yes' 5. Run the set_expired_lost.pl script again and observe no output is printed to the terminal - this is because all expired borrowers have already been marked as lost. Sponsored-by: Dalton McCaughey Library, Australia Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Sam Lau <samalau@gmail.com>
Created attachment 154223 [details] [review] Bug 32560: (follow-up) Altered wording of output Sponsored-by: Dalton McCaughey Library, Australia Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Sam Lau <samalau@gmail.com>
Created attachment 154224 [details] [review] Bug 32560: (follow-up) Replace raw SQL queries with Koha::Patrons search and update Also: - Add use Koha::Script -cron; - Add license - Add optional verbose flag - Add cronjob logging Test plan: 1. Set 'CronjobLog' syspref = 'Log' 2. Edit a borrower to have an expiration date in the past - also make sure they have 'Lost card' set to 'No' 3. Run the set_expired_lost.pl script: sudo koha-shell <instance> cd misc/cronjobs ./set_expired_lost.pl 4. Observe no output is displayed but viewing the borrower account from step #1 they now have a 'Lost card' value of 'Yes'. 5. Change the borrower account 'Lost card' back to 'No' 6. Run the set_expired_lost.pl script again, this time with the verbose flag: sudo koha-shell <instance> cd misc/cronjobs ./set_expired_lost.pl -v 7. Observe output is displayed to the commandline showing the number of borrowers that have been changed. Check the borrower account again and notice the 'Lost card' has been set back to 'Yes'. 8. Repeat step #6 and observe no output is printed to the terminal - this is because all expired borrowers have already been marked as lost. 9. Check there are log entries for set_expired_lost.pl cronjob in the action_logs table: sudo koha-mysql <instance> select * from action_logs where script='set_expired_lost.pl'; Sponsored-by: Dalton McCaughey Library, Australia Signed-off-by: Sam Lau <samalau@gmail.com>
This feels like a workaround for a specific use case or to achieve a certain behavior. We do have other means to lock a patron account now that is expired, either by using system preferences to block OPAC actions (BlockExpiredPatronOpacActions) or using LockExpiredDelay and cleanup_database.pl. Maybe one of these would achieve the wanted behaviour as well? You can now also create custom patron restrictions. I feel using one of those would also be a bit cleaner than using lost and give a less confusing message in the OPAC.
Please give feedback on last comment.
Hi Katrin and Marcel, My apologies for my late response, I am catching back up now I am back from leave in Italy. Yes I agree with you Katrin the suggestions you have noted are a cleaner option than this enhancement. That being the case I am going to close this bug report. Thank you both for your help. Alex