Wishlist: Patron notification via email when card is about to expire

Bug #1124498 reported by Michael Peters
38
This bug affects 6 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

Written for Evergreen Master

The attached code creates an action/trigger event which notifies a patron, via email, that their card is about to expire.

There are two portions to this code:

#1 - the A/T template, hooks, etc.
#2 - the modification of action_trigger_filters.json

Please see the git branch to follow for more information.

It would be EXTRA cool if we could somehow create a YAOUS so libraries could decide how early they wish to send the patron a pre-notice.

Revision history for this message
Michael Peters (mrpeters) wrote :
Ben Shum (bshum)
Changed in evergreen:
status: New → Triaged
Revision history for this message
Steve Callender (stevecallender) wrote :

There could be a possible flaw in this setup. The action triggers rely on event targets to make sure they don't duplicate themselves. Example, overdue notices use circ ID's for it's target. Password requests use the ID from the actor.usr_password_reset table. My concern is that this expire notice is using the target ID of the actor.usr table in which will never change for the lifetime of the patron. I believe that once a notice event is created, a year from now when that patron is up for expiration once again, the system will see that it already had sent a notice for that patron on that event and it will never send another one even if it's a year later and a different expiration period.

Steve

Revision history for this message
Steve Callender (stevecallender) wrote :

Another thought after talking to Mike Rylander about this, is that there may be an API out there that can be used to cross reference the data with the DB to make sure that it exists before building the link. It could be used to eliminate dead links. Just throwing this out there to spark ideas, not as a definitive solution.

Revision history for this message
Steve Callender (stevecallender) wrote :

Apologies. My latest comment was meant for a different launchpad ticket.

Revision history for this message
Dan Wells (dbw2) wrote :

Steve, I think your comment from 2013-03-27 is accurate. I think the best solution would be a definable 'expiration period' on the event definitions themselves. After an event expires, the target is again eligible for consideration. In addition to this new field, implementation would likely just be a filter on the JOIN which brings in the existing events.

Does anyone else with more A/T experience see a simpler way out of this problem?

Revision history for this message
Tim Spindler (tspindler-cwmars) wrote :

Version: 2.3.3

We have implemented this notice and other than the issue of not repeating notices it has been working smoothly. However, we are finding that libraries would like an "opt out" function. Right now, I have just created an event for some patrons in libraries that did not want a notice sent to their patrons.

Revision history for this message
Alex Lazar (alex-lazar) wrote :

I'm wondering if we have two expiration notifications set up, let's say a reminder sent at -30 days and another one at -7 days, would the -7 day notification also be treated as a duplicate due to the issue that Steve described?

Revision history for this message
Bill Erickson (berick) wrote :

Aleksey, in that case you have two different event definitions -- one per delay -- so the events would not be seen as duplicates.

Revision history for this message
Kathy Lussier (klussier) wrote :

Just adding a note that code from https://bugs.launchpad.net/evergreen/+bug/1207902 has been pushed into master, allowing action triggers to be repeated.

If there is still interest in this feature, could the branch be updated to allow for repeatability?

Revision history for this message
Michael Peters (mrpeters) wrote :

Kathy -- thanks for pointing that out. I personally can't work on this code currently due to other obligations but anyone who is interested is more than welcome to modify my branch as needed to make these repeatable.

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

I'll work on adding the expire time additions so that this notice can be included as a default in master.

Changed in evergreen:
assignee: nobody → Josh Stompro (u-launchpad-stompro-org)
status: Triaged → In Progress
Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

So what would be a sane repeat_delay for Michael's example of sending an expiration notice 30 days before an expiration date.

"365 days", "335 days" , "1 Year", "30 days" ? If the account is renewed for an additional year, then I would think that that repeat delay would just need to be more than the 'delay' so it doesn't happen before the account is actually expired... although the delay of "-30 days" wouldn't allow it to be run again anyway. So would a "repeat delay" of "2 days" cover it.

Account X expires on 11/30/2014 - on 11/1/2014 the notice is sent. On 11/3/2014 the event could run again if the "repeat_delay" is "2 days", but it wouldn't be valid until the expiration date is -30 days again, right? So if the account was renewed for 1 more month(12/30/2014), the event would run again the next month on 12/1/2014.

Josh

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

Michael, I see that you set the Errors-To: header in your example template, I had to look that up since I've never seen it before. The sites that I found said to not use it since it was an old hack from back in the UUCP mail days.

http://sendmail.org/~ca/email/doc8.12/op-sh-2.html
https://tools.ietf.org/html/rfc2076

Was there a specific reason you were using it?
Josh

Revision history for this message
Kathy Lussier (klussier) wrote :

Hi Josh,

Are you still working on this bug?

Kathy

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :
Download full text (3.9 KiB)

Kathy, I am still working on it, here is the branch with my changes.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp1124498_30day_preexpire_notice

I don't know how the sql files need to be properly formatted for upgrades and new installs. Hopefully one of the core devs can provide guidance or can just make the needed changes.

One thing that I couldn't get working was to set an event_params of recipient_email to be able to set the To: line for testing. It looks like it should work, I must be missing something that enables the params.

