Merge pull request #24 from edx/mikix/add-relative-date-toggle

Enable relative dates based off waffle

.annotation_safe_list.yml

@@ -17,10 +17,3 @@ auth.User:
   ".. pii_retirement:" : consumer_api
 contenttypes.ContentType:
   ".. no_pii:": "This model has no PII"
-# Via waffle
-waffle.Flag:
-  ".. no_pii:": "No PII"
-waffle.Sample:
-  ".. no_pii:": "No PII"
-waffle.Switch:
-  ".. no_pii:": "No PII"

edx_when/__init__.py

@@ -4,6 +4,6 @@
 
 from __future__ import absolute_import, unicode_literals
 
-__version__ = '0.7.1'
+__version__ = '1.0.0'
 
 default_app_config = 'edx_when.apps.EdxWhenConfig'  # pylint: disable=invalid-name

edx_when/api.py

@@ -25,19 +25,25 @@ def _ensure_key(key_class, key_obj):
     return key_obj
 
 
-def is_enabled_for_course(course_key):
+def _are_relative_dates_enabled(course_key=None):
     """
-    Return whether edx-when is enabled for this course.
+    Return whether it's OK to consider relative dates. If not, pretend those database entries don't exist.
     """
-    return models.ContentDate.objects.filter(course_id=course_key, active=True).exists()
+    try:
+        # It's bad form to depend on LMS code from inside a plugin like this. But we gracefully fail, and this is
+        # temporary code anyway, while we develop this feature.
+        from openedx.features.course_experience import RELATIVE_DATES_FLAG
+    except ImportError:
+        return False
 
+    return RELATIVE_DATES_FLAG.is_enabled(course_key)
 
-def override_enabled():
+
+def is_enabled_for_course(course_key):
     """
-    Return decorator that enables edx-when.
+    Return whether edx-when is enabled for this course.
     """
-    from waffle.testutils import override_flag
-    return override_flag('edx-when-enabled', active=True)
+    return models.ContentDate.objects.filter(course_id=course_key, active=True).exists()
 
 
 def set_dates_for_course(course_key, items):
@@ -72,8 +78,16 @@ def get_dates_for_course(course_id, user=None, use_cached=True, schedule=None):
 
         key: block location, field name
         value: datetime object
+
+    Arguments:
+        course_id: either a CourseKey or string representation of same
+        user: None, an int, or a User object
+        use_cached: will skip cache lookups (but not saves) if False
+        schedule: optional override for a user's enrollment Schedule, used for relative date calculations
     """
+    course_id = _ensure_key(CourseKey, course_id)
     log.debug("Getting dates for %s as %s", course_id, user)
+    allow_relative_dates = _are_relative_dates_enabled(course_id)
 
     cache_key = 'course_dates.%s' % course_id
     if user:
@@ -84,13 +98,18 @@ def get_dates_for_course(course_id, user=None, use_cached=True, schedule=None):
         cache_key += '.%s' % user_id
     else:
         user_id = None
+    if schedule:
+        cache_key += '.schedule-%s' % schedule.start_date
+    if allow_relative_dates:
+        cache_key += '.with-rel-dates'
 
     dates = DEFAULT_REQUEST_CACHE.data.get(cache_key, None)
     if use_cached and dates is not None:
         return dates
 
-    course_id = _ensure_key(CourseKey, course_id)
-    qset = models.ContentDate.objects.filter(course_id=course_id, active=True).select_related('policy')
+    rel_lookup = {} if allow_relative_dates else {'policy__rel_date': None}
+    qset = models.ContentDate.objects.filter(course_id=course_id, active=True, **rel_lookup).select_related('policy')
+
     dates = {}
     policies = {}
     for cdate in qset:
@@ -106,11 +125,14 @@ def get_dates_for_course(course_id, user=None, use_cached=True, schedule=None):
         except ValueError:
             log.warning("Unable to read date for %s", cdate.location, exc_info=True)
         policies[cdate.id] = key
+
     if user_id:
+        rel_lookup = {} if allow_relative_dates else {'content_date__policy__rel_date': None, 'rel_date': None}
         for userdate in models.UserDate.objects.filter(
             user_id=user_id,
             content_date__course_id=course_id,
-            content_date__active=True
+            content_date__active=True,
+            **rel_lookup,
         ).select_related(
             'content_date', 'content_date__policy'
         ).order_by('modified'):
@@ -127,6 +149,11 @@ def get_dates_for_course(course_id, user=None, use_cached=True, schedule=None):
 def get_date_for_block(course_id, block_id, name='due', user=None):
     """
     Return the date for block in the course for the (optional) user.
+
+    Arguments:
+        course_id: either a CourseKey or string representation of same
+        block_id: either a UsageKey or string representation of same
+        user: None, an int, or a User object
     """
     try:
         return get_dates_for_course(course_id, user).get((_ensure_key(UsageKey, block_id), name), None)
@@ -138,15 +165,24 @@ def get_overrides_for_block(course_id, block_id):
     """
     Return list of date overrides for a block.
 
