feat: Ability to configure edx-ace with course emails

[ziafazal]

This PR has changes to configure edx-ace with course emails.

Steps to test

1. set BULK_EMAIL_SEND_USING_EDX_ACE setting to True for LMS
2. Sign in to the Django administration console for your LMS. For example, http://{your_URL}/admin.
3. In the Bulk_Email section, next to Bulk email flags, select Add. Check the Enabled check box. Uncheck the "Require course email auth" check box and Save.
4. The instructor tab of the demo course should now include an "Email" tab. Use it to send an email to yourself.
5. Make sure the email you get use edx-ace base templates and Course Email ace templates found in lms/templates/bulk_email/edx_ace/bulkemail/email

Fixes openedx/wg-build-test-release#100

[justinhynes]

Hi @ziafazal,

At first glance these changes look look good. However, I am confused on one point about the implementation. How do we send an email through ACE and keep our custom email templates?

I pulled the branch as I wanted to try this out myself and found that when I set BULK_EMAIL_SEND_USING_EDX_ACE to True, it sends the default (Open edX?) template. If I set BULK_EMAIL_SEND_USING_EDX_ACE back to False then it sends an email with the customized template defined in my devstack instance.

My assumption here would be that this setting would control the mechanism that sends the email, not the template used. I find that to be confusing. Am I missing any additional configuration to have my message send through ACE and use the customized templates?

[justinhynes]

I'm no expert on the use of edx-ace, but I am used to seeing ACE messages inheriting from a common base message class named BaseMessageType. Example:

edx-platform/lms/djangoapps/verify_student/message_types.py

Line 7 in 6042972

class VerificationApproved(BaseMessageType):

.

It's not apparent to me why we used a different (abstract) base class. Why is that?

[justinhynes]

Is it to have a common send() function between the DjangoEmail and ACE implementations?

[ziafazal]

Yes to achieve polymorphism or have single method to send both types of emails.

[ziafazal]

BaseMessageType is limited to defining message types of edx-ace emails whereas CourseEmailMessage is base class for two types of course emails we have

1. Emails using Custom templates
2. Emails using edx-ace templates

[ziafazal]

@justinhynes Thanks for reviewing it.

when we set BULK_EMAIL_SEND_USING_EDX_ACE setting toggle to True that means we should send bulk emails via edx-ace and these emails should use templates available at lms/templates/bulk_email/edx_ace/bulkemail/email. This is how edx-ace work it looks for templates in the in app template directory.

when we set BULK_EMAIL_SEND_USING_EDX_ACE setting toggle to False that means we should send emails the way they are being sent right now(using django's mail module). Template would be pulled from temples from django admin(Home › Bulk_Email › Course email templates ›). We need to add a template having name field set to empty if there is not already one.

BULK_EMAIL_SEND_USING_EDX_ACE setting not only control the template selection but also the mechanism for sending bulk course emails.

[justinhynes]

Hi @ziafazal,

The ACE template that is being used would be an unbranded, Open edX template, no? Is there a way to override that template and use a branded one? My understanding is that setting BULK_EMAIL_SEND_USING_EDX_ACE to True would send the message via ACE but be locked into an unbranded/generic template.

I would think that the BULK_EMAIL_SEND_USING_EDX_ACE setting would control the mechanism sending messages, but would also consider any existing custom templates and use that as well. It looks like we only do that when we aren't using edX ace to send the message.

[ziafazal]

Hi @justinhynes

No. edx-ace templates would be branded see course email template is inheriting from {% extends 'ace_common/edx_ace/common/base_body.html' %}. That means our bulk course email templates are going to use same branding as rest of edx-ace email templates.

When we send emails via edx-ace we have to define our email templates in relevant app directory. Now if we want to customize those templates we can either change those html and txt files inside that directory(lms/templates/bulk_email/edx_ace/bulkemail/email) or we can customize them in our theme.

[ziafazal]

@justinhynes do you need anything else from me to review this PR?

[justinhynes]

@ziafazal Sorry, this fell off my list of things last week.

Sorry, I have a couple of additional questions.

1. Is there any work needed to brand this specifically for edX/2U in edx-themes? If I understand correctly, we should automatically be pulling in the styles we need to make this message look like the others?

2. When sending bulk messages through ACE, this means we would only support one custom template for all messages, right? I believe that today, not using ACE, that we can define multiple custom email templates.

Thanks.

[ziafazal]

@justinhynes

1. No we don't need any additional work to brand these edx-ace templates for edX/2U we are inheriting from default base templates and I guess those base templates already overriden/customized in edx-themes for edx/2u.

2. No matter we use edx-ace or not we define single custom template for bulk course emails. In case of edx-ace this template is on file system lms/templates/bulk_email/edx_ace/bulkemail/email whereas in case of django mail we define that template in django admin (Home › Bulk_Email › Course email templates ›). Although we can define multiple templates in django admin but only one with Name field set to empty is used for bulk course emails.

[justinhynes]

@ziafazal Thanks for all the answers to my questions. You've alleviated most of my concerns.

Although we can define multiple templates in Django admin but only one with Name field set to empty is used for bulk course emails.

This is true now, but in the past we've supported multiple templates, maybe this isn't as much of a concern now that the multi-tenancy/whitelabel features are gone.

Thanks for your patience.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.