episphere / connect

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

Menstrual Cycle Survey Trigger changes #1017

Closed cusackjm closed 2 months ago

cusackjm commented 4 months ago

Menstrual Cycle Survey Requirements: https://nih.app.box.com/file/1504923433431

sonyekere commented 3 months ago

Hi, Will this be going into the July release?

brotzmanmj commented 3 months ago

It can be July or August. We can leave the decision of where it fits best to the DevOps team

jhflorey commented 3 months ago

After looking through this problem I realized what needs to be done is

Please correct me if i was wrong.

brotzmanmj commented 3 months ago

@jhflorey not exactly, we need to review internally and better explain requirements. We will get back to you shortly.

rohanjay10 commented 3 months ago

If we are making these changes looks like the MOOP may have to be updated as well. Specifically when the survey is available, the number of follow up reminder emails, and when they are sent. As well as any new information we are adding.

MOOP pg 41

brotzmanmj commented 3 months ago

Requirements document has been updated.

Next step is that we need to discuss consolidating the existing email notifications and fixing the conditionality to include the derived variable. @HanaShiho will set up a meeting.

brotzmanmj commented 3 months ago

Deanna and Hana and I reviewed the notification specs this morning and found that they cannot be consolidated and they do already include the derived variable.

I updated the requirements document again. We are ready to discuss at the meeting Hana set up for next week.

rohanjay10 commented 3 months ago

@jhflorey Based on the updated requirements document:

All, let me know if the above needs to be corrected.

Hana has set up a meeting next week for the team to discuss this and other requirements further.

rohanjay10 commented 3 months ago

4/17/24 Call Summary and Next Steps

  • MC Survey trigger is the 1st email notification that is sent 5 days after the biospec survey submission.
    1. @jhflorey to find out from Tony how exactly the current MC Survey is triggered through the derived variable. Currently MC Survey is triggered through a derived variable upon "immediate" submission of bio specimen survey.

For the future state, we need to derive the variable as it is done currently, with no changes(as shown on Eligibility for Survey section of Requirements document), and then trigger the MC Survey upon the 1st email notification sent after 5 days upon either biopic. survey submission. Trigger will appear on the dashboard depending on when email is sent(as done on PROMIS survey).

  1. @jhflorey to check with Tony about what needs to be done to maintain the same message ids between tiers(PROD > DEV). The message IDS that exist on Prod, and do not exist in Dev(only in draft) should be the same message ids in Dev. We need to check on what needs to be done to maintain the same message ids in Dev.

2a) Once @jhflorey checks this, @HanaShiho will update Exclusion criteria.

  • Follow up reminders will be sent 10, 15, 20, and 35 days (2nd, 3rd, 4th, 5th notification respectively) after the date of submission of the biospec survey
    1. @HanaShiho to add reminder after 35 days.
  1. Once we have the above information and @jhflorey is ready we'll be able to test with the conditions and push it through all the tiers.

@brotzmanmj @HanaShiho Please feel free to add on or correct any of the above.

brotzmanmj commented 3 months ago

Thank you for this excellent summary. Just adding that for the future state, the MC Survey will trigger off two '1st email notifications' because one 1st notification goes to people who did research collections and one 1st notification goes to people who gave clinical collections.

anthonypetersen commented 3 months ago

@brotzmanmj @jhflorey @rohanjay10 here is what I think should be done regarding this update to trigger the survey, let me know your thoughts:

brotzmanmj commented 3 months ago

@anthonypetersen I agree with your approach. A few questions/clarifications: "Change the default value of the 459098666 (Menstrual Survey Status) from 972455046 (Not Started) to 789467219 (Not Yet Eligible). This would also require mass participant update in prod upon go-live of changes" Would this negatively impact anyone for whom the MC Survey has already been triggered?

"Add code to our notification logic that will change 459098666 (Menstrual Survey Status) to 972455046 (Not Started) upon first email notification (or sms?)" I think it should be first email notification but note there are two email notifications (one for people who did research collections and one for people who did clinical collections) so we need to tie it to both.

Also update the SMDB Participant Summary page to display the MC Survey status according to reflect 459098666 (Menstrual Survey Status) and show up for everyone, which is different than what it does now.

Will email/SMS notification logic need to change at all? Currently the message sends if someone is 289750687 (Menstrual Survey Eligible) = 'yes' and stops when 459098666 (Menstrual Survey Status) = 'submitted'

brotzmanmj commented 3 months ago

@anthonypetersen One more thing because this just came up... what happens if someone completes a clinical sample and a research sample as happened at HFH just now. The person submitted both the clinical biospec survey and then the research biospec survey. Does the variable 289750687 (Menstrual Survey Eligible) derive just once or would it possible get 're-derived'/changed/overwritten in this circumstance?

anthonypetersen commented 3 months ago

