diff --git a/cms/djangoapps/cms_user_tasks/signals.py b/cms/djangoapps/cms_user_tasks/signals.py index 40bfd57818..e6ddba747d 100644 --- a/cms/djangoapps/cms_user_tasks/signals.py +++ b/cms/djangoapps/cms_user_tasks/signals.py @@ -12,6 +12,7 @@ from user_tasks.signals import user_task_stopped from cms.djangoapps.contentstore.toggles import bypass_olx_failure_enabled from cms.djangoapps.contentstore.utils import course_import_olx_validation_is_enabled +from openedx.core.djangoapps.content_libraries.api import is_library_backup_task, is_library_restore_task from .tasks import send_task_complete_email @@ -64,6 +65,28 @@ def user_task_stopped_handler(sender, **kwargs): # pylint: disable=unused-argum if olx_artifact and not bypass_olx_failure_enabled(): return olx_artifact.text + def should_skip_end_of_task_email(task_name) -> bool: + """ + Studio tasks generally send an email when finished, but not always. + + Some tasks can last many minutes, e.g. course import/export. For these + tasks, there is a high chance that the user has navigated away and will + want to check back in later. Yet email notification is unnecessary and + distracting for things like the Library restore task, which is + relatively quick and cannot be resumed (i.e. if you navigate away, you + have to upload again). + + The task_name passed in will be lowercase. + """ + # We currently have to pattern match on the name to differentiate + # between tasks. A better long term solution would be to add a separate + # task type identifier field to Django User Tasks. + return ( + is_library_content_update(task_name) or + is_library_backup_task(task_name) or + is_library_restore_task(task_name) + ) + status = kwargs['status'] # Only send email when the entire task is complete, should only send when @@ -72,7 +95,7 @@ def user_task_stopped_handler(sender, **kwargs): # pylint: disable=unused-argum task_name = status.name.lower() # Also suppress emails on library content XBlock updates (too much like spam) - if is_library_content_update(task_name): + if should_skip_end_of_task_email(task_name): LOGGER.info(f"Suppressing end-of-task email on task {task_name}") return diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index bb4bde44ee..f5d60e9fc8 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -108,6 +108,8 @@ __all__ = [ "get_backup_task_status", "assign_library_role_to_user", "user_has_permission_across_lib_authz_systems", + "is_library_backup_task", + "is_library_restore_task", ] @@ -852,3 +854,15 @@ def _is_legacy_permission(permission: str) -> bool: or the new openedx-authz system. """ return permission in LEGACY_LIB_PERMISSIONS + + +def is_library_backup_task(task_name: str) -> bool: + """Case-insensitive match to see if a task is a library backup.""" + from ..tasks import LibraryBackupTask # avoid circular import error + return task_name.startswith(LibraryBackupTask.NAME_PREFIX.lower()) + + +def is_library_restore_task(task_name: str) -> bool: + """Case-insensitive match to see if a task is a library restore.""" + from ..tasks import LibraryRestoreTask # avoid circular import error + return task_name.startswith(LibraryRestoreTask.NAME_PREFIX.lower()) diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index c54779a8e6..bbbf847bfb 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -512,6 +512,7 @@ class LibraryBackupTask(UserTask): # pylint: disable=abstract-method """ Base class for tasks related with Library backup functionality. """ + NAME_PREFIX = "Library Learning Package Backup" @classmethod def generate_name(cls, arguments_dict) -> str: @@ -529,7 +530,7 @@ class LibraryBackupTask(UserTask): # pylint: disable=abstract-method str: The generated name """ key = arguments_dict['library_key_str'] - return f'Backup of {key}' + return f'{cls.NAME_PREFIX} of {key}' @shared_task(base=LibraryBackupTask, bind=True) @@ -591,10 +592,12 @@ class LibraryRestoreTask(UserTask): ERROR_LOG_ARTIFACT_NAME = 'Error log' + NAME_PREFIX = "Library Learning Package Restore" + @classmethod def generate_name(cls, arguments_dict): storage_path = arguments_dict['storage_path'] - return f'learning package restore of {storage_path}' + return f'{cls.NAME_PREFIX} of {storage_path}' def fail_with_error_log(self, logfile) -> None: """ diff --git a/openedx/core/djangoapps/content_libraries/tests/test_tasks.py b/openedx/core/djangoapps/content_libraries/tests/test_tasks.py index ac64bc86ef..a200471c00 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_tasks.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_tasks.py @@ -1,6 +1,7 @@ """ Unit tests for content libraries Celery tasks """ +from unittest import mock from django.test import override_settings from ..models import ContentLibrary @@ -14,6 +15,7 @@ class ContentLibraryBackupTaskTest(ContentLibrariesRestApiTest): """ Tests for Content Library export task. """ + SEND_TASK_COMPLETE_FN = 'cms.djangoapps.cms_user_tasks.tasks.send_task_complete_email.delay' def setUp(self) -> None: super().setUp() @@ -31,7 +33,9 @@ class ContentLibraryBackupTaskTest(ContentLibrariesRestApiTest): @override_settings(CMS_BASE="test.com") def test_backup_task_success(self): - result = backup_library.delay(self.user.id, str(self.lib1.library_key)) + with mock.patch(self.SEND_TASK_COMPLETE_FN) as send_task_complete_email: + result = backup_library.delay(self.user.id, str(self.lib1.library_key)) + send_task_complete_email.assert_not_called() assert result.state == 'SUCCESS' # Ensure an artifact was created with the output file artifact = UserTaskArtifact.objects.filter(status__task_id=result.task_id, name='Output').first() @@ -44,7 +48,9 @@ class ContentLibraryBackupTaskTest(ContentLibrariesRestApiTest): assert b'origin_server = "test.com"' in content def test_backup_task_failure(self): - result = backup_library.delay(self.user.id, self.wrong_task_id) + with mock.patch(self.SEND_TASK_COMPLETE_FN) as send_task_complete_email: + result = backup_library.delay(self.user.id, self.wrong_task_id) + send_task_complete_email.assert_not_called() assert result.state == 'FAILURE' # Ensure an error artifact was created artifact = UserTaskArtifact.objects.filter(status__task_id=result.task_id, name='Error').first()