New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updated mailers for facilitator workshops #34638
Conversation
f96e750
to
9afb0e5
Compare
@@ -98,13 +106,22 @@ def teacher_enrollment_reminder(enrollment, days_before: nil) | |||
@pre_workshop_survey_url = enrollment.pre_workshop_survey_url | |||
@is_first_pre_survey_email = days_before == INITIAL_PRE_SURVEY_DAYS_BEFORE | |||
|
|||
from = from_teacher |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put this first part in an else
so that we treat these as immutable values rather than overwriting them immediately in some cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -0,0 +1,82 @@ | |||
- pre_work_url = 'https://docs.google.com/document/d/1mmBcMoRKO2TVxQSEEb-SxMnhuDZUkjE06tHa-8CGeyE/edit' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it expected that this is an /edit
URL while the one just below is a /pub
one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is as provided in spec, and there isn't currently a "published" version of this document. I'm asking Megan if she'd like to publish this doc too.
|
||
%p | ||
Please have the following with you on the day of your session to ensure you | ||
are able to participate fully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
End in :
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
= link_to 'this page', Pd::WorkshopMailer::SUPPORTED_TECH_URL | ||
for more information regarding compatible operating systems and browsers. | ||
We do not recommend bringing a tablet as your primary device. | ||
%li Download the Zoom app (more information below) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
at end to be consistent with other bullets?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -65,8 +68,11 @@ PREVIEWS = %w( | |||
teacher_enrollment_reminder__csf_intro_3_day | |||
teacher_enrollment_reminder__csp_1_10_day | |||
teacher_enrollment_reminder__csp_1_3_day | |||
teacher_enrollment_reminder__csp_for_returning_teachers_10_day |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are just entries we forgot to add in previous PRs, yeah? No changes to CSP for returning teachers workshop mailers in this PR, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct - I didn't realize we had this script, and just ran across it while working on this update. I went ahead and updated the list of mailers it tests to reflect some recent additions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote this script long ago to verify that email template changes don't have unexpected results. (For example, 1 partial template can be used in multiple emails that we are not aware of.) Not sure if it is helpful to anyone but me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems very useful, thanks Ha!
Just trying to understand...what did our enrollment receipts / reminders for facilitator training workshops look like before this change? |
Oh, good point Ben. I'll address other review feedback and then post before/after for these mailers as a record of the changes we're making. |
I've updated the PR description with before/after screenshots of the mailer previews. |
Customizes the facilitator workshop enrollment receipt and enrollment reminder emails, as spec'd here.
Testing story
Manually tested using local mailer previews. Verified with the
diff_email_previews
script that no other mailers were changed.Enrollment receipt (Facilitator workshop):
Enrollment reminder (Facilitator workshop):
Reviewer Checklist: