Action Trigger for new user creation - Change hook to active

[Josh Stompro]

This is a follow up to bug 1392396, which added an action trigger setup for sending out a new user welcome message.

As it was commited it used an passive hook that queried the actor.usr table for new accounts since a certain date.

Bill Erickson mentioned in a comment after the fix was created that there is an active hook for au.create available.

I would like to switch the setup of that feature over to using the active hook for correctness. For our goal of sending out welcome emails as soon as possible after account creation, the active hook seems like a better design, since we wouldn't be searching the actor.usr table constantly for new accounts.

Josh

[Jessica Woolford (jwoolford)]

I am working with a library now who wants these messages sent only once every 3 days. So if we could choose between the passive and the active hook, that would be ideal.

[Bill Erickson (berick)]

Jessica, you can still chose to "fire" the events at any time (e.g. every 3 days), regardless of active vs. passive.

[Steve Callender (stevecallender)]

What I am seeing is that due to the au.created hook automatically firing off in the "update_patron" routine in OpenILS/Application/Actor.pm, that the delay on the trigger is completely ignored and a notice is created no matter what with no regard to the delay setting (the notice is created with a run_time as now() ). I believe it's because the trigger is marked as "passive".

So the quick fix here, and what I beleive is the correct thing to do, is to just change the trigger from passive to not in the database.

update action_trigger.hook set passive =false where key='au.created';

The catch here however, is that if folks have been relying on a "passive" trigger to go out and create notices after their repeat_delay threshold, it will no longer do that since it will rely on creating the notice at new user time, rather than going out and finding existing ones.

[Bill Erickson (berick)]

Looks like update_patron() creates the events, but I suspect the A/T runner is scooping them up and firing them shortly after they are created.

Applying a granularity to the event def with a matching A/T runner invocation should give full control of the timing of the firing.

Agreed setting the existing hook to active is the correct thing, since the code actively creates the events.

I think a migration strategy could be 1) set hook to active, 2) apply granularity to the affected event defs, 3) supply sample CRON A/T runner entry in the upgrade notes that leverages the new granularity.

[Terran McCanna (tmccanna)]

Noting that we're running into this as well since we're trying to send out a new user followup message two weeks after our original welcome notice.

Is there any use case for why au.create should remain passive?

[Chris Sharp (chrissharp123)]

Branch implementing Bill's suggestion here:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/csharp/lp1705333_au_created_passive_setting

Tested so far against a PINES-ish 3.10 server.

[Terran McCanna (tmccanna)]

We've had this in production in PINES for more than a month now and everything seems to be working fine.

[Jessica Woolford (jwoolford)]

Here is a sign-off branch that includes a check to see if the hook is already active, and skips the update if it is:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jwoolford/lp1705333_au_created_passive_setting_with_check

Bibliomation's hook is already active, and we have a different granularity set, so we wouldn't want the script to override that.

Not adding a sign-off tag just yet because IF THEN ELSE statements in SQL are new to me, so I'd appreciate a more experienced eyes on it.

Editing to add this has been tested on a 3.9 database with Bibliomation data, and seems to do the job in both scenarios (skip the UPDATE queries if hook is already active, and run the UPDATE queries if not).

[Terran McCanna (tmccanna)]

Adding back needswork because when I ran the included upgrade script I got:

ERROR: syntax error at or near "\"
LINE 15: \qecho

[Terran McCanna (tmccanna)]

Removing milestones for now since it needs a little work.