From 4343080c06b77379081efbe30fa0112db9790656 Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Fri, 28 Feb 2020 21:33:57 +0500 Subject: [PATCH] fix schedules for user view. --- AUTHORS | 1 + edx_when/__init__.py | 2 +- edx_when/api.py | 5 +---- edx_when/models.py | 32 ++++++++++++++++++++------------ tests/test_api.py | 16 +++++++++------- tests/test_models.py | 9 ++++++++- 6 files changed, 40 insertions(+), 25 deletions(-) diff --git a/AUTHORS b/AUTHORS index da426df..6e4a32d 100644 --- a/AUTHORS +++ b/AUTHORS @@ -1 +1,2 @@ +Awais Jibran Dave St.Germain diff --git a/edx_when/__init__.py b/edx_when/__init__.py index 73d15e5..0539fbd 100644 --- a/edx_when/__init__.py +++ b/edx_when/__init__.py @@ -4,6 +4,6 @@ from __future__ import absolute_import, unicode_literals -__version__ = '1.0.2' +__version__ = '1.0.3' default_app_config = 'edx_when.apps.EdxWhenConfig' # pylint: disable=invalid-name diff --git a/edx_when/api.py b/edx_when/api.py index 59240ec..7d6fbc1 100644 --- a/edx_when/api.py +++ b/edx_when/api.py @@ -114,10 +114,7 @@ def get_dates_for_course(course_id, user=None, use_cached=True, schedule=None): policies = {} for cdate in qset: if schedule is None and user is not None: - try: - schedule = cdate.schedule_for_user(user) - except ObjectDoesNotExist: - schedule = None + schedule = cdate.schedule_for_user(user) key = (cdate.location, cdate.field) try: diff --git a/edx_when/models.py b/edx_when/models.py index 155e9e3..a42e904 100644 --- a/edx_when/models.py +++ b/edx_when/models.py @@ -8,7 +8,7 @@ from datetime import datetime from django.contrib.auth import get_user_model -from django.core.exceptions import ValidationError +from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.db import models from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import gettext_lazy as _ @@ -16,7 +16,7 @@ from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField try: - from openedx.core.djangoapps.schedule import Schedule + from openedx.core.djangoapps.schedules.models import Schedule # TODO: Move schedules into edx-when except ImportError: Schedule = None @@ -92,19 +92,25 @@ def __str__(self): def schedule_for_user(self, user): """ - Return the schedule for the supplied user that applies to this piece of content. + Return the schedule for the supplied user that applies to this piece of content or None. """ + no_schedules_found = None if isinstance(user, int): if Schedule is None: - return None - - return Schedule.objects.get(enrollment__user__id=user, enrollment__course__id=self.course_id) + return no_schedules_found + try: + return Schedule.objects.get(enrollment__user__id=user, enrollment__course__id=self.course_id) + except Schedule.DoesNotExist: + return no_schedules_found else: if not hasattr(user, 'courseenrollment_set'): - return None + return no_schedules_found - # TODO: This will break prefetching, if the user object already had enrollments/schedules prefetched - return user.courseenrollment_set.get(course__id=self.course_id).schedule + try: + # TODO: This will break prefetching, if the user object already had enrollments/schedules prefetched + return user.courseenrollment_set.get(course__id=self.course_id).schedule + except ObjectDoesNotExist: + return no_schedules_found @python_2_unicode_compatible @@ -132,8 +138,9 @@ def actual_date(self): if self.abs_date: return self.abs_date - policy_date = self.content_date.policy.actual_date(self.content_date.schedule_for_user(self.user)) - if self.rel_date: + schedule_for_user = self.content_date.schedule_for_user(self.user) + policy_date = self.content_date.policy.actual_date(schedule_for_user) + if schedule_for_user and self.rel_date: return policy_date + self.rel_date else: return policy_date @@ -152,7 +159,8 @@ def clean(self): if self.abs_date and self.rel_date: raise ValidationError(_("Absolute and relative dates cannot both be used")) - policy_date = self.content_date.policy.actual_date(self.content_date.schedule_for_user(self.user)) + user_schedule = self.content_date.schedule_for_user(self.user) + policy_date = self.content_date.policy.actual_date(schedule=user_schedule) if self.rel_date is not None and self.rel_date.total_seconds() < 0: raise ValidationError(_("Override date must be later than policy date")) if self.abs_date is not None and isinstance(policy_date, datetime) and self.abs_date < policy_date: diff --git a/tests/test_api.py b/tests/test_api.py index f634cb4..98cc311 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -91,25 +91,27 @@ def test_get_dates_for_course(self): def test_get_dates_no_schedule(self): items = make_items(with_relative=True) - api.set_dates_for_course(items[0][0].course_key, items) - retrieved = api.get_dates_for_course(items[0][0].course_key, user=self.user) + course_key = items[0][0].course_key + api.set_dates_for_course(course_key, items) + retrieved = api.get_dates_for_course(course_key, user=self.user) assert len(retrieved) == 6 self.schedule.delete() - retrieved = api.get_dates_for_course(items[0][0].course_key, user=self.user, use_cached=False) + retrieved = api.get_dates_for_course(course_key, user=self.user, use_cached=False) assert len(retrieved) == 3 def test_get_user_date_no_schedule(self): items = make_items() - api.set_dates_for_course(items[0][0].course_key, items) - before_override = api.get_dates_for_course(items[0][0].course_key, user=self.user) + course_key = items[0][0].course_key + api.set_dates_for_course(course_key, items) + before_override = api.get_dates_for_course(course_key, user=self.user) assert len(before_override) == 3 # Override a date for the user with a relative date, but remove the schedule # so that the override can't be applied - api.set_date_for_block(items[0][0].course_key, items[0][0], 'due', timedelta(days=2), user=self.user) + api.set_date_for_block(course_key, items[0][0], 'due', timedelta(days=2), user=self.user) self.schedule.delete() - after_override = api.get_dates_for_course(items[0][0].course_key, user=self.user, use_cached=False) + after_override = api.get_dates_for_course(course_key, user=self.user, use_cached=False) assert before_override == after_override def test_clear_dates_for_course(self): diff --git a/tests/test_models.py b/tests/test_models.py index 9e1d742..ab560c8 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -10,7 +10,7 @@ import ddt from django.contrib.auth import get_user_model -from django.core.exceptions import ValidationError +from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.test import TestCase from mock import Mock, patch @@ -78,6 +78,13 @@ def setUp(self): def test_schedule_for_user_id(self): assert self.content_date.schedule_for_user(self.user.id) == self.schedule + @patch('edx_when.models.Schedule', DummySchedule) + def test_schedule_for_user_with_object_does_not_exist(self): + """Test that None is returned when schedules are fetched for user.""" + mock_user = Mock(wraps=self.user) + mock_user.courseenrollment_set.get.side_effect = ObjectDoesNotExist() + assert self.content_date.schedule_for_user(mock_user) is None + @patch('edx_when.models.Schedule', None) def test_schedule_for_user_id_no_schedule_installed(self): assert self.content_date.schedule_for_user(self.user.id) is None