Bug 30300 - Add patron expiration email to patron messaging preferences
Summary: Add patron expiration email to patron messaging preferences
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Matt Blenkinsop
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords: additional_work_needed, release-notes-needed
Depends on:
Blocks: 30301
  Show dependency treegraph
 
Reported: 2022-03-15 18:49 UTC by Nick Clemens (kidclamp)
Modified: 2025-02-03 13:29 UTC (History)
5 users (show)

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


Attachments
Bug 30300: Add a patron expiry messaging preference (7.24 KB, patch)
2024-07-30 13:11 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 30300: Update the cronjob to respect patron messaging preferences (4.78 KB, patch)
2024-07-30 13:11 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 30300: Add a patron expiry messaging preference (7.27 KB, patch)
2024-07-30 17:32 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 30300: Update the cronjob to respect patron messaging preferences (4.82 KB, patch)
2024-07-30 17:32 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 30300: Update the cronjob to respect patron messaging preferences (4.78 KB, patch)
2024-10-31 16:12 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 30300: (QA follow-up) Move logic to queue_notice (2.02 KB, patch)
2024-10-31 16:12 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 30300: (QA follow-up) Move logic to queue_notice (2.94 KB, patch)
2024-10-31 16:15 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 30300: Add a patron expiry messaging preference (7.33 KB, patch)
2024-10-31 16:57 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30300: Update the cronjob to respect patron messaging preferences (4.82 KB, patch)
2024-10-31 16:57 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30300: (QA follow-up) Move logic to queue_notice (2.98 KB, patch)
2024-10-31 16:57 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30300: (QA follow-up) Remove extra return (989 bytes, patch)
2024-10-31 16:57 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30300: Add a patron expiry messaging preference (7.35 KB, patch)
2024-11-18 15:01 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 30300: Update the cronjob to respect patron messaging preferences (4.84 KB, patch)
2024-11-18 15:01 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 30300: (QA follow-up) Move logic to queue_notice (2.99 KB, patch)
2024-11-18 15:01 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 30300: (QA follow-up) Remove extra return (1005 bytes, patch)
2024-11-18 15:01 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 30300: (QA follow-up) Prevent regression and improve clarity (5.58 KB, patch)
2024-11-18 15:01 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 30300: (QA follow-up) Prevent regression and improve clarity (5.27 KB, patch)
2024-11-21 14:46 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 30300: Add a patron expiry messaging preference (7.40 KB, patch)
2025-01-06 15:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30300: Update the cronjob to respect patron messaging preferences (4.89 KB, patch)
2025-01-06 15:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30300: (QA follow-up) Move logic to queue_notice (3.04 KB, patch)
2025-01-06 15:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30300: (QA follow-up) Remove extra return (1.03 KB, patch)
2025-01-06 15:28 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30300: (QA follow-up) Prevent regression and improve clarity (5.32 KB, patch)
2025-01-06 15:28 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30300: (QA follow-up) Remove use of Koha:: and C4:: in DB Update (3.13 KB, patch)
2025-01-06 15:28 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2022-03-15 18:49:49 UTC
This enhancement will add a messaging preference for patron expiration notices, and adjust the cronjob that sends these notices to obey the patrons preference
Comment 1 Matt Blenkinsop 2024-07-30 13:11:39 UTC
Created attachment 169860 [details] [review]
Bug 30300: Add a patron expiry messaging preference
Comment 2 Matt Blenkinsop 2024-07-30 13:11:41 UTC
Created attachment 169861 [details] [review]
Bug 30300: Update the cronjob to respect patron messaging preferences

Test plan:
1) Apply patch and run updatedatabase
2) In system preferences set MembershipExpiryDaysNotice to 5
3) Choose a patron and edit their expiry date to today's date
4) Ensure that patron has a primary email address
5) Run perl misc/cronjobs/membership_expiry.pl -c -v -before 10 -after 10
6) Check the patron's messages, there should be no expiry notice queued
7) Edit the patron's messaging preferences and select the checkbox for email in the row for 'Patron expiry'
8) Repeat step 5
9) There should be a patron expiry message in the patron's message queue
10) Sign off!
Comment 3 Roman Dolny 2024-07-30 17:32:03 UTC
Created attachment 169870 [details] [review]
Bug 30300: Add a patron expiry messaging preference

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 4 Roman Dolny 2024-07-30 17:32:05 UTC
Created attachment 169871 [details] [review]
Bug 30300: Update the cronjob to respect patron messaging preferences

