Skip to content

Commit

Permalink
fix: Return unmigrated rows for subsection_and_higher_only
Browse files Browse the repository at this point in the history
The call to get_dates_for_course would previously not return unmigrated
ContentDates when subsection_and_higher_only=True, because it was
scanning for equality to block_type=None as a fallback, when in SQL
None != None, and has to be queried for explicitly with isnull in the
Django ORM. TNL-8061
  • Loading branch information
David Ormsbee committed Sep 21, 2021
1 parent d09e20e commit 7e64b99
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 3 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ Change Log
Unreleased
~~~~~~~~~~

[2.2.1] - 2021-09-15
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
* Bug fix for optimization in 2.2.0, to account for missing block_type data.

[2.2.0] - 2021-08-27
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
* Added optimization when requesting course block dates for an outline, where block dates below subsections are unneeded.
Expand Down
2 changes: 1 addition & 1 deletion edx_when/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
Central source of course block dates for the LMS.
"""

__version__ = '2.2.0'
__version__ = '2.2.1'

default_app_config = 'edx_when.apps.EdxWhenConfig' # pylint: disable=invalid-name
8 changes: 6 additions & 2 deletions edx_when/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ def _processed_results_cache_key(
if allow_relative_dates:
cache_key += '.with-rel-dates'
if subsection_and_higher_only:
cache_key += '.subsection_and_higher_only'
# cache key incremented with ".v2" so we don't mix buggy cached data with fixed data
cache_key += '.subsection_and_higher_only.v2'
cache_key += '.%s' % published_version if published_version else ''
return cache_key

Expand Down Expand Up @@ -222,7 +223,10 @@ def get_dates_for_course(
qset = models.ContentDate.objects.filter(course_id=course_id, active=True, **rel_lookup)
if subsection_and_higher_only:
# Include NULL block_type values as well because of lazy rollout.
qset = qset.filter(Q(block_type__in=('course', 'chapter', 'sequential', None)))
qset = qset.filter(
Q(block_type__in=('course', 'chapter', 'sequential')) |
Q(block_type__isnull=True)
)

qset = list(
qset.select_related('policy')
Expand Down
Empty file removed tests/__init__.py
Empty file.
14 changes: 14 additions & 0 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,20 @@ def test_get_dates_for_course_outline(self):
)
assert len(retrieved) == NUM_OVERRIDES

# Set all the ContentDates for this course's structural blocks to have
# None for their block_type to test compatibilty with a half-migrated
# state. They should still be returned by get_dates_for_course with
# subsection_and_higher_only=True.
structural_dates = models.ContentDate.objects.filter(
course_id=course_key,
block_type__in=['course', 'chapter', 'sequential']
)
structural_dates.update(block_type=None)
retrieved = api.get_dates_for_course(
course_key, subsection_and_higher_only=True, published_version=self.course_version, use_cached=False
)
assert len(retrieved) == NUM_OVERRIDES

def test_get_dates_for_course(self):
items = make_items()
api.set_dates_for_course(items[0][0].course_key, items)
Expand Down

0 comments on commit 7e64b99

Please sign in to comment.