feat: Ability to configure edx-ace with course emails

fix: fix typo

fix: Fixed broken unit tests and added new unit tests

fix: Fixed quality violations

fix: Fixed quality violations related to translations
This commit is contained in:
zia.fazal@arbisoft.com
2022-02-10 14:35:17 +05:00
parent 9d3f52ed67
commit b7ad137c88
10 changed files with 233 additions and 35 deletions

View File

@@ -0,0 +1,14 @@
"""
ACE message types for bulk course emails.
"""
from openedx.core.djangoapps.ace_common.message import BaseMessageType
class BulkEmail(BaseMessageType):
"""
Course message to list of recepient by instructors.
"""
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.options['from_address'] = kwargs['context']['from_address']

View File

@@ -0,0 +1,84 @@
"""
Module to define email message related classes and methods
"""
from abc import ABC, abstractmethod
from django.contrib.auth import get_user_model
from django.core.mail import EmailMultiAlternatives
from edx_ace import ace
from edx_ace.recipient import Recipient
from lms.djangoapps.bulk_email.message_types import BulkEmail
from openedx.core.lib.celery.task_utils import emulate_http_request
User = get_user_model()
class CourseEmailMessage(ABC):
"""
Abstract base class for course email messages
"""
@abstractmethod
def send(self):
"""
Triggers sending of email message
"""
class DjangoEmail(CourseEmailMessage):
"""
Email message class to send email directly using django mail API.
"""
def __init__(self, connection, course_email, email_context):
"""
Construct message content using course_email model and context
"""
self.connection = connection
template_context = email_context.copy()
# use the CourseEmailTemplate that was associated with the CourseEmail
course_email_template = course_email.get_template()
plaintext_msg = course_email_template.render_plaintext(course_email.text_message, template_context)
html_msg = course_email_template.render_htmltext(course_email.html_message, template_context)
# Create email:
message = EmailMultiAlternatives(
course_email.subject,
plaintext_msg,
email_context['from_address'],
[email_context['email']]
)
message.attach_alternative(html_msg, 'text/html')
self.message = message
def send(self):
"""
send email using already opened connection
"""
self.connection.send_messages([self.message])
class ACEEmail(CourseEmailMessage):
"""
Email message class to send email using edx-ace.
"""
def __init__(self, site, email_context):
"""
Construct edx-ace message using email_context
"""
self.site = site
self.user = User.objects.get(email=email_context['email'])
message = BulkEmail(context=email_context).personalize(
recipient=Recipient(email_context['user_id'], email_context['email']),
language=email_context['course_language'],
user_context={"name": email_context['name']},
)
self.message = message
def send(self):
"""
Send message by emulating request in the context of site and user
"""
with emulate_http_request(site=self.site, user=self.user):
ace.send(self.message)

View File