Test plan:
1) Apply patch and run updatedatabase
2) In system preferences set MembershipExpiryDaysNotice to 5
3) Choose a patron and edit their expiry date to today's date
4) Ensure that patron has a primary email address
5) Run perl misc/cronjobs/membership_expiry.pl -c -v -before 10 -after 10
6) Check the patron's messages, there should be no expiry notice queued
7) Edit the patron's messaging preferences and select the checkbox for email in the row for 'Patron expiry'
8) Repeat step 5
9) There should be a patron expiry message in the patron's message queue
10) Sign off!

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 5 Katrin Fischer 2024-07-31 09:27:23 UTC
I think a lot of libraries currently use the "where" parameter of this script to limit to certain patron categories. I think it's a good idea to add to messaging preferences, but will this still work with the patch?
Comment 6 Matt Blenkinsop 2024-07-31 09:45:46 UTC
It would do, the script fetches patrons based on the "where" option and then loops through them which is where the patron messaging preferences stuff is handled
Comment 7 Katrin Fischer 2024-07-31 11:21:31 UTC
(In reply to Matt Blenkinsop from comment #6)
> It would do, the script fetches patrons based on the "where" option and then
> loops through them which is where the patron messaging preferences stuff is
> handled

+1 Thanks!
Comment 8 Nick Clemens (kidclamp) 2024-10-31 14:27:26 UTC
If I read this correctly, before this patch all expiring patrons selected by the job would receive a notice. Afterwards, no one will receive a notice until they set messaging preferences, so libraries will need to add a preference for all patrons, should we do this in the update? or simply note in the release notes?

We get the messaging preferences here, but queue_notice also does - do we need to do this? Or can we simply let queue_notice decide what/if to send?
Comment 9 Nick Clemens (kidclamp) 2024-10-31 14:31:19 UTC
Also, the user won't get a membership renewal notice if they don't want expiry notices? Is that correct?
Comment 10 Matt Blenkinsop 2024-10-31 16:12:31 UTC
Created attachment 173810 [details] [review]
Bug 30300: Update the cronjob to respect patron messaging preferences

Test plan:
1) Apply patch and run updatedatabase
2) In system preferences set MembershipExpiryDaysNotice to 5
3) Choose a patron and edit their expiry date to today's date
4) Ensure that patron has a primary email address
5) Run perl misc/cronjobs/membership_expiry.pl -c -v -before 10 -after 10
6) Check the patron's messages, there should be no expiry notice queued
7) Edit the patron's messaging preferences and select the checkbox for email in the row for 'Patron expiry'
8) Repeat step 5
9) There should be a patron expiry message in the patron's message queue
10) Sign off!
Comment 11 Matt Blenkinsop 2024-10-31 16:12:46 UTC
Created attachment 173811 [details] [review]
Bug 30300: (QA follow-up) Move logic to queue_notice
Comment 12 Matt Blenkinsop 2024-10-31 16:15:37 UTC
Created attachment 173812 [details] [review]
Bug 30300: (QA follow-up) Move logic to queue_notice
Comment 13 Matt Blenkinsop 2024-10-31 16:18:00 UTC
(In reply to Nick Clemens (kidclamp) from comment #8)
> If I read this correctly, before this patch all expiring patrons selected by
> the job would receive a notice. Afterwards, no one will receive a notice
> until they set messaging preferences, so libraries will need to add a
> preference for all patrons, should we do this in the update? or simply note
> in the release notes?

I'm not sure what the preference would be for libraries? Having it set by default or wanting it to be set explicitly
Comment 14 Nick Clemens (kidclamp) 2024-10-31 16:57:06 UTC
Created attachment 173820 [details] [review]
Bug 30300: Add a patron expiry messaging preference

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 15 Nick Clemens (kidclamp) 2024-10-31 16:57:08 UTC
Created attachment 173821 [details] [review]
Bug 30300: Update the cronjob to respect patron messaging preferences

Test plan:
1) Apply patch and run updatedatabase
2) In system preferences set MembershipExpiryDaysNotice to 5
3) Choose a patron and edit their expiry date to today's date
4) Ensure that patron has a primary email address
5) Run perl misc/cronjobs/membership_expiry.pl -c -v -before 10 -after 10
6) Check the patron's messages, there should be no expiry notice queued
7) Edit the patron's messaging preferences and select the checkbox for email in the row for 'Patron expiry'
8) Repeat step 5
9) There should be a patron expiry message in the patron's message queue
10) Sign off!

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Nick Clemens (kidclamp) 2024-10-31 16:57:10 UTC
Created attachment 173822 [details] [review]
Bug 30300: (QA follow-up) Move logic to queue_notice

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Nick Clemens (kidclamp) 2024-10-31 16:57:13 UTC
Created attachment 173823 [details] [review]
Bug 30300: (QA follow-up) Remove extra return