@brotzmanmj going to respond to your points one at a time

  1. We could do a mass update that says something like "if the participant hasn't gotten the first email and hasn't started the survey, update their status to Not Yet Eligible" - that way the system can update them to Not Yet Started when appropriate
  2. We can easily listen for the first instance of either email. Do we need to listen for SMS messages as well?
  3. Do you just mean update it so that it shows Not Yet Eligible / Not Yet Started as appropriate?
  4. No, we should be able to keep all of that intact.
  5. We only try to derive 289750687 (Menstrual Survey Eligible) as long as it is not equal to Yes. Once they become eligible, we no longer try to re-derive it.
brotzmanmj commented 3 months ago
  1. We could do a mass update that says something like "if the participant hasn't gotten the first email and hasn't started the survey, update their status to Not Yet Eligible" - that way the system can update them to Not Yet Started when appropriate. [Sounds good]
  2. We can easily listen for the first instance of either email. Do we need to listen for SMS messages as well? [No, email will be more inclusive so just email is fine]
  3. Do you just mean update it so that it shows Not Yet Eligible / Not Yet Started as appropriate? [Yes]
  4. No, we should be able to keep all of that intact. [Great]
  5. We only try to derive 289750687 (Menstrual Survey Eligible) as long as it is not equal to Yes. Once they become eligible, we no longer try to re-derive it. [This should also be fine]

I think we are good to go forward as suggested! Thanks.

anthonypetersen commented 3 months ago

@jhflorey please see last few comments

we-ai commented 2 months ago

Below are my comments on this issue. Please correct me if I'm wrong.

The notification system is for sending out emails/SMS messages and saving notification records. We're trying to boost to 10x of its current capacity so that it can efficiently send out 200,000 emails and 200,000 SMS messages during a scheduled run. If some data updates in participants (or other collections) are needed, they are better done outside notification handling.

anthonypetersen commented 2 months ago

@brotzmanmj is it a hard requirement that the survey needs to appear at the same time the email goes out? would it be acceptable to have the survey become available on the day the participant is set to receive the first email?

brotzmanmj commented 2 months ago

@anthonypetersen I think that would require a quick discussion. Are you saying you don't want to condition the survey trigger on the notification?

anthonypetersen commented 2 months ago

@brotzmanmj with Warren attempting to make improvements to our notification code to allow for further scaling as the study grows, he has concerns about putting additional code like what would be required. See one of his comments on the PR: https://github.com/episphere/connectFaas/pull/616#issuecomment-2214159828

@we-ai please comment if you have any other thoughts

brotzmanmj commented 2 months ago

@anthonypetersen and @we-ai Going back to the original premise, the first instance where we decided to link a survey trigger to a notification was for PROMIS/QOL survey and that was because there was nothing else to trigger it on. In other words, we did not have the ability to condition the trigger on 'if the person was verified after 12/1/2023 and had not refused future surveys and had not withdrawn from Connect, trigger this survey to appear on their To Do list 90 days after verification.' Instead we conditioned the email on the above and then set the survey trigger based on the email notification being sent (or attempted to send).

Is the menstrual cycle survey different in some fundamental way such that we can condition is to appear 5 days after the biospecimen survey is submitted if they are eligible for it, not withdrawn, and not refused?

anthonypetersen commented 2 months ago

@brotzmanmj it would not be difficult to have the survey display 5 days after the survey is submitted, it's displaying at the moment a specific notification is sent that makes things more "complicated". We did it with PROMIS, and could do the same thing here, but Warren is suggesting against using the same code strategy due to scalability concerns r.e. notifications.

brotzmanmj commented 2 months ago

@anthonypetersen In that case, we could do as Warren suggested:

If we want to match the time of survey reminder and survey, assuming notifications are sent out without issues, we can open the survey at 3pm on day 5 by checking participant data.

We just need to match conditions.

anthonypetersen commented 2 months ago

@brotzmanmj we are discussing technical options internally and will report back what we decided and can discuss what implications there might be from an operations pov

we-ai commented 2 months ago

@anthonypetersen and @we-ai Going back to the original premise, the first instance where we decided to link a survey trigger to a notification was for PROMIS/QOL survey and that was because there was nothing else to trigger it on. In other words, we did not have the ability to condition the trigger on 'if the person was verified after 12/1/2023 and had not refused future surveys and had not withdrawn from Connect, trigger this survey to appear on their To Do list 90 days after verification.' Instead we conditioned the email on the above and then set the survey trigger based on the email notification being sent (or attempted to send).

We should be able to implement the conditions in PWA without using email notifications to update data.

anthonypetersen commented 2 months ago

Discussion notes w/ @brotzmanmj -

jhflorey commented 2 months ago

@HanaShiho The code change is already in dev. Please plan to test it.

HanaShiho commented 2 months ago

This is ready for stage (thank you Erin for testing the survey triggers in dev!)