-    list of (username, full_name, date)
+    Arguments:
+        course_id: either a CourseKey or string representation of same
+        block_id: either a UsageKey or string representation of same
+
+    Returns:
+        list of (username, full_name, date)
     """
     course_id = _ensure_key(CourseKey, course_id)
     block_id = _ensure_key(UsageKey, block_id)
 
+    allow_relative_dates = _are_relative_dates_enabled(course_id)
+    rel_lookup = {} if allow_relative_dates else {'content_date__policy__rel_date': None, 'rel_date': None}
     query = models.UserDate.objects.filter(
-                content_date__course_id=course_id,
-                content_date__location=block_id,
-                content_date__active=True).order_by('-modified')
+        content_date__course_id=course_id,
+        content_date__location=block_id,
+        content_date__active=True,
+        **rel_lookup,
+    ).order_by('-modified')
     dates = []
     users = set()
     for udate in query:
@@ -168,14 +204,23 @@ def get_overrides_for_user(course_id, user):
     """
     Return all user date overrides for a particular course.
 
-    iterator of {'location': location, 'actual_date': date}
+    Arguments:
+        course_id: either a CourseKey or string representation of same
+        user: a User object
+
+    Returns:
+        iterator of {'location': location, 'actual_date': date}
     """
     course_id = _ensure_key(CourseKey, course_id)
 
+    allow_relative_dates = _are_relative_dates_enabled(course_id)
+    rel_lookup = {} if allow_relative_dates else {'abs_date__isnull': False}
     query = models.UserDate.objects.filter(
-                content_date__course_id=course_id,
-                user=user,
-                content_date__active=True).order_by('-modified')
+        content_date__course_id=course_id,
+        user=user,
+        content_date__active=True,
+        **rel_lookup,
+    ).order_by('-modified')
     blocks = set()
     for udate in query:
         if udate.content_date.location in blocks:

edx_when/models.py

@@ -25,7 +25,7 @@
 @python_2_unicode_compatible
 class DatePolicy(TimeStampedModel):
     """
-    TODO: replace with a brief description of the model.
+    Stores a date (either absolute or relative).
 
     .. no_pii:
     """
@@ -67,7 +67,7 @@ def clean(self):
 @python_2_unicode_compatible
 class ContentDate(models.Model):
     """
-    TODO: replace with a brief description of the model.
+    Ties a DatePolicy to a specific piece of course content. (e.g. a due date for a homework).
 
     .. no_pii:
     """
@@ -110,7 +110,7 @@ def schedule_for_user(self, user):
 @python_2_unicode_compatible
 class UserDate(TimeStampedModel):
     """
-    TODO: replace with a brief description of the model.
+    Stores a user-specific date override for a given ContentDate.
 
     .. no_pii:
     """

test_settings.py

@@ -32,7 +32,6 @@ def root(*args):
     'django.contrib.auth',
     'django.contrib.contenttypes',
     'edx_when',
-    'waffle',
     'tests.test_models_app',
 )
 

tests/test_api.py

@@ -3,14 +3,16 @@
 """
 from __future__ import absolute_import, unicode_literals
 
+import sys
 from datetime import datetime, timedelta
 
 import ddt
 import six
 from django.contrib.auth.models import User
 from django.test import TestCase
 from edx_django_utils.cache.utils import DEFAULT_REQUEST_CACHE
-from mock import patch
+from mock import Mock, patch
+from opaque_keys.edx.locator import CourseLocator
 
 from edx_when import api, models
 from test_utils import make_block_id, make_items
@@ -42,6 +44,10 @@ def setUp(self):
         dummy_schedule_patcher.start()
         self.addCleanup(dummy_schedule_patcher.stop)
 
+        relative_dates_patcher = patch('edx_when.api._are_relative_dates_enabled', return_value=True)
+        relative_dates_patcher.start()
+        self.addCleanup(relative_dates_patcher.stop)
+
         DEFAULT_REQUEST_CACHE.clear()
 
     def test_set_dates_for_course(self):
@@ -213,3 +219,89 @@ def test_is_enabled(self):
         assert not api.is_enabled_for_course(course_id)
         api.set_dates_for_course(course_id, items)
         assert api.is_enabled_for_course(course_id)
