episphere / connect

Connect API for DCEG's Cohort Study
10 stars 5 forks source link

Upgrade handling of notification specification conditions #1064

Open sonyekere opened 2 months ago

sonyekere commented 2 months ago

Hi! @HanaShiho , Placeholder for requirements .

sonyekere commented 2 months ago

Closing, DevOps assist may not be needed. Comms will be programming and testing on their end.

sonyekere commented 2 months ago

Reopening, DevOps may need to support in creating a conditionality in order to program the email.

HanaShiho commented 2 months ago

Thanks, @sonyekere! For the Welcome to Connect email, we are going to be sending a separate one to HFH participants (part of the July release) and a separate one to HP participants, and everyone else will remain in the existing one. So to implement the change, we will need a new derived variable for 'health provider= everyone except HP and HFH' since we cannot use OR conditionality. Alternatively, we could create a separate Welcome spec for each site similar to how we have for the biospecimen invite emails.

Theoretically, having OR conditionality in SMDB would be ideal. However, I'm unsure if this is feasible, and if so, how much effort from DevOps would be needed.

we-ai commented 2 months ago

The best option might be creating a "Welcome to Connect" notification spec for each provider, so that welcome email can be customized for each provider (not only to HFH and HP) . And we don't need to change code for this and generate derived variables.

@brotzmanmj Do you think a separate notification spec for each provider is a good solution? I believe so. With time goes, each provider may want customized welcome emails and the dedicated notification spec can accommodate it.

sonyekere commented 1 month ago

Following the discussion on 8/1/2024, @we-ai, @jhflorey and @Davinkjohnson will connect further to discuss potentially building out a separate notification spec. Considering to handle this programmatically with SQL queries instead of multiple email templates. @brotzmanmj agreed that this could potentially be a task for September, depending on complexity and bandwidth.

we-ai commented 1 month ago

Based on above updates, looks like we need to upgrade query condition handling in SMDB, with below added functionalities:

With these added functionalities, queries with more flexibility can be handled, including scenarios of HP and HFH mentioned, for example (with pseudocode):

@HanaShiho @brotzmanmj Please comment.

brotzmanmj commented 1 month ago

Hi @we-ai , yes this sounds like what we need. We understand it will work for discrete values like this, but it is much more complicated to have it allow for conditioning on a date range (like verified between date x and date y). Is that correct?

we-ai commented 1 month ago

Hi @we-ai , yes this sounds like what we need. We understand it will work for discrete values like this, but it is much more complicated to have it allow for conditioning on a date range (like verified between date x and date y). Is that correct?

Date range isn't more complicated and it's covered by multiple uses of the same key. An example query can be: other conditions AND verifiedDate > "2024-07-01" AND verifiedDate < "2024-08-05".

HanaShiho commented 1 month ago

@we-ai Sounds good to me. I didn't know we could specify a date range so that's good to know!

depietrodeanna commented 1 month ago

Hi @we-ai , yes this sounds like what we need. We understand it will work for discrete values like this, but it is much more complicated to have it allow for conditioning on a date range (like verified between date x and date y). Is that correct?

Date range isn't more complicated and it's covered by multiple uses of the same key. An example query can be: other conditions AND verifiedDate > "2024-07-01" AND verifiedDate < "2024-08-05".

Hi Warren- we attempted this in the conditionality with the date range and it did not work previously. Are you suggesting it should work given current functionality or is the functionality not yet in place?

HanaShiho commented 1 month ago

@we-ai @sonyekere just checking on the timeline---will these updates/changes be included in the August or September release?

brotzmanmj commented 1 month ago

@sonyekere is this moving to Sept release?

sonyekere commented 1 month ago

Hi @HanaShiho and @brotzmanmj , Yes. This is expected to be worked on in the September release

we-ai commented 1 month ago

The proposed new functionalities (allow repeated use of a key; OR operator) will allow more flexible conditions in notification specs. The topic (HP separate welcome email) of current issue will be addressed after implementing these functionalities.

Here I try to explain why data range will be addressed after the updates. Currently a key can only keep one value in saved specs. If you use a key multiple times when setting up spec conditions in SMDB, only the last value of the key will be saved and used to generate data queries. Example below with pseudo to show what your want and what you get based on code spec handler:

After implementing functionality that allows repeated uses of key in conditions of a spec, lots more queries can be generated, including queries using range of values that cannot be done currently. Example:

Let me know if you have questions. @brotzmanmj @depietrodeanna @HanaShiho

HanaShiho commented 1 month ago

Thank you for the explanation, @we-ai! The current setup and the new features you outlined make sense to me.

brotzmanmj commented 1 month ago

This makes sense to me too, @we-ai . For OR conditionality, can you explain if it will work in complex conditionality where the order of operations matters?

we-ai commented 1 month ago

@brotzmanmj OR to AND is like plus (+) to multiply (*) in math. If needed, you can adjust the order using parenthesis. For example (pseudocode):

brotzmanmj commented 1 month ago

Perfect, thanks

HanaShiho commented 5 days ago

Ready for stage