Merge pull request #28837 from edx/ormsbee/edx_when_outline_optimization

Re-enable edx-when outline optimization
This commit is contained in:
David Ormsbee
2021-09-29 09:17:39 -04:00
committed by GitHub
15 changed files with 55 additions and 29 deletions

View File

@@ -22,6 +22,8 @@ from lms.djangoapps.courseware.courses import (
get_permission_for_course_about get_permission_for_course_about
) )
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.content.learning_sequences.api import get_course_outline
from openedx.core.djangoapps.content.learning_sequences.data import CourseOutlineData
from openedx.core.lib.api.view_utils import LazySequence from openedx.core.lib.api.view_utils import LazySequence
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.exceptions import ItemNotFoundError
@@ -235,9 +237,18 @@ def get_due_dates(request, course_key, user):
url: the deep link to the block url: the deep link to the block
date: the due date for the block date: the due date for the block
""" """
try:
outline = get_course_outline(course_key)
except (ValueError, CourseOutlineData.DoesNotExist):
# Either this course is Old Mongo-backed or doesn't have a generated course outline.
course_version = None
else:
course_version = outline.published_version
dates = get_dates_for_course( dates = get_dates_for_course(
course_key, course_key,
user, user,
published_version=course_version
) )
store = modulestore() store = modulestore()

View File

@@ -1559,8 +1559,8 @@ class ProgressPageTests(ProgressPageBaseTests):
@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
@ddt.data( @ddt.data(
(False, 63, 46), (False, 63, 44),
(True, 55, 40) (True, 55, 38)
) )
@ddt.unpack @ddt.unpack
def test_progress_queries(self, enable_waffle, initial, subsequent): def test_progress_queries(self, enable_waffle, initial, subsequent):

View File

@@ -2739,7 +2739,9 @@ def reset_due_date(request, course_id):
unit = find_unit(course, request.POST.get('url')) unit = find_unit(course, request.POST.get('url'))
reason = strip_tags(request.POST.get('reason', '')) reason = strip_tags(request.POST.get('reason', ''))
original_due_date = get_date_for_block(course_id, unit.location) version = getattr(course, 'course_version', None)
original_due_date = get_date_for_block(course_id, unit.location, published_version=version)
set_due_date_extension(course, unit, student, None, request.user, reason=reason) set_due_date_extension(course, unit, student, None, request.user, reason=reason)
if not original_due_date: if not original_due_date:

View File

