Merge pull request #23978 from edx/mikix/highlights-dont-assume-due-dates

Don't assume due dates on sections

openedx/core/djangoapps/course_date_signals/handlers.py

@@ -9,7 +9,7 @@ from xblock.fields import Scope
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import SignalHandler, modulestore
 from .models import SelfPacedRelativeDatesConfig
-from .utils import get_expected_duration
+from .utils import spaced_out_sections
 from edx_when.api import FIELDS_TO_EXTRACT, set_dates_for_course
 
 log = logging.getLogger(__name__)
@@ -49,19 +49,16 @@ def extract_dates_from_course(course):
         date_items = [(course.location, metadata)]
 
         if SelfPacedRelativeDatesConfig.current(course_key=course.id).enabled:
-            duration = get_expected_duration(course)
-            sections = course.get_children()
-            time_per_week = duration / len(sections)
             # Apply the same relative due date to all content inside a section,
             # unless that item already has a relative date set
-            for idx, section in enumerate(sections):
+            for _, section, weeks_to_complete in spaced_out_sections(course):
                 items = [section]
                 while items:
                     next_item = items.pop()
                     if next_item.graded:
                         # TODO: Once studio can manually set relative dates,
                         # we would need to manually check for them here
-                        date_items.append((next_item.location, {'due': time_per_week * (idx + 1)}))
+                        date_items.append((next_item.location, {'due': weeks_to_complete}))
                     items.extend(next_item.get_children())
     else:
         date_items = []

openedx/core/djangoapps/course_date_signals/utils.py