Here are my notes on what needs to be done to set it up after I went through it a second time. I would be happy to fix this up and add it to the official docs in the future.

How to enable the 30 Day Account Expiration Notice

Step 1 - Import the SQL to create the Hook, Event Definition, Event Params and Event Environment values.

Step 2 - Edit your /openils/conf/action_trigger_filters.json to include the following, and example is included in action_trigger_filters.json-example. This tells the action_trigger_runner to process the expire hook and filters out accounts
that are not active and deleted, as well as restricting the scope to the home_ou of the org unit.

    "expire" : {
        "context_org":"home_ou",
        "filter" :
                        { "active":"t",
                        "deleted":"f" }
        },

Step3 - Edit the event in the evergreen staff client. Under Admin -> Local Administration -> Notification/Action Triggers.
   - Edit the template if you want to change the language.
   - For testing you can replace the to: line with a specific email address and all notices will go to that address. Set it back when you are ready.
   - Change the Owning Library if you need to limit which locations send this out.
   - Enable the template when ready.
   - Processing Delay is how many days before the account expires you want to start sending out the notice. The default is -30 days.
   - Max Event Validity Delay sets a bounding date on the expiration date. The default of -29 means that only accounts that expire between -29 and -30 days from now get processed, a 24 hour period. If you want to cover a wider range of time for each run, say if you were running this weekly, then you should subtract 6 days from the current value and use -23 days. Note: An event will only be triggered once, so you won't be sending out multiple notices if your dates overlap from one run to the next. The event will only be able to repeat after the Event Repeatability delay is up.
   - Event Repeatability Delay is how long after the first notice you could send out another notice. This allows the event to fire again after an account is renewed and is about to expire again.
   - Granularity - Set a granularity to match up with how your cron jobs are setup. If you want this to be processed daily and you have a daily cron job setup, choose "daily". (Note: the ganularity settings are case sensitive. Make sure you are using the same case in your event def and in your cron jobs.)

Step 4 - Cron Jobs - Make sure your cron jobs are setup to run the acti...

Read more...

Changed in evergreen:
assignee: Josh Stompro (u-launchpad-stompro-org) → nobody
Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

Here is a new branch against master for this feature.
http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp1124498-pre-expire-notice

Added the notice to 950.data.seed-values.sql to add it to new installs.

Reformatted the sql a little to match what I see in the existing sql.

For the upgrade script, if someone has already added this notice, the insert for the expire hook just fails and rolls the transaction back, so it shouldn't effect those that are already using this.

Anyone have any suggestions as to what else is needed to get this committed?
Josh

Changed in evergreen:
status: In Progress → Confirmed
tags: added: pullrequest
Changed in evergreen:
milestone: none → 2.next
Revision history for this message
Kathy Lussier (klussier) wrote :

Hi Josh,

Looks like this one will need a release notes entry before it goes in.

Thanks!
Kathy

tags: added: needsreleasenote
Michele Morgan (mmorgan)
Changed in evergreen:
assignee: nobody → Michele Morgan (mmorgan)
Revision history for this message
Michele Morgan (mmorgan) wrote :
Changed in evergreen:
assignee: Michele Morgan (mmorgan) → nobody
tags: added: signedoff
Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

Added release notes to this branch
user/stompro/lp1124498-pre-expire-notice-releasenotes

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp1124498-pre-expire-notice-releasenotes

I wasn't sure if I should checkout Michele's signoff branch and add it to that... I can do that if that make it easier.

Josh

tags: removed: needsreleasenote
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Yes, please add it to Michele's sign off branch. That makes it easier for someone else to merge it with her sign off.

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

I forced pushed over user/stompro/lp1124498-pre-expire-notice-releasenotes with a new branch that is based off of mmorgan's signoff branch, that includes the release notes.

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

miker suggested that the hook name have a prefix so it is clear that it is for actor.usr, so I'll change it to be "au.expired". The suggestion was in a different ticket for a similar new notice, but makes sense here also.

tags: removed: pullrequest
tags: removed: signedoff
Changed in evergreen:
assignee: nobody → Josh Stompro (u-launchpad-stompro-org)
Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

I pushed a new commit that changes the hook name. I think this probably merits a new signoff needed. mmorgan, want to test this again? :-)

tags: added: pullrequest
Changed in evergreen:
assignee: Josh Stompro (u-launchpad-stompro-org) → nobody
Michele Morgan (mmorgan)
Changed in evergreen:
assignee: nobody → Michele Morgan (mmorgan)
Revision history for this message
Michele Morgan (mmorgan) wrote :

Retested and looks good! Force pushed my signoff branch.

tags: added: signedoff
Changed in evergreen:
assignee: Michele Morgan (mmorgan) → nobody
Kathy Lussier (klussier)
Changed in evergreen:
milestone: 2.next → 2.9-beta
Revision history for this message
Ben Shum (bshum) wrote :

Pushed to master, thanks everyone!

Changed in evergreen:
status: Confirmed → Fix Committed
Revision history for this message
Ben Shum (bshum) wrote :

Also grabbed the release notes commit from the other branch. mmorgan seems to have missed that in her branch signoffs. No big deal.

Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.