@@ -29,7 +29,8 @@ from celery import current_task, shared_task
from celery.exceptions import RetryTaskError
from celery.states import FAILURE, RETRY, SUCCESS
from django.conf import settings
from django.core.mail import EmailMultiAlternatives, get_connection
from django.contrib.sites.models import Site
from django.core.mail import get_connection
from django.core.mail.message import forbid_multi_line_headers
from django.urls import reverse
from django.utils import timezone
@@ -42,7 +43,14 @@ from common.djangoapps.util.date_utils import get_default_time_display
from common.djangoapps.util.string_utils import _has_non_ascii_characters
from lms.djangoapps.branding.api import get_logo_url_for_email
from lms.djangoapps.bulk_email.api import get_unsubscribed_link
from lms.djangoapps.bulk_email.toggles import is_email_use_course_id_from_for_bulk_enabled
from lms.djangoapps.bulk_email.messages import (
DjangoEmail,
ACEEmail,
)
from lms.djangoapps.bulk_email.toggles import (
is_bulk_email_edx_ace_enabled,
is_email_use_course_id_from_for_bulk_enabled,
)
from lms.djangoapps.bulk_email.models import CourseEmail, Optout
from lms.djangoapps.courseware.courses import get_course
from lms.djangoapps.instructor_task.models import InstructorTask
@@ -52,6 +60,7 @@ from lms.djangoapps.instructor_task.subtasks import (
queue_subtasks_for_query,
update_subtask_status
)
from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.lib.courses import course_image_url
@@ -498,16 +507,16 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
# use the email from address in the CourseEmail, if it is present, otherwise compute it.
from_addr = course_email.from_addr or _get_source_address(course_email.course_id, course_title, course_language)
# use the CourseEmailTemplate that was associated with the CourseEmail
course_email_template = course_email.get_template()
site = Site.objects.get_current()
try:
connection = get_connection()
connection.open()
# Define context values to use in all course emails:
email_context = {'name': '', 'email': ''}
email_context = {'name': '', 'email': '', 'course_email': course_email, 'from_address': from_addr}
template_context = get_base_template_context(site)
email_context.update(global_email_context)
email_context.update(template_context)
start_time = time.time()
while to_list:
@@ -519,6 +528,8 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
recipient_num += 1
current_recipient = to_list[-1]
email = current_recipient['email']
user_id = current_recipient['pk']
profile_name = current_recipient['profile__name']
if _has_non_ascii_characters(email):
to_list.pop()
total_recipients_failed += 1
@@ -530,26 +541,16 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
continue
email_context['email'] = email
email_context['name'] = current_recipient['profile__name']
email_context['user_id'] = current_recipient['pk']
email_context['name'] = profile_name
email_context['user_id'] = user_id
email_context['course_id'] = course_email.course_id
email_context['unsubscribe_link'] = get_unsubscribed_link(current_recipient['username'],
str(course_email.course_id))
# Construct message content using templates and context:
plaintext_msg = course_email_template.render_plaintext(course_email.text_message, email_context)
html_msg = course_email_template.render_htmltext(course_email.html_message, email_context)
# Create email:
email_msg = EmailMultiAlternatives(
course_email.subject,
plaintext_msg,
from_addr,
[email],
connection=connection
)
email_msg.attach_alternative(html_msg, 'text/html')
if is_bulk_email_edx_ace_enabled():
message = ACEEmail(site, email_context)
else:
message = DjangoEmail(connection, course_email, email_context)
# Throttle if we have gotten the rate limiter. This is not very high-tech,
# but if a task has been retried for rate-limiting reasons, then we sleep
# for a period of time between all emails within this task. Choice of
@@ -563,8 +564,7 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
f"BulkEmail ==> Task: {parent_task_id}, SubTask: {task_id}, EmailId: {email_id}, Recipient num: "
f"{recipient_num}/{total_recipients}, Recipient UserId: {current_recipient['pk']}"
)
connection.send_messages([email_msg])
message.send()
except SMTPDataError as exc:
# According to SMTP spec, we'll retry error codes in the 4xx range. 5xx range indicates hard failure.
total_recipients_failed += 1

View File

@@ -24,6 +24,7 @@ from common.djangoapps.student.roles import CourseStaffRole
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from common.djangoapps.student.tests.factories import InstructorFactory
from common.djangoapps.student.tests.factories import StaffFactory
from lms.djangoapps.bulk_email.messages import ACEEmail
from lms.djangoapps.bulk_email.tasks import _get_course_email_context, _get_source_address
from lms.djangoapps.instructor_task.subtasks import update_subtask_status
from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort
@@ -176,25 +177,57 @@ class LocalizedFromAddressPlatformLangTestCase(SendEmailWithMockedUgettextMixin,
"""
Tests to ensure that the bulk email has the "From" address localized according to LANGUAGE_CODE.
"""
@override_settings(LANGUAGE_CODE='en', EMAIL_USE_COURSE_ID_FROM_FOR_BULK=True)
def test_english_platform(self):
@ddt.data(
('en', True, False),
('eo', True, False),
('en', True, True),
('eo', True, True),
)
@ddt.unpack
def test_english_platform(self, language_code, enable_use_corse_id_in_from, ace_enabled):
"""
Ensures that the source-code language (English) works well.
"""
assert self.course.language is None
# Sanity check
message = self.send_email()
self.assertRegex(message.from_email, '.*Course Staff.*')
with override_settings(
LANGUAGE_CODE=language_code,
EMAIL_USE_COURSE_ID_FROM_FOR_BULK=enable_use_corse_id_in_from,
BULK_EMAIL_SEND_USING_EDX_ACE=ace_enabled
):
message = self.send_email()
self.assertRegex(message.from_email, f'{language_code.upper()} .* Course Staff')
@override_settings(LANGUAGE_CODE='eo', EMAIL_USE_COURSE_ID_FROM_FOR_BULK=True)
def test_esperanto_platform(self):
@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False})
@ddt.ddt
class AceEmailTestCase(SendEmailWithMockedUgettextMixin, EmailSendFromDashboardTestCase):
"""
Tests to ensure that the bulk email is sent using edx-ace when BULK_EMAIL_SEND_USING_EDX_ACE toggle is enabled.
"""
@ddt.data(
(True, True),
(False, False),
)
@ddt.unpack
@patch.object(ACEEmail, 'send')
def test_ace_eanbled_toggle(self, ace_enabled, email_sent_with_ace, mock_ace_email_send):
"""
Tests the fake Esperanto language to ensure proper gettext calls.
Ensures that the email message is sent via edx-ace when BULK_EMAIL_SEND_USING_EDX_ACE toggle is enabled.
"""
assert self.course.language is None
# Sanity check
message = self.send_email()
self.assertRegex(message.from_email, 'EO .* Course Staff')
mock_ace_email_send.return_value = None
test_email = {
'action': 'Send email',
'send_to': '["myself"]',
'subject': 'test subject for myself',
'message': 'test message for myself'
}
with override_settings(
BULK_EMAIL_SEND_USING_EDX_ACE=ace_enabled
):
response = self.client.post(self.send_mail_url, test_email)
self.assertEqual(email_sent_with_ace, mock_ace_email_send.called)
@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False})

