Merge pull request #29 from edx/aj/fix-schedule-for-user

fix schedules for user view.

AUTHORS

@@ -1 +1,2 @@
+Awais Jibran <[email protected]>
 Dave St.Germain <[email protected]>

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

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:

edx_when/models.py

@@ -8,15 +8,15 @@
 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 _
 from model_utils.models import TimeStampedModel
 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:

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):

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