@@ -125,11 +125,13 @@ def get_units_with_due_date(course):
""" """
units = [] units = []
version = getattr(course, 'course_version', None)
# Pass in a schedule here so that we get back any relative dates in the course, but actual value # Pass in a schedule here so that we get back any relative dates in the course, but actual value
# doesn't matter, since we don't care about the dates themselves, just whether they exist. # doesn't matter, since we don't care about the dates themselves, just whether they exist.
# Thus we don't save or care about this temporary schedule object. # Thus we don't save or care about this temporary schedule object.
schedule = Schedule(start_date=course.start) schedule = Schedule(start_date=course.start)
course_dates = api.get_dates_for_course(course.id, schedule=schedule) course_dates = api.get_dates_for_course(course.id, schedule=schedule, published_version=version)
def visit(node): def visit(node):
""" """
@@ -173,7 +175,8 @@ def set_due_date_extension(course, unit, student, due_date, actor=None, reason='
# We normally set dates at the subsection level. But technically dates can be anywhere down the tree (and # We normally set dates at the subsection level. But technically dates can be anywhere down the tree (and
# usually are in self paced courses, where the subsection date gets propagated down). # usually are in self paced courses, where the subsection date gets propagated down).
# So find all children that we need to set the date on, then set those dates. # So find all children that we need to set the date on, then set those dates.
course_dates = api.get_dates_for_course(course.id, user=student) version = getattr(course, 'course_version', None)
course_dates = api.get_dates_for_course(course.id, user=student, published_version=version)
blocks_to_set = {unit} # always include the requested unit, even if it doesn't appear to have a due date now blocks_to_set = {unit} # always include the requested unit, even if it doesn't appear to have a due date now
def visit(node): def visit(node):
@@ -191,8 +194,9 @@ def set_due_date_extension(course, unit, student, due_date, actor=None, reason='
for block in blocks_to_set: for block in blocks_to_set:
if due_date: if due_date:
try: try:
api.set_date_for_block(course.id, block.location, 'due', due_date, user=student, reason=reason, api.set_date_for_block(
actor=actor) course.id, block.location, 'due', due_date, user=student, reason=reason, actor=actor
)
except api.MissingDateError as ex: except api.MissingDateError as ex:
raise DashboardError(_("Unit {0} has no due date to extend.").format(unit.location)) from ex raise DashboardError(_("Unit {0} has no due date to extend.").format(unit.location)) from ex
except api.InvalidDateError as ex: except api.InvalidDateError as ex:

View File

@@ -356,7 +356,7 @@ def _get_user_course_outline_and_processors(course_key: CourseKey, # lint-amnes
# particular ordering). # particular ordering).
processor = processor_cls(course_key, user, at_time) processor = processor_cls(course_key, user, at_time)
processors[name] = processor processors[name] = processor
processor.load_data() processor.load_data(full_course_outline)
if not user_can_see_all_content: if not user_can_see_all_content:
# function_trace lets us see how expensive each processor is being. # function_trace lets us see how expensive each processor is being.
with function_trace(f'learning_sequences.api.outline_processors.{name}'): with function_trace(f'learning_sequences.api.outline_processors.{name}'):

View File

@@ -8,6 +8,8 @@ from datetime import datetime
from opaque_keys.edx.keys import CourseKey # lint-amnesty, pylint: disable=unused-import from opaque_keys.edx.keys import CourseKey # lint-amnesty, pylint: disable=unused-import
from openedx.core import types from openedx.core import types
from ...data import CourseOutlineData
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
@@ -44,7 +46,7 @@ class OutlineProcessor:
self.user = user self.user = user
self.at_time = at_time self.at_time = at_time
def load_data(self): def load_data(self, full_course_outline: CourseOutlineData): # pylint: disable=unused-argument
""" """
Fetch whatever data you need about the course and user here. Fetch whatever data you need about the course and user here.
@@ -59,7 +61,7 @@ class OutlineProcessor:
""" """
pass # lint-amnesty, pylint: disable=unnecessary-pass pass # lint-amnesty, pylint: disable=unnecessary-pass
def inaccessible_sequences(self, full_course_outline): # lint-amnesty, pylint: disable=unused-argument def inaccessible_sequences(self, full_course_outline: CourseOutlineData): # pylint: disable=unused-argument
""" """
Return a set/frozenset of Sequence UsageKeys that are not accessible. Return a set/frozenset of Sequence UsageKeys that are not accessible.
@@ -68,7 +70,7 @@ class OutlineProcessor:
""" """
return frozenset() return frozenset()
def usage_keys_to_remove(self, full_course_outline): # lint-amnesty, pylint: disable=unused-argument def usage_keys_to_remove(self, full_course_outline: CourseOutlineData): # pylint: disable=unused-argument
""" """
Return a set/frozenset of UsageKeys to remove altogether. Return a set/frozenset of UsageKeys to remove altogether.

View File

@@ -27,7 +27,7 @@ class ContentGatingOutlineProcessor(OutlineProcessor):
self.required_content = None self.required_content = None
self.can_skip_entrance_exam = False self.can_skip_entrance_exam = False
def load_data(self): def load_data(self, full_course_outline):
""" """
Get the required content for the course, and whether Get the required content for the course, and whether
or not the user can skip the entrance exam. or not the user can skip the entrance exam.

View File

@@ -1,5 +1,9 @@
# lint-amnesty, pylint: disable=missing-module-docstring # lint-amnesty, pylint: disable=missing-module-docstring
import logging import logging
from datetime import datetime
from opaque_keys.edx.keys import CourseKey
from openedx.core import types
from xmodule.partitions.enrollment_track_partition_generator import ( from xmodule.partitions.enrollment_track_partition_generator import (
create_enrollment_track_partition_with_course_id create_enrollment_track_partition_with_course_id
@@ -22,12 +26,12 @@ class EnrollmentTrackPartitionGroupsOutlineProcessor(OutlineProcessor):
significant limitation. Nonetheless, it is a step towards the goal of significant limitation. Nonetheless, it is a step towards the goal of
supporting all partition schemes in the future. supporting all partition schemes in the future.
""" """
def __init__(self, course_key, user, at_time): def __init__(self, course_key: CourseKey, user: types.User, at_time: datetime):
super().__init__(course_key, user, at_time) super().__init__(course_key, user, at_time)
self.enrollment_track_groups = {} self.enrollment_track_groups = {}
self.user_group = None self.user_group = None
def load_data(self): def load_data(self, full_course_outline):
""" """
Pull track groups for this course and which group the user is in. Pull track groups for this course and which group the user is in.
""" """

View File

@@ -42,11 +42,16 @@ class ScheduleOutlineProcessor(OutlineProcessor):
self._course_end = None self._course_end = None
self._is_beta_tester = False self._is_beta_tester = False
def load_data(self): def load_data(self, full_course_outline):
"""Pull dates information from edx-when.""" """
# (usage_key, 'due'): datetime.datetime(2019, 12, 11, 15, 0, tzinfo=<UTC>) Pull dates information from edx-when.
# TODO: Merge https://github.com/edx/edx-when/pull/48 and add `outline_only=True`
self.dates = get_dates_for_course(self.course_key, self.user) Return data format: (usage_key, 'due'): datetime.datetime(2019, 12, 11, 15, 0, tzinfo=<UTC>)
"""
self.dates = get_dates_for_course(
self.course_key, self.user, subsection_and_higher_only=True,
published_version=full_course_outline.published_version
)
for (usage_key, field_name), date in self.dates.items(): for (usage_key, field_name), date in self.dates.items():
self.keys_to_schedule_fields[usage_key][field_name] = date self.keys_to_schedule_fields[usage_key][field_name] = date

View File

@@ -28,7 +28,7 @@ class SpecialExamsOutlineProcessor(OutlineProcessor):
""" """
Responsible for applying all outline processing related to special exams. Responsible for applying all outline processing related to special exams.
""" """
def load_data(self): def load_data(self, full_course_outline):
""" """
Check if special exams are enabled Check if special exams are enabled
""" """

View File

@@ -1737,8 +1737,10 @@ class EnrollmentTrackPartitionGroupsTestCase(OutlineProcessorTestCase): # lint-
check_date = datetime(2021, 3, 27, tzinfo=timezone.utc) check_date = datetime(2021, 3, 27, tzinfo=timezone.utc)
for learner_to_verify in learners_to_verify: for learner_to_verify in learners_to_verify:
processor = EnrollmentTrackPartitionGroupsOutlineProcessor(self.course_key, learner_to_verify, check_date) processor = EnrollmentTrackPartitionGroupsOutlineProcessor(
processor.load_data() self.course_key, learner_to_verify, check_date
)
processor.load_data(full_outline)
removed_usage_keys = processor.usage_keys_to_remove(full_outline) removed_usage_keys = processor.usage_keys_to_remove(full_outline)
assert len(removed_usage_keys) == expected_values_dict[learner_to_verify.username] assert len(removed_usage_keys) == expected_values_dict[learner_to_verify.username]

View File

@@ -106,7 +106,3 @@ pylint<2.10.0
# Feel free to loosen this constraint if/when it is confirmed that a later # Feel free to loosen this constraint if/when it is confirmed that a later
# version of py2neo will work with Neo4j 3.5. # version of py2neo will work with Neo4j 3.5.
py2neo<2022 py2neo<2022
# as of right now (Sep 8 2021), we need to not use the latest edx-when
# version which might be causing issues with course publishing
edx-when==2.1.0

View File

@@ -508,7 +508,7 @@ edx-toggles==4.2.0
# ora2 # ora2
edx-user-state-client==1.3.2 edx-user-state-client==1.3.2
# via -r requirements/edx/base.in # via -r requirements/edx/base.in
edx-when==2.1.0 edx-when==2.2.1
# via # via
# -c requirements/edx/../constraints.txt # -c requirements/edx/../constraints.txt
# -r requirements/edx/base.in # -r requirements/edx/base.in

View File

@@ -618,7 +618,7 @@ edx-toggles==4.2.0
# ora2 # ora2
edx-user-state-client==1.3.2 edx-user-state-client==1.3.2
# via -r requirements/edx/testing.txt # via -r requirements/edx/testing.txt
edx-when==2.1.0 edx-when==2.2.1
# via # via
# -c requirements/edx/../constraints.txt # -c requirements/edx/../constraints.txt
# -r requirements/edx/testing.txt # -r requirements/edx/testing.txt

View File

@@ -597,7 +597,7 @@ edx-toggles==4.2.0
# ora2 # ora2
edx-user-state-client==1.3.2 edx-user-state-client==1.3.2
# via -r requirements/edx/base.txt # via -r requirements/edx/base.txt
edx-when==2.1.0 edx-when==2.2.1
# via # via
# -c requirements/edx/../constraints.txt # -c requirements/edx/../constraints.txt
# -r requirements/edx/base.txt # -r requirements/edx/base.txt