Bug 30300 - Add patron expiration email to patron messaging preferences
Summary: Add patron expiration email to patron messaging preferences
Status: Signed Off
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: rel_25_05_candidate, release-notes-needed
Depends on:
Blocks: 30301
  Show dependency treegraph
 
Reported: 2022-03-15 18:49 UTC by Nick Clemens (kidclamp)
Modified: 2024-12-04 16:59 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:
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

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