+
+    def test_allow_relative_dates(self):
+        course_key = CourseLocator('testX', 'tt101', '2019')
+        block1 = make_block_id(course_key)
+        date1 = datetime(2019, 3, 22)
+        block2 = make_block_id(course_key)
+        date2 = datetime(2019, 3, 23)
+        date2_override_delta = timedelta(days=10)
+        date2_override = date2 + date2_override_delta
+        block3 = make_block_id(course_key)
+        date3_delta = timedelta(days=1)
+        date3 = self.schedule.start_date + date3_delta
+        block4 = make_block_id(course_key)
+        date4_delta = timedelta(days=2)
+        date4 = self.schedule.start_date + date4_delta
+        date4_override = datetime(2019, 4, 24)
+        items = [
+            (block1, {'due': date1}),  # absolute
+            (block2, {'due': date2}),  # absolute, to be overwritten by relative date
+            (block3, {'due': date3_delta}),  # relative
+            (block4, {'due': date4_delta}),  # relative, to be overwritten by absolute date
+        ]
+        api.set_dates_for_course(course_key, items)
+        api.set_date_for_block(course_key, block2, 'due', date2_override_delta, user=self.user)
+        api.set_date_for_block(course_key, block4, 'due', date4_override, user=self.user)
+
+        # get_dates_for_course
+        dates = [
+            ((block1, 'due'), date1),
+            ((block2, 'due'), date2),
+            ((block3, 'due'), date3),
+            ((block4, 'due'), date4),
+        ]
+        assert api.get_dates_for_course(course_key, schedule=self.schedule) == dict(dates)
+        with patch('edx_when.api._are_relative_dates_enabled', return_value=False):
+            assert api.get_dates_for_course(course_key, schedule=self.schedule) == dict(dates[0:2])
+            assert api.get_dates_for_course(course_key, schedule=self.schedule, user=self.user) == dict(dates[0:2])
+
+        # get_date_for_block
+        assert api.get_date_for_block(course_key, block2) == date2
+        assert api.get_date_for_block(course_key, block4, user=self.user) == date4_override
+        with patch('edx_when.api._are_relative_dates_enabled', return_value=False):
+            assert api.get_date_for_block(course_key, block2) == date2
+            assert api.get_date_for_block(course_key, block1, user=self.user) == date1
+            assert api.get_date_for_block(course_key, block2, user=self.user) == date2
+            assert api.get_date_for_block(course_key, block4, user=self.user) is None
+
+        # get_overrides_for_block
+        block2_overrides = [(self.user.username, 'unknown', date2_override)]
+        assert api.get_overrides_for_block(course_key, block2) == block2_overrides
+        with patch('edx_when.api._are_relative_dates_enabled', return_value=False):
+            assert api.get_overrides_for_block(course_key, block2) == []
+
+        # get_overrides_for_user
+        user_overrides = [
+            {'location': block4, 'actual_date': date4_override},
+            {'location': block2, 'actual_date': date2_override},
+        ]
+        assert list(api.get_overrides_for_user(course_key, self.user)) == user_overrides
+        with patch('edx_when.api._are_relative_dates_enabled', return_value=False):
+            assert list(api.get_overrides_for_user(course_key, self.user)) == [user_overrides[0]]
+
+
+class ApiWaffleTests(TestCase):
+    """
+    Tests for edx_when.api waffle usage.
+
+    These are isolated because they have pretty different patch requirements.
+    """
+    @patch.dict(sys.modules, {'openedx.features.course_experience': Mock()})
+    def test_relative_dates_enabled(self):
+        from openedx.features.course_experience import RELATIVE_DATES_FLAG as mock_flag  # pylint: disable=import-error
+        mock_flag.is_enabled.return_value = True
+        assert api._are_relative_dates_enabled()  # pylint: disable=protected-access
+        assert mock_flag.is_enabled.called
+
+    @patch.dict(sys.modules, {'openedx.features.course_experience': Mock()})
+    def test_relative_dates_disabled(self):
+        from openedx.features.course_experience import RELATIVE_DATES_FLAG as mock_flag  # pylint: disable=import-error
+        mock_flag.is_enabled.return_value = False
+        assert not api._are_relative_dates_enabled()  # pylint: disable=protected-access
+        assert mock_flag.is_enabled.called
+
+    @patch.dict(sys.modules, {'openedx.features.course_experience': None})
+    def test_relative_dates_import_error(self):
+        assert not api._are_relative_dates_enabled()  # pylint: disable=protected-access

tests/test_xblock_services.py

@@ -61,7 +61,6 @@ class TestFieldData(XblockTests):
     Tests for the FieldData subclass.
     """
 
-    @api.override_enabled()
     def test_field_data_get(self):
         defaults = mock.MagicMock()
         dfd = field_data.DateLookupFieldData(defaults, course_id=self.course_id, use_cached=False, user=self.user)
@@ -128,7 +127,8 @@ def test_collect(self):
         field_data.DateOverrideTransformer.collect(block_structure)
         assert block_structure.request_xblock_fields.called_once_with('due', 'start')
 
-    def test_transform(self):
+    @mock.patch('edx_when.api._are_relative_dates_enabled', return_value=True)
+    def test_transform(self, _mock):
         override = datetime.datetime(2020, 1, 1)
         api.set_date_for_block(self.items[0][0].course_key, self.items[0][0], 'due', override, user=self.user)
         usage_info = mock.MagicMock()