|
Description
Marcel de Rooy
2023-04-13 13:57:31 UTC
(In reply to Marcel de Rooy from comment #0) > Could we use the patron->is_active method here? See bug 33245 (wip). Yes. Bug 33245 provides a base for this. You can do $patron->is_active({ months => 6 }) or something and continue. The is_active method looks at datelastseen, enrollment date, expiry date, anonymized status, and recent holds, issues and article requests. Created attachment 151885 [details] [review] Bug 33522: Add active and inactive parameter to membership_expiry.pl Test plan: Enable TrackLastPatronActivity. Set MembershipExpiryDaysNotice to X days. Pick a patron, make it expire in X days. Add email address. Update borrowers.lastseen for this patron to make it active. Run membership_expiry.pl -c -v -active You should see at least one enqueued notice. Note: Since we passed no number, we default to 6 months. Run membership_expiry.pl -c -v -inactive You should see that it skipped a number of active patrons (>=1). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Note: bug 28688 will be based on top of this one. And will handle the case where a patron might become inactive in the time between notification moment and expiry date. If he is active at notification and if you would skip him, but becomes inactive, than he would have no notice but would not be renewed. We need an additional flag to view those patrons as inactive already at notify time. The other way around could be a bit harder. He was not active but becomes active again just before expiry. If you would auto renew, no problem. If you sent a notice to inactive ones, no problem too. So only if you skipped inactive ones, he may silently expire without a notice. Note that we also have the NotifyBorrowerDeparture warn. Since he became active again, he might have seen that one. It seems that SIP does not warn you for expiry though. Opened bug 33878 for that. If you send two expiry notices (we send a second one 7 days before), the risk of this case is even smaller. For now, I would consider this a side-effect of deciding to not inform inactive users about expiry given the other possible warning. And document it. Created attachment 151986 [details] [review] Bug 33522: Add active and inactive parameter to membership_expiry.pl Test plan: Enable TrackLastPatronActivity. Set MembershipExpiryDaysNotice to X days. Pick a patron, make it expire in X days. Add email address. Update borrowers.lastseen for this patron to make it active. Run membership_expiry.pl -c -v -active You should see at least one enqueued notice. Note: Since we passed no number, we default to 6 months. Run membership_expiry.pl -c -v -inactive You should see that it skipped a number of active patrons (>=1). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Sam Lau <samalau@gmail.com> (In reply to Sam Lau from comment #4) > Signed-off-by: Sam Lau <samalau@gmail.com> Thanks Sam Lau for testing. Did you also have a look at the test plan of the base dependency (bug 33245)? It has not been signed off yet after all? Created attachment 155164 [details] [review] Bug 33522: Add active and inactive parameter to membership_expiry.pl Test plan: Enable TrackLastPatronActivity. Set MembershipExpiryDaysNotice to X days. Pick a patron, make it expire in X days. Add email address. Update borrowers.lastseen for this patron to make it active. Run membership_expiry.pl -c -v -active 6 You should see at least one enqueued notice. Run membership_expiry.pl -c -v -inactive 6 You should see that it skipped a number of active patrons (>=1). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Sam Lau <samalau@gmail.com> Created attachment 155165 [details] [review] Bug 33522: (follow-up) Do not rely on fallback Bug 33245 has been adjusted; the fallback has been removed. We should check now if the -active or -inactive parameter actually has been passed. Test plan: See earlier test plan. Run the script without active/inactive. Run the script with active. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 156531 [details] [review] Bug 33522: Add active and inactive parameter to membership_expiry.pl Test plan: Enable TrackLastPatronActivity. Set MembershipExpiryDaysNotice to X days. Pick a patron, make it expire in X days. Add email address. Update borrowers.lastseen for this patron to make it active. Run membership_expiry.pl -c -v -active 6 You should see at least one enqueued notice. Run membership_expiry.pl -c -v -inactive 6 You should see that it skipped a number of active patrons (>=1). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 156532 [details] [review] Bug 33522: (follow-up) Do not rely on fallback Bug 33245 has been adjusted; the fallback has been removed. We should check now if the -active or -inactive parameter actually has been passed. Test plan: See earlier test plan. Run the script without active/inactive. Run the script with active. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for 23.11. Nice work everyone, thanks! Enhancement not pushed to 23.05.x |