From 4d4b93c6ed92e3be3d06ae68b1994751a3e9c6e6 Mon Sep 17 00:00:00 2001 From: Dillon Dumesnil Date: Wed, 13 May 2020 10:22:50 -0700 Subject: [PATCH] AA-159: Improving performance of get_course_assignments We saw an increase in response time with recent changes to the logic behind get_course_assignments. This effort works to better access the information we need in order to improve performance. Namely, this is done by using the course_blocks_api --- .../blocks/transformers/block_completion.py | 47 ++++++++++++++++++- lms/djangoapps/course_blocks/api.py | 4 ++ lms/djangoapps/courseware/courses.py | 37 ++++++++------- 3 files changed, 70 insertions(+), 18 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/transformers/block_completion.py b/lms/djangoapps/course_api/blocks/transformers/block_completion.py index 6d02caff2d..57cf23c070 100644 --- a/lms/djangoapps/course_api/blocks/transformers/block_completion.py +++ b/lms/djangoapps/course_api/blocks/transformers/block_completion.py @@ -16,6 +16,8 @@ class BlockCompletionTransformer(BlockStructureTransformer): READ_VERSION = 1 WRITE_VERSION = 1 COMPLETION = 'completion' + COMPLETE = 'complete' + RESUME_BLOCK = 'resume_block' @classmethod def name(cls): @@ -43,9 +45,46 @@ class BlockCompletionTransformer(BlockStructureTransformer): def collect(cls, block_structure): block_structure.request_xblock_fields('completion_mode') + def mark_complete(self, course_block_completions, latest_completion_block_key, block_key, block_structure): + """ + Helper function to mark a block as 'complete' as dictated by + course_block_completions (for problems) or all of a block's children being complete. + This also sets the 'resume_block' field as that is connected to the latest completed block. + + :param course_block_completions: dict[course_completion_object] = completion_value + :param latest_completion_block_key: block key for the latest completed block. + :param block_key: A opaque_keys.edx.locator.BlockUsageLocator object + :param block_structure: A BlockStructureBlockData object + """ + if block_key in course_block_completions: + block_structure.override_xblock_field(block_key, self.COMPLETE, True) + if block_key == latest_completion_block_key: + block_structure.override_xblock_field(block_key, self.RESUME_BLOCK, True) + + children = block_structure.get_children(block_key) + non_discussion_children = (child_key for child_key in children + if block_structure.get_xblock_field(child_key, 'category') != 'discussion') + child_complete = (block_structure.get_xblock_field(child_key, self.COMPLETE) + for child_key in non_discussion_children) + if children and all(child_complete): + block_structure.override_xblock_field(block_key, self.COMPLETE, True) + + if any(block_structure.get_xblock_field(child_key, self.RESUME_BLOCK) for child_key in children): + block_structure.override_xblock_field(block_key, self.RESUME_BLOCK, True) + def transform(self, usage_info, block_structure): """ - Mutates block_structure adding extra field which contains block's completion. + Mutates block_structure adding three extra fields which contains block's completion, + complete status, and if the block is a resume_block, indicating it is the most recently + completed block. + + IMPORTANT!: There is a subtle, but important difference between 'completion' and 'complete' + which are both set in this transformer: + 'completion': Returns a percentile (0.0 - 1.0) of correctness for a _problem_. This field will + be None for all other blocks that are not leaves and captured in BlockCompletion. + 'complete': Returns a boolean indicating whether the block is complete. For problems, this will + be taken from a BlockCompletion entry existing. For all other blocks, it will be marked True + if all of the children of the block are all marked complete (this is calculated recursively) """ def _is_block_an_aggregator_or_excluded(block_key): """ @@ -82,3 +121,9 @@ class BlockCompletionTransformer(BlockStructureTransformer): block_structure.set_transformer_block_field( block_key, self, self.COMPLETION, completion_value ) + + latest_completion = completions.latest() if completions.exists() else None + if latest_completion: + latest_completion_block_key = latest_completion[0] + for block_key in block_structure.post_order_traversal(): + self.mark_complete(completions_dict, latest_completion_block_key, block_key, block_structure) diff --git a/lms/djangoapps/course_blocks/api.py b/lms/djangoapps/course_blocks/api.py index 05a9614a4b..569eeb947e 100644 --- a/lms/djangoapps/course_blocks/api.py +++ b/lms/djangoapps/course_blocks/api.py @@ -7,6 +7,7 @@ get_course_blocks function. from django.conf import settings from edx_when import field_data +from lms.djangoapps.course_api.blocks.transformers.block_completion import BlockCompletionTransformer from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers from openedx.features.content_type_gating.block_transformers import ContentTypeGateTransformer @@ -58,6 +59,7 @@ def get_course_blocks( transformers=None, collected_block_structure=None, allow_start_dates_in_future=False, + include_completion=False, ): """ A higher order function implemented on top of the @@ -91,6 +93,8 @@ def get_course_blocks( """ if not transformers: transformers = BlockStructureTransformers(get_course_block_access_transformers(user)) + if include_completion: + transformers += [BlockCompletionTransformer()] transformers.usage_info = CourseUsageInfo(starting_block_usage_key.course_key, user, allow_start_dates_in_future) return get_block_structure_manager(starting_block_usage_key.course_key).get_transformed( diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 0403f81ef3..40e8261c2d 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -52,13 +52,13 @@ from lms.djangoapps.courseware.access_utils import ( ) from lms.djangoapps.courseware.courseware_access_exception import CoursewareAccessException from lms.djangoapps.courseware.exceptions import CourseAccessRedirect +from lms.djangoapps.course_blocks.api import get_course_blocks from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.enrollments.api import get_course_enrollment_details from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.lib.api.view_utils import LazySequence from openedx.features.course_duration_limits.access import AuditExpiredError from openedx.features.course_experience import RELATIVE_DATES_FLAG -from openedx.features.course_experience.utils import get_course_outline_block_tree from static_replace import replace_static_urls from student.models import CourseEnrollment from survey.utils import SurveyRequiredAccessError, check_survey_required_and_unanswered @@ -517,31 +517,34 @@ def get_course_assignments(course_key, user, request, include_access=False): Each returned object is a namedtuple with fields: title, url, date, contains_gated_content, complete, past_due """ - assignments = [] - # Ideally this function is always called with a request being passed in, but because it is also - # a subfunction of `get_course_date_blocks` which does not require a request, we are being defensive here. - if not request: - return assignments + store = modulestore() + course_usage_key = store.make_course_usage_key(course_key) + block_data = get_course_blocks(user, course_usage_key, allow_start_dates_in_future=True, include_completion=True) now = datetime.now(pytz.UTC) - course_root_block = get_course_outline_block_tree(request, str(course_key), user, allow_start_dates_in_future=True) - for section in course_root_block.get('children', []): - for subsection in section.get('children', []): - if not subsection.get('due') or not subsection.get('graded'): + assignments = [] + for section_key in block_data.get_children(course_usage_key): + for subsection_key in block_data.get_children(section_key): + due = block_data.get_xblock_field(subsection_key, 'due') + graded = block_data.get_xblock_field(subsection_key, 'graded', False) + if not due or not graded: continue - contains_gated_content = include_access and subsection.get('contains_gated_content', False) - title = subsection.get('display_name', _('Assignment')) + contains_gated_content = include_access and block_data.get_xblock_field( + subsection_key, 'contains_gated_content', False) + title = block_data.get_xblock_field(subsection_key, 'display_name', _('Assignment')) url = None - assignment_released = not subsection.get('start') or subsection.get('start') < now + start = block_data.get_xblock_field(subsection_key, 'start') + assignment_released = not start or start < now if assignment_released: - url = subsection.get('lms_web_url') + url = reverse('jump_to', args=[course_key, subsection_key]) + url = request and request.build_absolute_uri(url) - complete = subsection.get('complete') - past_due = not complete and subsection.get('due', now + timedelta(1)) < now + complete = block_data.get_xblock_field(subsection_key, 'complete', False) + past_due = not complete and due < now assignments.append(_Assignment( - subsection.get('id'), title, url, subsection.get('due'), contains_gated_content, complete, past_due + subsection_key, title, url, due, contains_gated_content, complete, past_due )) return assignments