The routine loops on the found transports, if there are none it will simply return, no need to short circuit

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 18 Nick Clemens (kidclamp) 2024-10-31 17:00:05 UTC
(In reply to Matt Blenkinsop from comment #13)
> (In reply to Nick Clemens (kidclamp) from comment #8)
> > If I read this correctly, before this patch all expiring patrons selected by
> > the job would receive a notice. Afterwards, no one will receive a notice
> > until they set messaging preferences, so libraries will need to add a
> > preference for all patrons, should we do this in the update? or simply note
> > in the release notes?
> 
> I'm not sure what the preference would be for libraries? Having it set by
> default or wanting it to be set explicitly

We have the pref "AutoRenewlNotices" for automatic renewals - but it's a little crufty

Leaving this for RM decisions
Comment 19 Katrin Fischer 2024-11-11 10:53:53 UTC
I struggle a bit with this one in terms of having no regressions/confusion for libraries and I think this needs more work.

1) Regression (blocker)

After updating, no MEMBERSHIP_EXPIRY notices will be sent as no patrons will have the flag in their patron accounts, even if it worked before.

I don't have a good solution here. Maybe the best we could do is globally add the flag if MembershipExpiryDaysNotice  is set to a value >= 1? I believe together with the where it would keep the current behavior?

2) Documentation (not blocker, but easy)

It's not clear from the GUI that a "where" option might overwrite the setting in the patron account.

I could have a patron with checked patron expiry notice, that is then "removed" from the list by a where parameter. I think at least a note on the system preference could help here to fight off general confusion.

3) Visibility of configuration option (blocker)

I feel the visiblity of the messaging preference should be dependent on the  MembershipExpiryDaysNotice system preference so libraries have the option to easily turn this feature off or "keep it invisible" after update.

I hope I was not missing something!
Comment 20 Matt Blenkinsop 2024-11-18 15:01:41 UTC
Created attachment 174713 [details] [review]
Bug 30300: Add a patron expiry messaging preference

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 21 Matt Blenkinsop 2024-11-18 15:01:44 UTC
Created attachment 174714 [details] [review]
Bug 30300: Update the cronjob to respect patron messaging preferences

Test plan:
1) Apply patch and run updatedatabase
2) In system preferences set MembershipExpiryDaysNotice to 5
3) Choose a patron and edit their expiry date to today's date
4) Ensure that patron has a primary email address
5) Run perl misc/cronjobs/membership_expiry.pl -c -v -before 10 -after 10
6) Check the patron's messages, there should be no expiry notice queued
7) Edit the patron's messaging preferences and select the checkbox for email in the row for 'Patron expiry'
8) Repeat step 5
9) There should be a patron expiry message in the patron's message queue
10) Sign off!

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 22 Matt Blenkinsop 2024-11-18 15:01:47 UTC
Created attachment 174715 [details] [review]
Bug 30300: (QA follow-up) Move logic to queue_notice

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 23 Matt Blenkinsop 2024-11-18 15:01:50 UTC
Created attachment 174716 [details] [review]
Bug 30300: (QA follow-up) Remove extra return

The routine loops on the found transports, if there are none it will simply return, no need to short circuit

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 24 Matt Blenkinsop 2024-11-18 15:01:53 UTC
Created attachment 174717 [details] [review]
Bug 30300: (QA follow-up) Prevent regression and improve clarity

This patch amends the DB update to check the MembershipExpiryDaysNotice syspref and sets the flag for all patrons if the syspref is set

It also improves the UI visiblity by hiding the messaging preference if the syspref isn't set and adding more clarity to the syspref definition
Comment 25 Matt Blenkinsop 2024-11-18 15:04:02 UTC
(In reply to Katrin Fischer from comment #19)

> 1) Regression (blocker)
> 
> After updating, no MEMBERSHIP_EXPIRY notices will be sent as no patrons will
> have the flag in their patron accounts, even if it worked before.
> 
> I don't have a good solution here. Maybe the best we could do is globally
> add the flag if MembershipExpiryDaysNotice  is set to a value >= 1? I
> believe together with the where it would keep the current behavior?

