From 3f2b2da22bb523d895f4489aa721d853bde6be2d Mon Sep 17 00:00:00 2001
From: Michael Terry <mterry@edx.org>
Date: Tue, 12 May 2020 15:13:32 -0400
Subject: [PATCH] Don't assume due dates on sections
The content highlights code assumed due dates existed on all
sections. But we recently broke that assumption. So now we
recalculate the spread of sections across the expected duration
ourselves rather than rely on due dates.
---
.../course_date_signals/handlers.py | 9 ++---
.../djangoapps/course_date_signals/utils.py | 22 ++++++++----
.../schedules/content_highlights.py | 35 ++++++++++++-------
.../core/djangoapps/schedules/resolvers.py | 3 +-
.../tests/test_content_highlights.py | 12 ++++---
.../features/course_duration_limits/access.py | 4 +++
6 files changed, 54 insertions(+), 31 deletions(-)
diff --git a/openedx/core/djangoapps/course_date_signals/handlers.py b/openedx/core/djangoapps/course_date_signals/handlers.py
index 61514a571c2..f846e6e7757 100644
--- a/openedx/core/djangoapps/course_date_signals/handlers.py
+++ b/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 = []
diff --git a/openedx/core/djangoapps/course_date_signals/utils.py b/openedx/core/djangoapps/course_date_signals/utils.py
index a006268c9e6..4ff133f21ed 100644
--- a/openedx/core/djangoapps/course_date_signals/utils.py
+++ b/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)
diff --git a/openedx/core/djangoapps/schedules/content_highlights.py b/openedx/core/djangoapps/schedules/content_highlights.py
index dd005382ddf..e0ea6f5b164 100644
--- a/openedx/core/djangoapps/schedules/content_highlights.py
+++ b/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
)
)
diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py
index adae88b0f85..fa00e83c547 100644
--- a/openedx/core/djangoapps/schedules/resolvers.py
+++ b/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(
diff --git a/openedx/core/djangoapps/schedules/tests/test_content_highlights.py b/openedx/core/djangoapps/schedules/tests/test_content_highlights.py
index 5519125408e..004b5b1533a 100644
--- a/openedx/core/djangoapps/schedules/tests/test_content_highlights.py
+++ b/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())
diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py
index ea43617e4da..fd018d7d11e 100644
--- a/openedx/features/course_duration_limits/access.py
+++ b/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)
--
GitLab