Skip to content

Commit

Permalink
fix: update API endpoint to return task due time for future scheduled…
Browse files Browse the repository at this point in the history
… tasks (#34332)

* fix: update API endpoint to return task due time for future scheduled tasks

* test: updated InstructorEmailContentList tests to accomodate changes

* fix: returend unformatted created date to support easy conversion at frontend

* test: updated tests to accommodate unformatted date value in API response

* refactor: removed duplicate code and use of random numbers

---------

Co-authored-by: eemaanamir <eemaan.amir@gmail.com>
  • Loading branch information
ayesha-waris and eemaanamir authored Mar 7, 2024
1 parent 1dab744 commit c848767
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 10 deletions.
24 changes: 19 additions & 5 deletions lms/djangoapps/instructor/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
generate_already_running_error_message
)
from lms.djangoapps.instructor_task.data import InstructorTaskTypes
from lms.djangoapps.instructor_task.models import InstructorTask, InstructorTaskSchedule
from lms.djangoapps.program_enrollments.tests.factories import ProgramEnrollmentFactory
from openedx.core.djangoapps.course_date_signals.handlers import extract_dates
from openedx.core.djangoapps.course_groups.cohorts import set_course_cohorted
Expand Down Expand Up @@ -3773,7 +3774,7 @@ def setUp(self):
self.emails = {}
self.emails_info = {}

def setup_fake_email_info(self, num_emails, with_failures=False):
def setup_fake_email_info(self, num_emails, with_failures=False, with_schedules=False):
""" Initialize the specified number of fake emails """
for email_id in range(num_emails):
num_sent = random.randint(1, 15401)
Expand All @@ -3785,15 +3786,25 @@ def setup_fake_email_info(self, num_emails, with_failures=False):
self.tasks[email_id] = FakeContentTask(email_id, num_sent, failed, 'expected')
self.emails[email_id] = FakeEmail(email_id)
self.emails_info[email_id] = FakeEmailInfo(self.emails[email_id], num_sent, failed)
if with_schedules and email_id % 2 == 0:
instructor_task = InstructorTask.create(self.course.id, self.tasks[email_id].task_type,
self.tasks[email_id].task_key, self.tasks[email_id].task_input,
self.instructor)
schedule = "2099-05-02T14:00:00.000Z"
InstructorTaskSchedule.objects.create(
task=instructor_task,
task_args=json.dumps(self.tasks[email_id].task_output),
task_due=schedule,
)

def get_matching_mock_email(self, **kwargs):
""" Returns the matching mock emails for the given id """
email_id = kwargs.get('id', 0)
return self.emails[email_id]

def get_email_content_response(self, num_emails, task_history_request, with_failures=False):
def get_email_content_response(self, num_emails, task_history_request, with_failures=False, with_schedules=False):
""" Calls the list_email_content endpoint and returns the respsonse """
self.setup_fake_email_info(num_emails, with_failures)
self.setup_fake_email_info(num_emails, with_failures, with_schedules)
task_history_request.return_value = list(self.tasks.values())
url = reverse('list_email_content', kwargs={'course_id': str(self.course.id)})
with patch(
Expand All @@ -3804,9 +3815,9 @@ def get_email_content_response(self, num_emails, task_history_request, with_fail
assert response.status_code == 200
return response

def check_emails_sent(self, num_emails, task_history_request, with_failures=False):
def check_emails_sent(self, num_emails, task_history_request, with_failures=False, with_schedules=False):
""" Tests sending emails with or without failures """
response = self.get_email_content_response(num_emails, task_history_request, with_failures)
response = self.get_email_content_response(num_emails, task_history_request, with_failures, with_schedules)
assert task_history_request.called
expected_email_info = [email_info.to_dict() for email_info in self.emails_info.values()]
actual_email_info = json.loads(response.content.decode('utf-8'))['emails']
Expand Down Expand Up @@ -3844,6 +3855,7 @@ def test_content_list_no_emails(self, task_history_request):
def test_content_list_email_content_many(self, task_history_request):
""" Test listing of bulk emails sent large amount of emails """
self.check_emails_sent(50, task_history_request)
self.check_emails_sent(50, task_history_request, False, True)

def test_list_email_content_error(self, task_history_request):
""" Test handling of error retrieving email """
Expand All @@ -3864,10 +3876,12 @@ def test_list_email_content_error(self, task_history_request):
def test_list_email_with_failure(self, task_history_request):
""" Test the handling of email task that had failures """
self.check_emails_sent(1, task_history_request, True)
self.check_emails_sent(1, task_history_request, True, True)

def test_list_many_emails_with_failures(self, task_history_request):
""" Test the handling of many emails with failures """
self.check_emails_sent(50, task_history_request, True)
self.check_emails_sent(50, task_history_request, True, True)

def test_list_email_with_no_successes(self, task_history_request):
task_info = FakeContentTask(0, 0, 10, 'expected')
Expand Down
7 changes: 4 additions & 3 deletions lms/djangoapps/instructor/tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@

from pytz import UTC

from common.djangoapps.util.date_utils import get_default_time_display


class FakeInfo:
"""Parent class for faking objects used in tests"""
Expand All @@ -35,6 +33,9 @@ class FakeContentTask(FakeInfo):

def __init__(self, email_id, num_sent, num_failed, sent_to): # lint-amnesty, pylint: disable=unused-argument
super().__init__()
self.task_id = random.randint(1, 15401)
self.task_type = 'test_task'
self.task_key = random.randint(1, 15401)
self.task_input = {'email_id': email_id}
self.task_input = json.dumps(self.task_input)
self.task_output = {'succeeded': num_sent, 'failed': num_failed}
Expand Down Expand Up @@ -102,7 +103,7 @@ class FakeEmailInfo(FakeInfo):

def __init__(self, fake_email, num_sent, num_failed):
super().__init__()
self.created = get_default_time_display(fake_email.created)
self.created = fake_email.created.strftime("%Y-%m-%dT%H:%M:%SZ")

number_sent = str(num_sent) + ' sent'
if num_failed > 0:
Expand Down
10 changes: 8 additions & 2 deletions lms/djangoapps/instructor/views/instructor_task_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
from django.utils.translation import gettext as _
from django.utils.translation import ngettext

from common.djangoapps.util.date_utils import get_default_time_display
from lms.djangoapps.bulk_email.models import CourseEmail
from lms.djangoapps.instructor_task.views import get_task_completion_info
from lms.djangoapps.instructor_task.models import InstructorTaskSchedule

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -60,10 +60,16 @@ def extract_email_features(email_task):

email = CourseEmail.objects.get(id=task_input_information['email_id'])
email_feature_dict = {
'created': get_default_time_display(email.created),
'created': email.created,
'sent_to': [target.long_display() for target in email.targets.all()],
'requester': str(email_task.requester),
}
try:
instructor_task_schedule = InstructorTaskSchedule.objects.get(task__task_id=email_task.task_id)
scheduled_time = instructor_task_schedule.task_due
email_feature_dict['created'] = scheduled_time
except InstructorTaskSchedule.DoesNotExist:
pass
features = ['subject', 'html_message', 'id']
email_info = {feature: str(getattr(email, feature)) for feature in features}

Expand Down

0 comments on commit c848767

Please sign in to comment.