search

mitodl / mitxpro

BSD 3-Clause "New" or "Revised" License
12 stars 2 forks source link

feat: welcome emails for xPRO Learners #3017

Closed mudassir-hafeez closed 2 months ago

mudassir-hafeez commented 3 months ago

What are the relevant tickets?

#2470

Description (What does it do?)

This PR implements the welcome email feature, which sends a welcome email based on each user's course enrollment.

How can this be tested?

  1. Checkout to this branch and make sure openedx is configured.

  2. Try to enroll any user to any course through frontend, or you can use create enrollment command to enroll a user.

    • docker-compose exec web ./manage.py create_enrollment --user "" --run "" --code="test7e91c44ea5a999beasdfadsfadfadfwer"

  3. Check if you receive welcome email along with enrollment email.

  4. Verify the content of the email with welcome letter provided in the the ticket https://github.com/mitodl/hq/issues/2470#issue-1912198810 or you can see the link here: https://docs.google.com/document/d/1gGdXSVT4U34_bIADqGzZvcEyWuulM6VK8VDbqyDF1MY/edit

  5. Now, again try to enroll user with transfer enrollment, and follow steps 3 and 4.

Screenshots (if appropriate):

Note: The logo is missing from the email attachment above because the links point to a local server. However, you can refer to the screenshot below to review the logo, which should ideally be the same once deployed:

image

Additional Context

mudassir-hafeez commented 3 months ago

@arslanashraf7 would you please do an quick review before merging this PR?

arslanashraf7 commented 3 months ago

@arslanashraf7 would you please do an quick review before merging this PR?

I'll take a look, Could you also get a review from @cachob on the screenshots?

mudassir-hafeez commented 3 months ago

I'll take a look, Could you also get a review from @cachob on the screenshots?

I've updated the PR description with the new screenshots and email attachments for @cachob to review.

mudassir-hafeez commented 3 months ago

Share the complete screenshot of the email with @cachob - You can add it to the PR description.

Added

The order of the enrollment and welcome emails is not specified - In my testing the order was random e.g. sometimes I got the welcome email before the enrollment email. (We should check with @cachob If that matters)

At my end, it seems to be in order, first enrollment email and then welcome email. Also we are sending them synchronously. Not sure if it is due to server delay or latency at your end. @cachob would you please look into the Arslan's comment https://github.com/mitodl/mitxpro/pull/3017#pullrequestreview-2151458408 if that matters?

arslanashraf7 commented 3 months ago

At my end, it seems to be in order, first enrollment email and then welcome email. Also we are sending them synchronously. Not sure if it is due to server delay or latency at your end.

This was the order I received them while enrolling in a program

image
cachob commented 3 months ago

@arslanashraf7 / @mudassir-hafeez - I don't think the order of which emails come first is critical. Are we able to ensure the order? I assumed that these could be subject to delays depending on the email service, user email settings, etc.

The screenshots look good. Do you mind giving me a quick overview of how this feature will work?

arslanashraf7 commented 3 months ago

@arslanashraf7 / @mudassir-hafeez - I don't think the order of which emails come first is critical. Are we able to ensure the order? I assumed that these could be subject to delays depending on the email service, user email settings, etc.

We can get around this with:

  1. Send the welcome email in a celery task with a delay of a few seconds after sending the successful enrollment email. (Will help maintain the order in general but sometimes it might not work for reasons you mentioned)
  2. Send the enrollment email and wait on it's success and then send the welcome email. (Good but since the order won't matter we can skip this as well)

A question for you @cachob:

Do you this we should keep this feature behind a feature flag as well? Any chance the course team might want to turn this off at some point?

The screenshots look good. Do you mind giving me a quick overview of how this feature will work?

I'd let @mudassir-hafeez answer this.

mudassir-hafeez commented 3 months ago

The screenshots look good. Do you mind giving me a quick overview of how this feature will work?

@cachob, this feature works similarly to the enrollment email. There is no admin interaction required. The welcome email will be delivered to the user along with the enrollment email for each course enrollment. If we enroll a user in a course (or program), the welcome email will be automatically delivered on enrollment. This also applies to program enrollment, where the welcome email will be delivered for each course within the program. This ensures that learners receive the welcome email automatically at the moment of enrollment.

CC: @arslanashraf7

mudassir-hafeez commented 3 months ago

Also discussed with @arslanashraf7 specifically for the case of a program. For example, if we have 5 courses in a program (in production) and 5 enrollment emails were being delivered previously. With this feature, now it will add 5 welcome emails for these course enrollments in the program, which will make a total of 10 emails. This number will vary if we have more or fewer courses in the program.

We thought to check with @blarghmatey from DevOps to ensure we wouldn't encounter any issues (e.g., rate limiting, emails being marked as spam or any other potential issues) with this number of emails being sent.

cachob commented 2 months ago

@arslanashraf7 / @mudassir-hafeez - thanks for the responses. Let's set this up behind a feature flag as well. I think moving forward all enhancements like these should be behind one as well.

mudassir-hafeez commented 2 months ago

@arslanashraf7 would you please re-review as I have made some changes to make this feature behind a feature flag? Related ol-infrastructure PR is here https://github.com/mitodl/ol-infrastructure/pull/2548.