@@ -6,7 +6,6 @@ get_expected_duration: return the expected duration of a course (absent any user
 
 from datetime import timedelta
 
-from course_modes.models import CourseMode
 from openedx.core.djangoapps.catalog.utils import get_course_run_details
 
 
@@ -21,11 +20,6 @@ def get_expected_duration(course):
 
     access_duration = MIN_DURATION
 
-    verified_mode = CourseMode.verified_mode_for_course(course=course, include_expired=True)
-
-    if not verified_mode:
-        return None
-
     # The user course expiration date is the content availability date
     # plus the weeks_to_complete field from course-discovery.
     discovery_course_details = get_course_run_details(course.id, ['weeks_to_complete'])
@@ -37,3 +31,19 @@ def get_expected_duration(course):
     access_duration = max(MIN_DURATION, min(MAX_DURATION, access_duration))
 
     return access_duration
+
+
+def spaced_out_sections(course):
+    """
+    Generator that returns sections of the course module with a suggested time to complete for each
+
+    Returns:
+        index (int): index of section
+        section (block): a section block of the course
+        relative time (timedelta): the amount of weeks to complete the section, since start of course
+    """
+    duration = get_expected_duration(course)
+    sections = course.get_children()
+    weeks_per_section = duration / len(sections)
+    for idx, section in enumerate(sections):
+        yield idx, section, weeks_per_section * (idx + 1)

openedx/core/djangoapps/schedules/content_highlights.py

@@ -6,8 +6,7 @@ schedule experience built on the Schedules app.
 
 import logging
 
-from datetime import datetime, timedelta
-
+from openedx.core.djangoapps.course_date_signals.utils import spaced_out_sections
 from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG
 from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist
 from openedx.core.lib.request_utils import get_request_or_stub
@@ -64,7 +63,7 @@ def get_week_highlights(user, course_key, week_num):
     return highlights
 
 
-def get_next_section_highlights(user, course_key, target_date):
+def get_next_section_highlights(user, course_key, start_date, target_date):
     """
     Get highlights (list of unicode strings) for a week, based upon the current date.
 
@@ -75,8 +74,9 @@ def get_next_section_highlights(user, course_key, target_date):
     course_module = _get_course_module(course_descriptor, user)
     sections_with_highlights = _get_sections_with_highlights(course_module)
     highlights = _get_highlights_for_next_section(
+        course_module,
         sections_with_highlights,
-        course_key,
+        start_date,
         target_date
     )
     return highlights
@@ -129,11 +129,12 @@ def _get_course_module(course_descriptor, user):
     )
 
 
+def _section_has_highlights(section):
+    return section.highlights and not section.hide_from_toc
+
+
 def _get_sections_with_highlights(course_module):
-    return [
-        section for section in course_module.get_children()
-        if section.highlights and not section.hide_from_toc
-    ]
+    return list(filter(_section_has_highlights, course_module.get_children()))
 
 
 def _get_highlights_for_week(sections, week_num, course_key):
@@ -150,15 +151,23 @@ def _get_highlights_for_week(sections, week_num, course_key):
     return section.highlights
 
 
-def _get_highlights_for_next_section(sections, course_key, target_date):
-    sorted_sections = sorted(sections, key=lambda section: section.due)
-    for index, sorted_section in enumerate(sorted_sections):
-        if sorted_section.due.date() == target_date and index + 1 < len(sorted_sections):
+def _get_highlights_for_next_section(course_module, sections, start_date, target_date):
+    for index, section, weeks_to_complete in spaced_out_sections(course_module):
+        if not _section_has_highlights(section):
+            continue
+
+        # We calculate section due date ourselves (rather than grabbing the due attribute),
+        # since not every section has a real due date (i.e. not all are graded), but we still
+        # want to know when this section should have been completed by the learner.
+        section_due_date = start_date + weeks_to_complete
+
+        if section_due_date.date() == target_date and index + 1 < len(sections):
             # Return index + 2 for "week_num", since weeks start at 1 as opposed to indexes,
             # and we want the next week, so +1 for index and +1 for next
             return sections[index + 1].highlights, index + 2
+
     raise CourseUpdateDoesNotExist(
         u"No section found ending on {} for {}".format(
-            target_date, course_key
+            target_date, course_module.id
         )
     )

openedx/core/djangoapps/schedules/resolvers.py

@@ -469,6 +469,7 @@ class CourseNextSectionUpdate(PrefixedDebugLoggerMixin, RecipientResolver):
             enrollment = schedule.enrollment
             course = schedule.enrollment.course
             user = enrollment.user
+            start_date = schedule.start_date
             LOG.info(u'Received a schedule for user {} in course {} for date {}'.format(
                 user.username,
                 self.course_id,
@@ -476,7 +477,7 @@ class CourseNextSectionUpdate(PrefixedDebugLoggerMixin, RecipientResolver):
             ))
 
             try:
-                week_highlights, week_num = get_next_section_highlights(user, course.id, target_date)
+                week_highlights, week_num = get_next_section_highlights(user, course.id, start_date, target_date)
             except CourseUpdateDoesNotExist:
                 LOG.warning(
                     u'Weekly highlights for user {} of course {} does not exist or is disabled'.format(

openedx/core/djangoapps/schedules/tests/test_content_highlights.py

@@ -2,6 +2,8 @@
 
 import datetime
 
+import mock
+
 from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG
 from openedx.core.djangoapps.schedules.content_highlights import (
     course_has_highlights, get_week_highlights, get_next_section_highlights,
@@ -135,23 +137,23 @@ class TestContentHighlights(ModuleStoreTestCase):
             get_week_highlights(self.user, self.course_key, week_num=1)
 
     @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True)
-    def test_get_next_section_highlights(self):
+    @mock.patch('openedx.core.djangoapps.course_date_signals.utils.get_expected_duration')
+    def test_get_next_section_highlights(self, mock_duration):
+        mock_duration.return_value = datetime.timedelta(days=2)
         yesterday = datetime.datetime.utcnow() - datetime.timedelta(days=1)
         today = datetime.datetime.utcnow()
         tomorrow = datetime.datetime.utcnow() + datetime.timedelta(days=1)
         with self.store.bulk_operations(self.course_key):
             self._create_chapter(  # Week 1
                 highlights=[u'a', u'b', u'á'],
-                due=yesterday,
             )
             self._create_chapter(  # Week 2
                 highlights=[u'skipped a week'],
-                due=today,
             )
 
         self.assertEqual(
-            get_next_section_highlights(self.user, self.course_key, yesterday.date()),
+            get_next_section_highlights(self.user, self.course_key, yesterday, today.date()),
             ([u'skipped a week'], 2),
         )
         with self.assertRaises(CourseUpdateDoesNotExist):
-            get_next_section_highlights(self.user, self.course_key, tomorrow.date())
+            get_next_section_highlights(self.user, self.course_key, yesterday, tomorrow.date())

openedx/features/course_duration_limits/access.py

@@ -67,6 +67,10 @@ def get_user_course_duration(user, course):
     if enrollment is None or enrollment.mode != CourseMode.AUDIT:
         return None
 
+    verified_mode = CourseMode.verified_mode_for_course(course=course, include_expired=True)
+    if not verified_mode:
+        return None
+
     return get_expected_duration(course)