View File

@@ -16,3 +16,15 @@ from edx_toggles.toggles import SettingToggle
def is_email_use_course_id_from_for_bulk_enabled():
return SettingToggle("EMAIL_USE_COURSE_ID_FROM_FOR_BULK", default=False).is_enabled()
# .. toggle_name: BULK_EMAIL_SEND_USING_EDX_ACE
# .. toggle_implementation: DjangoSetting
# .. toggle_default: False
# .. toggle_description: If True, use edx-ace to send bulk email messages
# .. toggle_use_cases: open_edx
# .. toggle_creation_date: 2022-02-10
# .. toggle_tickets: https://github.com/openedx/build-test-release-wg/issues/100
def is_bulk_email_edx_ace_enabled():
return SettingToggle("BULK_EMAIL_SEND_USING_EDX_ACE", default=False).is_enabled()

View File

@@ -0,0 +1,39 @@
{% extends 'ace_common/edx_ace/common/base_body.html' %}
{% load i18n %}
{% load static %}
{% block content %}
<table width="100%" align="left" border="0" cellpadding="0" cellspacing="0" role="presentation">
<tr>
<td>
<p style="color: rgba(0,0,0,.75);">
{% autoescape off %}
{{ course_email.html_message }}
{% endautoescape %}
<br/>
</p>
<p style="font-size: 11px;">
{% filter force_escape %}
{% blocktrans %}
This email was automatically sent from {{ platform_name }}.
{% endblocktrans %}
{% endfilter %}
<br>
{% filter force_escape %}
{% blocktrans %}
You are receiving this email at address {{ email }} because you are enrolled in
{% endblocktrans %}
{% endfilter %} <a href='{{ course_url }}'>{{ course_title }}</a>.
<br>
{% filter force_escape %}
{% blocktrans %}
To stop receiving email like this, update your course email settings
{% endblocktrans %}
{% endfilter %} <a href='{{ email_settings_url }}'>{% trans "here" as transhere %}{{transhere|force_escape}}</a>.
<br><a href='{{ unsubscribe_link }}'>{% trans "unsubscribe" as transunsub %}{{transunsub|force_escape}}</a>
</p>
</td>
</tr>
</table>
{% endblock %}

View File

@@ -0,0 +1,10 @@
{% load i18n %}
{% autoescape off %}
{{ course_email.text_message }}
{% endautoescape %}
{% blocktrans %}This email was automatically sent from {{ platform_name }}.{% endblocktrans %}
{% blocktrans %}You are receiving this email at address {{ email }} because you are enrolled in {{course_title}}.{% endblocktrans %}
{% blocktrans %}To stop receiving email like this, update your course email settings here {{ email_settings_url }}{% endblocktrans %}
{% blocktrans %}To unsubscribe click here {{ unsubscribe_link }}{% endblocktrans %}

View File

@@ -0,0 +1 @@
{{ from_address }}

View File

@@ -0,0 +1 @@
{% extends 'ace_common/edx_ace/common/base_head.html' %}

View File

@@ -0,0 +1,4 @@
{% load i18n %}
{% autoescape off %}
{{ course_email.subject }}
{% endautoescape %}