Merge pull request #30126 from openedx/jhynes/microba-1508_add-scheduledinstructortask-model
feat: add models for scheduled instructor tasks
This commit is contained in:
@@ -4,11 +4,12 @@ Python APIs exposed by the bulk_email app to other in-process apps.
|
||||
"""
|
||||
|
||||
# Public Bulk Email Functions
|
||||
|
||||
import logging
|
||||
|
||||
from django.conf import settings
|
||||
from django.urls import reverse
|
||||
|
||||
from lms.djangoapps.bulk_email.models import CourseEmail
|
||||
from lms.djangoapps.bulk_email.models_api import (
|
||||
is_bulk_email_disabled_for_course,
|
||||
is_bulk_email_enabled_for_course,
|
||||
@@ -18,6 +19,8 @@ from lms.djangoapps.bulk_email.models_api import (
|
||||
from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher
|
||||
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def get_emails_enabled(user, course_id):
|
||||
"""
|
||||
@@ -48,3 +51,39 @@ def get_unsubscribed_link(username, course_id):
|
||||
optout_url = reverse('bulk_email_opt_out', kwargs={'token': token, 'course_id': course_id})
|
||||
url = f'{lms_root_url}{optout_url}'
|
||||
return url
|
||||
|
||||
|
||||
def create_course_email(course_id, sender, targets, subject, html_message, text_message=None, template_name=None,
|
||||
from_addr=None):
|
||||
"""
|
||||
Python API for creating a new CourseEmail instance.
|
||||
|
||||
Args:
|
||||
course_id (CourseKey): The CourseKey of the course.
|
||||
sender (String): Email author.
|
||||
targets (Target): Recipient groups the message should be sent to (e.g. SEND_TO_MYSELF)
|
||||
subject (String)): Email subject.
|
||||
html_message (String): Email body. Includes HTML markup.
|
||||
text_message (String, optional): Plaintext version of email body. Defaults to None.
|
||||
template_name (String, optional): Name of custom email template to use. Defaults to None.
|
||||
from_addr (String, optional): Custom sending address, if desired. Defaults to None.
|
||||
|
||||
Returns:
|
||||
CourseEmail: Returns the created CourseEmail instance.
|
||||
"""
|
||||
try:
|
||||
course_email = CourseEmail.create(
|
||||
course_id,
|
||||
sender,
|
||||
targets,
|
||||
subject,
|
||||
html_message,
|
||||
text_message=text_message,
|
||||
template_name=template_name,
|
||||
from_addr=from_addr
|
||||
)
|
||||
|
||||
return course_email
|
||||
except ValueError as err:
|
||||
log.exception(f"Cannot create course email for {course_id} requested by user {sender} for targets {targets}")
|
||||
raise ValueError from err
|
||||
|
||||
22
lms/djangoapps/bulk_email/data.py
Normal file
22
lms/djangoapps/bulk_email/data.py
Normal file
@@ -0,0 +1,22 @@
|
||||
"""
|
||||
Bulk Email Data
|
||||
|
||||
This provides Data models to represent Bulk Email data.
|
||||
"""
|
||||
|
||||
|
||||
class BulkEmailTargetChoices:
|
||||
"""
|
||||
Enum for the available targets (recipient groups) of an email authored with the bulk course email tool.
|
||||
|
||||
SEND_TO_MYSELF - Message intended for author of the message
|
||||
SEND_TO_STAFF - Message intended for all course staff
|
||||
SEND_TO_LEARNERS - Message intended for all enrolled learners
|
||||
SEND_TO_COHORT - Message intended for a specific cohort
|
||||
SEND_TO_TRACK - Message intended for all learners in a specific track (e.g. audit or verified)
|
||||
"""
|
||||
SEND_TO_MYSELF = "myself"
|
||||
SEND_TO_STAFF = "staff"
|
||||
SEND_TO_LEARNERS = "learners"
|
||||
SEND_TO_COHORT = "cohort"
|
||||
SEND_TO_TRACK = "track"
|
||||
96
lms/djangoapps/bulk_email/tests/test_api.py
Normal file
96
lms/djangoapps/bulk_email/tests/test_api.py
Normal file
@@ -0,0 +1,96 @@
|
||||
"""
|
||||
Tests for the public Python API functions of the Bulk Email app.
|
||||
"""
|
||||
from testfixtures import LogCapture
|
||||
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import CourseFactory
|
||||
|
||||
from common.djangoapps.student.tests.factories import InstructorFactory
|
||||
from lms.djangoapps.bulk_email.api import create_course_email
|
||||
from lms.djangoapps.bulk_email.data import BulkEmailTargetChoices
|
||||
from openedx.core.lib.html_to_text import html_to_text
|
||||
|
||||
|
||||
class CreateCourseEmailTests(ModuleStoreTestCase):
|
||||
"""
|
||||
Tests for the `create_course_email` function of the bulk email app's public Python API.
|
||||
"""
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.course = CourseFactory.create()
|
||||
self.instructor = InstructorFactory(course_key=self.course.id)
|
||||
self.target = [BulkEmailTargetChoices.SEND_TO_MYSELF]
|
||||
self.subject = "email subject"
|
||||
self.html_message = "<p>test message</p>"
|
||||
|
||||
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()
|
||||
@@ -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
|
||||
)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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),
|
||||
),
|
||||
]
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
134
lms/djangoapps/instructor_task/tests/test_api_helper.py
Normal file
134
lms/djangoapps/instructor_task/tests/test_api_helper.py
Normal file
@@ -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",
|
||||
"<p>Test message.</p>"
|
||||
)
|
||||
|
||||
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)
|
||||
Reference in New Issue
Block a user