From 97f39f697253f5ef336552540516487e33af1de4 Mon Sep 17 00:00:00 2001 From: bszabo Date: Fri, 8 Dec 2023 10:08:37 -0500 Subject: [PATCH] Bszabo/tnl 11230 library content emails (#33899) * feat: suppress emails * feat: TNL-11230 No Lib reference Work around dependency failure * feat: TNL-11230 cleanup * feat: TNL-11230 pylint comment line too long --------- Co-authored-by: Bernard Szabo --- cms/djangoapps/cms_user_tasks/signals.py | 27 +++++++++++++++++++++++- cms/djangoapps/cms_user_tasks/tests.py | 13 ++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/cms_user_tasks/signals.py b/cms/djangoapps/cms_user_tasks/signals.py index 8011f2be1c..b4f86807fd 100644 --- a/cms/djangoapps/cms_user_tasks/signals.py +++ b/cms/djangoapps/cms_user_tasks/signals.py @@ -2,6 +2,7 @@ Receivers of signals sent from django-user-tasks """ import logging +import re from urllib.parse import urljoin from django.dispatch import receiver @@ -15,6 +16,7 @@ from cms.djangoapps.contentstore.utils import course_import_olx_validation_is_en from .tasks import send_task_complete_email LOGGER = logging.getLogger(__name__) +LIBRARY_CONTENT_TASK_NAME_TEMPLATE = 'updating .*type@library_content.* from library' @receiver(user_task_stopped, dispatch_uid="cms_user_task_stopped") @@ -33,6 +35,22 @@ def user_task_stopped_handler(sender, **kwargs): # pylint: disable=unused-argum Returns: None """ + + def is_library_content_update(task_name: str) -> bool: + """ + Decides whether to suppress an end-of-task email on the basis that the just-ended task was a library content + XBlock update operation, and that emails following such operations amount to spam + Arguments: + task_name: The name of the just-ended task. By convention, if this was a library content XBlock update + task, then the task name follows the pattern prescribed in LibrarySyncChildrenTask + (content_libraries under openedx) 'Updating {key} from library'. Moreover, the block type + in the task name is always of type 'library_content' for such operations + Returns: + True if the end-of-task email should be suppressed + """ + p = re.compile(LIBRARY_CONTENT_TASK_NAME_TEMPLATE) + return p.match(task_name) + def get_olx_validation_from_artifact(): """ Get olx validation error if available for current task. @@ -47,9 +65,17 @@ def user_task_stopped_handler(sender, **kwargs): # pylint: disable=unused-argum return olx_artifact.text status = kwargs['status'] + # Only send email when the entire task is complete, should only send when # a chain / chord / etc completes, not on sub-tasks. if status.parent is None: + task_name = status.name.lower() + + # Also suppress emails on library content XBlock updates (too much like spam) + if is_library_content_update(task_name): + LOGGER.info(f"Suppressing end-of-task email on task {task_name}") + return + # `name` and `status` are not unique, first is our best guess artifact = UserTaskArtifact.objects.filter(status=status, name="BASE_URL").first() @@ -61,7 +87,6 @@ def user_task_stopped_handler(sender, **kwargs): # pylint: disable=unused-argum ) user_email = status.user.email - task_name = status.name.lower() olx_validation_text = get_olx_validation_from_artifact() task_args = [task_name, str(status.state_text), user_email, detail_url, olx_validation_text] try: diff --git a/cms/djangoapps/cms_user_tasks/tests.py b/cms/djangoapps/cms_user_tasks/tests.py index f50e5002fe..feeac4db09 100644 --- a/cms/djangoapps/cms_user_tasks/tests.py +++ b/cms/djangoapps/cms_user_tasks/tests.py @@ -208,6 +208,19 @@ class TestUserTaskStopped(APITestCase): self.assert_msg_subject(msg) self.assert_msg_body_fragments(msg, body_fragments) + def test_email_not_sent_with_libary_content_update(self): + """ + Check the signal receiver and email sending. + """ + UserTaskArtifact.objects.create( + status=self.status, name='BASE_URL', url='https://test.edx.org/' + ) + end_of_task_status = self.status + end_of_task_status.name = "updating block-v1:course+type@library_content+block@uuid from library" + user_task_stopped.send(sender=UserTaskStatus, status=end_of_task_status) + + self.assertEqual(len(mail.outbox), 0) + def test_email_sent_with_olx_validations_with_config_enabled(self): """ Tests that email is sent with olx validation errors.