I've implemented the above suggestion in the database update file and I've set the bug back to SO so that this can be properly QA checked. This could be quite an intensive update on systems with large numbers of patrons

 
> 2) Documentation (not blocker, but easy)
> 
> It's not clear from the GUI that a "where" option might overwrite the
> setting in the patron account.
> 
> I could have a patron with checked patron expiry notice, that is then
> "removed" from the list by a where parameter. I think at least a note on the
> system preference could help here to fight off general confusion.

Added a note to the syspref

 
> 3) Visibility of configuration option (blocker)
> 
> I feel the visiblity of the messaging preference should be dependent on the 
> MembershipExpiryDaysNotice system preference so libraries have the option to
> easily turn this feature off or "keep it invisible" after update.

The line in the table is now hidden if the syspref isn't switched on
Comment 26 Matt Blenkinsop 2024-11-21 14:46:33 UTC
Created attachment 174888 [details] [review]
Bug 30300: (QA follow-up) Prevent regression and improve clarity

This patch amends the DB update to check the MembershipExpiryDaysNotice syspref and sets the flag for all patrons if the syspref is set

It also improves the UI visiblity by hiding the messaging preference if the syspref isn't set and adding more clarity to the syspref definition
Comment 27 Martin Renvoize (ashimema) 2025-01-06 15:27:52 UTC
Created attachment 176154 [details] [review]
Bug 30300: Add a patron expiry messaging preference

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize (ashimema) 2025-01-06 15:27:55 UTC
Created attachment 176155 [details] [review]
Bug 30300: Update the cronjob to respect patron messaging preferences

Test plan:
1) Apply patch and run updatedatabase
2) In system preferences set MembershipExpiryDaysNotice to 5
3) Choose a patron and edit their expiry date to today's date
4) Ensure that patron has a primary email address
5) Run perl misc/cronjobs/membership_expiry.pl -c -v -before 10 -after 10
6) Check the patron's messages, there should be no expiry notice queued
7) Edit the patron's messaging preferences and select the checkbox for email in the row for 'Patron expiry'
8) Repeat step 5
9) There should be a patron expiry message in the patron's message queue
10) Sign off!

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 29 Martin Renvoize (ashimema) 2025-01-06 15:27:57 UTC
Created attachment 176156 [details] [review]
Bug 30300: (QA follow-up) Move logic to queue_notice

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 30 Martin Renvoize (ashimema) 2025-01-06 15:28:00 UTC
Created attachment 176157 [details] [review]
Bug 30300: (QA follow-up) Remove extra return

The routine loops on the found transports, if there are none it will simply return, no need to short circuit

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 31 Martin Renvoize (ashimema) 2025-01-06 15:28:02 UTC
Created attachment 176158 [details] [review]
Bug 30300: (QA follow-up) Prevent regression and improve clarity

This patch amends the DB update to check the MembershipExpiryDaysNotice syspref and sets the flag for all patrons if the syspref is set

It also improves the UI visiblity by hiding the messaging preference if the syspref isn't set and adding more clarity to the syspref definition

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 32 Martin Renvoize (ashimema) 2025-01-06 15:28:05 UTC
Created attachment 176159 [details] [review]
Bug 30300: (QA follow-up) Remove use of Koha:: and C4:: in DB Update

We require that database updates don't rely on Koha:: or C4:: code to
reliably maintain idempotency (The underlying methods may change at a
future date).

This approach uses two INSERT IGNORE statements which should also
improve performance.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 33 Katrin Fischer 2025-02-03 13:19:04 UTC
I've made a few little changes in a follow-up patch:

1) Terminology

I have updated "Patron expiry" to read "Patron account expiry". 

I was wondering if 'library account expiry warning' or similar might not even be better as a description for the messaging preference in the OPAC?

2) Database update

I have updated to latest "standard" using say_sucess etc.


To be fixed:

3) Sample notices

The patch adds message transports SMS and phone, but we don't ship sample notices for these. I think at least for SMS this would make sense and would be consistent with other features. Can go on a separate bug.

4) Release notes 

Summarizing the behavior on update etc. would be really useful here.
Comment 34 Katrin Fischer 2025-02-03 13:29:06 UTC
Pushed for 25.05!

Well done everyone, thank you!