test message
" + + def test_create_course_email(self): + """ + Happy path test for the `create_course_email` function. Verifies the creation of a CourseEmail instance with + the bare minimum information required for the function call. + """ + course_email = create_course_email( + self.course.id, + self.instructor, + self.target, + self.subject, + self.html_message, + ) + + assert course_email.sender.id == self.instructor.id + assert course_email.subject == self.subject + assert course_email.html_message == self.html_message + assert course_email.course_id == self.course.id + assert course_email.text_message == html_to_text(self.html_message) + + def test_create_course_email_with_optional_args(self): + """ + Additional testing to verify that optional data is used as expected when passed into the `create_course_email` + function. + """ + text_message = "everything is awesome!" + template_name = "gnarly_template" + from_addr = "blub@noreply.fish.com" + + course_email = create_course_email( + self.course.id, + self.instructor, + self.target, + self.subject, + self.html_message, + text_message=text_message, + template_name=template_name, + from_addr=from_addr + ) + + assert course_email.sender.id == self.instructor.id + assert course_email.subject == self.subject + assert course_email.html_message == self.html_message + assert course_email.course_id == self.course.id + assert course_email.text_message == text_message + assert course_email.template_name == template_name + assert course_email.from_addr == from_addr + + def test_create_course_email_expect_exception(self): + """ + Test to verify behavior when an exception occurs when calling teh `create_course_email` function. + """ + targets = ["humpty dumpty"] + + expected_messages = [ + f"Cannot create course email for {self.course.id} requested by user {self.instructor} for targets " + f"{targets}", + ] + + with self.assertRaises(ValueError): + with LogCapture() as log: + create_course_email( + self.course.id, + self.instructor, + targets, + self.subject, + self.html_message + ) + + for index, message in enumerate(expected_messages): + assert message in log.records[index].getMessage() diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 67cc72b68b..81b9b6b487 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -2733,12 +2733,15 @@ def send_email(request, course_id): # Create the CourseEmail object. This is saved immediately, so that # any transaction that has been pending up to this point will also be # committed. + # TODO: convert to use bulk_email app's `create_course_email` API function and remove direct import and use of + # bulk_email model try: email = CourseEmail.create( course_id, request.user, targets, - subject, message, + subject, + message, template_name=template_name, from_addr=from_addr ) diff --git a/lms/djangoapps/instructor_task/api_helper.py b/lms/djangoapps/instructor_task/api_helper.py index 59096b91d0..506833e6de 100644 --- a/lms/djangoapps/instructor_task/api_helper.py +++ b/lms/djangoapps/instructor_task/api_helper.py @@ -17,7 +17,7 @@ from opaque_keys.edx.keys import UsageKey from common.djangoapps.util.db import outer_atomic from lms.djangoapps.courseware.courses import get_problems_in_section -from lms.djangoapps.instructor_task.models import PROGRESS, InstructorTask +from lms.djangoapps.instructor_task.models import PROGRESS, SCHEDULED, InstructorTask, InstructorTaskSchedule from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order log = logging.getLogger(__name__) @@ -448,3 +448,46 @@ def submit_task(request, task_type, task_class, course_key, task_input, task_key _handle_instructor_task_failure(instructor_task, error) return instructor_task + + +def schedule_task(request, task_type, course_key, task_input, task_key, schedule): + """ + Helper function to schedule a background task. + + Reserves the requested task and stores it until the task is ready for execution. We also create an instance of a + InstructorTaskSchedule object responsible for maintaining the details of _when_ a task should be executed. Extracts + arguments important to the task from the originating server request and stores them as part of the schedule object. + Sets the `task_status` to SCHEDULED to indicate this task will be executed in the future. + + Args: + request (WSGIRequest): The originating web request associated with this task request. + task_type (String): Text describing the type of task (e.g. 'bulk_course_email' or 'grade_course') + course_key (CourseKey): The CourseKey of the course-run the task belongs to. + task_input (dict): Task input arguments stores as JSON-serialized dictionary. + task_key (String): Encoded input arguments used during task execution. + schedule (DateTime): DateTime (in UTC) describing when the task should be executed. + """ + instructor_task = None + try: + log.info(f"Creating a scheduled instructor task of type '{task_type}' for course '{course_key}' requested by " + f"user with id '{request.user.id}'") + instructor_task = InstructorTask.create(course_key, task_type, task_key, task_input, request.user) + + task_id = instructor_task.task_id + task_args = _get_xmodule_instance_args(request, task_id) + log.info(f"Creating a task schedule associated with instructor task '{instructor_task.id}' and due after " + f"'{schedule}'") + InstructorTaskSchedule.objects.create( + task=instructor_task, + task_args=json.dumps(task_args), + task_due=schedule, + ) + + log.info(f"Updating task state of instructor task '{instructor_task.id}' to '{SCHEDULED}'") + instructor_task.task_state = SCHEDULED + instructor_task.save() + except Exception as error: # pylint: disable=broad-except + log.error(f"Error occurred during task or schedule creation: {error}") + # Set any orphaned instructor tasks to the FAILURE state. + if instructor_task: + _handle_instructor_task_failure(instructor_task, error) diff --git a/lms/djangoapps/instructor_task/migrations/0004_historicalinstructortaskschedule_instructortaskschedule.py b/lms/djangoapps/instructor_task/migrations/0004_historicalinstructortaskschedule_instructortaskschedule.py new file mode 100644 index 0000000000..36b6ddb186 --- /dev/null +++ b/lms/djangoapps/instructor_task/migrations/0004_historicalinstructortaskschedule_instructortaskschedule.py @@ -0,0 +1,55 @@ +# Generated by Django 3.2.12 on 2022-03-22 18:49 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import model_utils.fields +import simple_history.models + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('instructor_task', '0003_alter_task_input_field'), + ] + + operations = [ + migrations.CreateModel( + name='InstructorTaskSchedule', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('task_args', models.TextField()), + ('task_due', models.DateTimeField()), + ('task', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to='instructor_task.instructortask')), + ], + options={ + 'abstract': False, + }, + ), + migrations.CreateModel( + name='HistoricalInstructorTaskSchedule', + fields=[ + ('id', models.IntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')), + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('task_args', models.TextField()), + ('task_due', models.DateTimeField()), + ('history_id', models.AutoField(primary_key=True, serialize=False)), + ('history_date', models.DateTimeField()), + ('history_change_reason', models.CharField(max_length=100, null=True)), + ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)), + ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), + ('task', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='instructor_task.instructortask')), + ], + options={ + 'verbose_name': 'historical instructor task schedule', + 'ordering': ('-history_date', '-history_id'), + 'get_latest_by': 'history_date', + }, + bases=(simple_history.models.HistoricalChanges, models.Model), + ), + ] diff --git a/lms/djangoapps/instructor_task/models.py b/lms/djangoapps/instructor_task/models.py index 1309ac3283..92268c3090 100644 --- a/lms/djangoapps/instructor_task/models.py +++ b/lms/djangoapps/instructor_task/models.py @@ -20,13 +20,16 @@ import os.path from uuid import uuid4 from boto.exception import BotoServerError +from django.apps import apps from django.conf import settings from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.core.files.base import ContentFile from django.db import models, transaction from django.utils.translation import gettext as _ +from model_utils.models import TimeStampedModel from opaque_keys.edx.django.models import CourseKeyField +from simple_history.models import HistoricalRecords from openedx.core.storage import get_storage @@ -35,6 +38,7 @@ logger = logging.getLogger(__name__) # define custom states used by InstructorTask QUEUING = 'QUEUING' PROGRESS = 'PROGRESS' +SCHEDULED = 'SCHEDULED' TASK_INPUT_LENGTH = 10000 @@ -187,6 +191,26 @@ class InstructorTask(models.Model): return json.dumps({'message': 'Task revoked before running'}) +class InstructorTaskSchedule(TimeStampedModel): + """ + A database model to store information about _when_ to execute a scheduled background task. + + The primary use case is to allow instructors to schedule their email messages (authored with the bulk course email + tool) to be sent at a later date and time. + + .. no_pii: + """ + class Meta: + app_label = "instructor_task" + + task = models.OneToOneField(InstructorTask, on_delete=models.CASCADE) + task_args = models.TextField(null=False, blank=False) + task_due = models.DateTimeField(null=False) + + if 'instructor_task' in apps.app_configs: + history = HistoricalRecords() + + class ReportStore: """ Simple abstraction layer that can fetch and store CSV files for reports diff --git a/lms/djangoapps/instructor_task/tests/test_api.py b/lms/djangoapps/instructor_task/tests/test_api.py index 678291cc28..fd0c842944 100644 --- a/lms/djangoapps/instructor_task/tests/test_api.py +++ b/lms/djangoapps/instructor_task/tests/test_api.py @@ -205,6 +205,8 @@ class InstructorTaskCourseSubmitTest(TestReportMixin, InstructorTaskCourseTestCa def _define_course_email(self): """Create CourseEmail object for testing.""" + # TODO: convert to use bulk_email app's `create_course_email` API function and remove direct import and use of + # bulk_email model course_email = CourseEmail.create( self.course.id, self.instructor, diff --git a/lms/djangoapps/instructor_task/tests/test_api_helper.py b/lms/djangoapps/instructor_task/tests/test_api_helper.py new file mode 100644 index 0000000000..dcba37bb46 --- /dev/null +++ b/lms/djangoapps/instructor_task/tests/test_api_helper.py @@ -0,0 +1,134 @@ +""" +Tests for the Instructor Task `api_helper.py` functions. +""" +import datetime +import hashlib +import json +from unittest.mock import patch +from testfixtures import LogCapture + +from celery.states import FAILURE + +from common.djangoapps.student.tests.factories import UserFactory +from lms.djangoapps.bulk_email.api import create_course_email +from lms.djangoapps.bulk_email.data import BulkEmailTargetChoices +from lms.djangoapps.instructor_task.api_helper import QueueConnectionError, schedule_task +from lms.djangoapps.instructor_task.models import SCHEDULED, InstructorTask, InstructorTaskSchedule +from lms.djangoapps.instructor_task.tests.test_base import InstructorTaskCourseTestCase + + +class ScheduledBulkEmailInstructorTaskTests(InstructorTaskCourseTestCase): + """ + Tests for the `schedule_task` functionality, with a focus on the scheduled bulk email tasks. + """ + class FakeRequest: + """ + Test class reflecting a portion of the properties expected in a WSGIRequest. We use data from the originating + web request during execution of Instructor Tasks. + """ + def __init__(self, user): + self.user = user + self.META = { + "REMOTE_ADDR": "127.0.0.1", + 'HTTP_USER_AGENT': 'test_agent', + 'SERVER_NAME': 'test_server_name', + } + + def setUp(self): + super().setUp() + + self.initialize_course() + self.instructor = UserFactory.create(username="instructor", email="instructor@edx.org") + self.request = self.FakeRequest(self.instructor) + self.targets = [BulkEmailTargetChoices.SEND_TO_MYSELF] + self.course_email = self._create_course_email(self.targets) + self.schedule = datetime.datetime.now(datetime.timezone.utc) + self.task_type = "bulk_course_email" + self.task_input = json.dumps(self._generate_bulk_email_task_input(self.course_email, self.targets)) + self.task_key = hashlib.md5(str(self.course_email.id).encode('utf-8')).hexdigest() + + def _create_course_email(self, targets): + """ + Create CourseEmail object for testing. + """ + course_email = create_course_email( + self.course.id, + self.instructor, + targets, + "Test Subject", + "Test message.
" + ) + + return course_email + + def _generate_bulk_email_task_input(self, course_email, targets): + return { + "email_id": course_email.id, + "to_option": targets + } + + def _verify_log_messages(self, expected_messages, log): + for index, message in enumerate(expected_messages): + assert message in log.records[index].getMessage() + + def test_create_scheduled_instructor_task(self): + """ + Happy path test for the `schedule_task` function. Verifies that we create an InstructorTask instance and an + associated InstructorTaskSchedule instance as expected. + """ + with LogCapture() as log: + schedule_task(self.request, self.task_type, self.course.id, self.task_input, self.task_key, self.schedule) + + # get the task instance and its associated schedule for verifications + task = InstructorTask.objects.get(course_id=self.course.id, task_key=self.task_key) + task_schedule = InstructorTaskSchedule.objects.get(task=task) + expected_task_args = { + "request_info": { + "username": self.instructor.username, + "user_id": self.instructor.id, + "ip": "127.0.0.1", + "agent": "test_agent", + "host": "test_server_name", + }, + "task_id": task.task_id + } + expected_messages = [ + f"Creating a scheduled instructor task of type '{self.task_type}' for course '{self.course.id}' requested " + f"by user with id '{self.request.user.id}'", + f"Creating a task schedule associated with instructor task '{task.id}' and due after '{self.schedule}'", + f"Updating task state of instructor task '{task.id}' to '{SCHEDULED}'" + ] + # convert from text back to JSON before comparison + actual_task_args = json.loads(task_schedule.task_args) + + # verify the task has the correct state + assert task.task_state == SCHEDULED + # verify that the schedule is associated with the correct task_id (UUID) + assert task_schedule.task_id == task.id + # verify that the schedule is the expected date and time + assert task_schedule.task_due == self.schedule + # verify the task_arguments are as expected + assert expected_task_args == actual_task_args + self._verify_log_messages(expected_messages, log) + + @patch("lms.djangoapps.instructor_task.api_helper._get_xmodule_instance_args", side_effect=Exception("boom!")) + def test_create_scheduled_instructor_task_expect_failure(self, mock_get_xmodule_instance_args): + """ + A test to verify that we will mark a task as `FAILED` if a failure occurs during the creation of the task + schedule. + """ + expected_messages = [ + f"Creating a scheduled instructor task of type '{self.task_type}' for course '{self.course.id}' requested " + f"by user with id '{self.request.user.id}'", + "Error occurred during task or schedule creation: boom!", + ] + + with self.assertRaises(QueueConnectionError): + with LogCapture() as log: + schedule_task( + self.request, self.task_type, self.course.id, self.task_input, self.task_key, self.schedule + ) + + task = InstructorTask.objects.get(course_id=self.course.id, task_key=self.task_key) + assert task.task_state == FAILURE + self._verify_log_messages(expected_messages, log)