From 35549f56ba13a17a57b4436f5714eb254e365e78 Mon Sep 17 00:00:00 2001
From: Dillon Dumesnil <ddumesnil@edx.org>
Date: Tue, 30 Jun 2020 12:44:13 -0400
Subject: [PATCH] AA-220: Making Library Content an Aggregator CompletionMode
This change will prevent Library Content from being marked as
complete on view and the corresponding version bump to
edx-completion contains code that will start looking at the
children of the library content for completeness.
---
.../xmodule/xmodule/library_content_module.py | 2 +
.../blocks/transformers/block_completion.py | 7 +-
.../completion_integration/test_services.py | 111 +++++++++++++++++-
requirements/constraints.txt | 3 -
requirements/edx/base.txt | 2 +-
requirements/edx/development.txt | 2 +-
requirements/edx/testing.txt | 2 +-
7 files changed, 118 insertions(+), 11 deletions(-)
diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py
index 077d457fce6..a8bd13a4a87 100644
--- a/common/lib/xmodule/xmodule/library_content_module.py
+++ b/common/lib/xmodule/xmodule/library_content_module.py
@@ -19,6 +19,7 @@ from six import text_type
from six.moves import zip
from web_fragments.fragment import Fragment
from webob import Response
+from xblock.completable import XBlockCompletionMode
from xblock.core import XBlock
from xblock.fields import Integer, List, Scope, String
@@ -65,6 +66,7 @@ class LibraryContentFields(object):
Separated out for now because they need to be added to the module and the
descriptor.
"""
+ completion_mode = XBlockCompletionMode.AGGREGATOR
# Please note the display_name of each field below is used in
# common/test/acceptance/pages/studio/library.py:StudioLibraryContentXBlockEditModal
# to locate input elements - keep synchronized
diff --git a/lms/djangoapps/course_api/blocks/transformers/block_completion.py b/lms/djangoapps/course_api/blocks/transformers/block_completion.py
index 57cf23c0709..dad229953e7 100644
--- a/lms/djangoapps/course_api/blocks/transformers/block_completion.py
+++ b/lms/djangoapps/course_api/blocks/transformers/block_completion.py
@@ -64,9 +64,10 @@ class BlockCompletionTransformer(BlockStructureTransformer):
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):
+ all_children_complete = all(block_structure.get_xblock_field(child_key, self.COMPLETE)
+ for child_key in non_discussion_children)
+
+ if children and all_children_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):
diff --git a/openedx/tests/completion_integration/test_services.py b/openedx/tests/completion_integration/test_services.py
index 60b97fc6e3a..8e15f64f278 100644
--- a/openedx/tests/completion_integration/test_services.py
+++ b/openedx/tests/completion_integration/test_services.py
@@ -12,8 +12,10 @@ from six.moves import range
from openedx.core.djangolib.testing.utils import skip_unless_lms
from student.tests.factories import UserFactory
-from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
-from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.library_tools import LibraryToolsService
+from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory
+from xmodule.tests import get_test_system
@ddt.ddt
@@ -22,6 +24,8 @@ class CompletionServiceTestCase(CompletionWaffleTestMixin, SharedModuleStoreTest
"""
Test the data returned by the CompletionService.
"""
+ MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
@classmethod
def setUpClass(cls):
super(CompletionServiceTestCase, cls).setUpClass()
@@ -30,38 +34,47 @@ class CompletionServiceTestCase(CompletionWaffleTestMixin, SharedModuleStoreTest
cls.chapter = ItemFactory.create(
parent=cls.course,
category="chapter",
+ publish_item=False,
)
cls.sequence = ItemFactory.create(
parent=cls.chapter,
category='sequential',
+ publish_item=False,
)
cls.vertical = ItemFactory.create(
parent=cls.sequence,
category='vertical',
+ publish_item=False,
)
cls.html = ItemFactory.create(
parent=cls.vertical,
category='html',
+ publish_item=False,
)
cls.problem = ItemFactory.create(
parent=cls.vertical,
category="problem",
+ publish_item=False,
)
cls.problem2 = ItemFactory.create(
parent=cls.vertical,
category="problem",
+ publish_item=False,
)
cls.problem3 = ItemFactory.create(
parent=cls.vertical,
category="problem",
+ publish_item=False,
)
cls.problem4 = ItemFactory.create(
parent=cls.vertical,
category="problem",
+ publish_item=False,
)
cls.problem5 = ItemFactory.create(
parent=cls.vertical,
category="problem",
+ publish_item=False,
)
cls.store.update_item(cls.course, UserFactory().id)
cls.problems = [cls.problem, cls.problem2, cls.problem3, cls.problem4, cls.problem5]
@@ -105,6 +118,25 @@ class CompletionServiceTestCase(CompletionWaffleTestMixin, SharedModuleStoreTest
completion=0.75,
)
+ def _bind_course_module(self, module):
+ """
+ Bind a module (part of self.course) so we can access student-specific data.
+ """
+ module_system = get_test_system(course_id=module.location.course_key)
+ module_system.descriptor_runtime = module.runtime._descriptor_system # pylint: disable=protected-access
+ module_system._services['library_tools'] = LibraryToolsService(self.store) # pylint: disable=protected-access
+
+ def get_module(descriptor):
+ """Mocks module_system get_module function"""
+ sub_module_system = get_test_system(course_id=module.location.course_key)
+ sub_module_system.get_module = get_module
+ sub_module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access
+ descriptor.bind_for_student(sub_module_system, self.user.id)
+ return descriptor
+
+ module_system.get_module = get_module
+ module.xmodule_runtime = module_system
+
def test_completion_service(self):
# Only the completions for the user and course specified for the CompletionService
# are returned. Values are returned for all keys provided.
@@ -165,3 +197,78 @@ class CompletionServiceTestCase(CompletionWaffleTestMixin, SharedModuleStoreTest
self.assertEqual(self.completion_service.can_mark_block_complete_on_view(self.vertical), False)
self.assertEqual(self.completion_service.can_mark_block_complete_on_view(self.html), True)
self.assertEqual(self.completion_service.can_mark_block_complete_on_view(self.problem), False)
+
+ def test_vertical_completion_with_library_content(self):
+ library = LibraryFactory.create(modulestore=self.store)
+ ItemFactory.create(parent=library, category='problem', publish_item=False, user_id=self.user.id)
+ ItemFactory.create(parent=library, category='problem', publish_item=False, user_id=self.user.id)
+ ItemFactory.create(parent=library, category='problem', publish_item=False, user_id=self.user.id)
+ lib_vertical = ItemFactory.create(parent=self.sequence, category='vertical', publish_item=False)
+ library_content_block = ItemFactory.create(
+ parent=lib_vertical,
+ category='library_content',
+ max_count=1,
+ source_library_id=str(library.location.library_key),
+ user_id=self.user.id,
+ )
+ library_content_block.refresh_children()
+ lib_vertical = self.store.get_item(lib_vertical.location)
+ self._bind_course_module(lib_vertical)
+ # We need to refetch the library_content_block to retrieve the
+ # fresh version from the call to get_item for lib_vertical
+ library_content_block = [child for child in lib_vertical.get_children()
+ if child.scope_ids.block_type == 'library_content'][0]
+
+ ## Ensure the library_content_block is properly set up
+ # This is needed so we can call get_child_descriptors
+ self._bind_course_module(library_content_block)
+ # Make sure the runtime knows that the block's children vary per-user:
+ self.assertTrue(library_content_block.has_dynamic_children())
+ self.assertEqual(len(library_content_block.children), 3)
+ # Check how many children each user will see:
+ self.assertEqual(len(library_content_block.get_child_descriptors()), 1)
+
+ # No problems are complete yet
+ self.assertFalse(self.completion_service.vertical_is_complete(lib_vertical))
+
+ for block_key in self.block_keys:
+ BlockCompletion.objects.submit_completion(
+ user=self.user,
+ block_key=block_key,
+ completion=1.0
+ )
+ # Library content problems aren't complete yet
+ self.assertFalse(self.completion_service.vertical_is_complete(lib_vertical))
+
+ for child in library_content_block.get_child_descriptors():
+ BlockCompletion.objects.submit_completion(
+ user=self.user,
+ block_key=child.scope_ids.usage_id,
+ completion=1.0
+ )
+ self.assertTrue(self.completion_service.vertical_is_complete(lib_vertical))
+
+ def test_vertical_completion_with_nested_children(self):
+ parent_vertical = ItemFactory(parent=self.sequence, category='vertical')
+ extra_vertical = ItemFactory(parent=parent_vertical, category='vertical')
+ problem = ItemFactory(parent=extra_vertical, category='problem')
+ parent_vertical = self.store.get_item(parent_vertical.location)
+
+ # Nothing is complete
+ self.assertFalse(self.completion_service.vertical_is_complete(parent_vertical))
+
+ for block_key in self.block_keys:
+ BlockCompletion.objects.submit_completion(
+ user=self.user,
+ block_key=block_key,
+ completion=1.0
+ )
+ # The nested child isn't complete yet
+ self.assertFalse(self.completion_service.vertical_is_complete(parent_vertical))
+
+ BlockCompletion.objects.submit_completion(
+ user=self.user,
+ block_key=problem.location,
+ completion=1.0
+ )
+ self.assertTrue(self.completion_service.vertical_is_complete(parent_vertical))
diff --git a/requirements/constraints.txt b/requirements/constraints.txt
index 48a17de4017..bfc99cf2fac 100644
--- a/requirements/constraints.txt
+++ b/requirements/constraints.txt
@@ -40,9 +40,6 @@ drf-yasg<1.17.1
# drf-jwt 1.15.0 contains a migration that breaks on MySQL: https://github.com/Styria-Digital/django-rest-framework-jwt/issues/40
drf-jwt==1.14.0
-# Tests are failing with 3.2.2: https://build.testeng.edx.org/job/edx-platform-python-pipeline-pr/19138/
-edx-completion==3.2.1
-
# Upgrading to 2.12.0 breaks several test classes due to API changes, need to update our code accordingly
factory-boy==2.8.1
diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt
index 805a17b2f8f..ffb0f26ac25 100644
--- a/requirements/edx/base.txt
+++ b/requirements/edx/base.txt
@@ -94,7 +94,7 @@ edx-api-doc-tools==1.3.1 # via -r requirements/edx/base.in
edx-bulk-grades==0.7.0 # via -r requirements/edx/base.in, staff-graded-xblock
edx-ccx-keys==1.1.0 # via -r requirements/edx/base.in
edx-celeryutils==0.5.1 # via -r requirements/edx/base.in, super-csv
-edx-completion==3.2.1 # via -c requirements/edx/../constraints.txt, -r requirements/edx/base.in
+edx-completion==3.2.3 # via -r requirements/edx/base.in
edx-django-release-util==0.4.4 # via -r requirements/edx/base.in
edx-django-sites-extensions==2.5.1 # via -r requirements/edx/base.in
edx-django-utils==3.2.3 # via -r requirements/edx/base.in, django-config-models, edx-drf-extensions, edx-enterprise, edx-rest-api-client, edx-when
diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt
index 3d5844698cb..ca7f51ecbc5 100644
--- a/requirements/edx/development.txt
+++ b/requirements/edx/development.txt
@@ -106,7 +106,7 @@ edx-api-doc-tools==1.3.1 # via -r requirements/edx/testing.txt
edx-bulk-grades==0.7.0 # via -r requirements/edx/testing.txt, staff-graded-xblock
edx-ccx-keys==1.1.0 # via -r requirements/edx/testing.txt
edx-celeryutils==0.5.1 # via -r requirements/edx/testing.txt, super-csv
-edx-completion==3.2.1 # via -c requirements/edx/../constraints.txt, -r requirements/edx/testing.txt
+edx-completion==3.2.3 # via -r requirements/edx/testing.txt
edx-django-release-util==0.4.4 # via -r requirements/edx/testing.txt
edx-django-sites-extensions==2.5.1 # via -r requirements/edx/testing.txt
edx-django-utils==3.2.3 # via -r requirements/edx/testing.txt, django-config-models, edx-drf-extensions, edx-enterprise, edx-rest-api-client, edx-when
diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt
index b40a92de4cd..2ed9d484919 100644
--- a/requirements/edx/testing.txt
+++ b/requirements/edx/testing.txt
@@ -103,7 +103,7 @@ edx-api-doc-tools==1.3.1 # via -r requirements/edx/base.txt
edx-bulk-grades==0.7.0 # via -r requirements/edx/base.txt, staff-graded-xblock
edx-ccx-keys==1.1.0 # via -r requirements/edx/base.txt
edx-celeryutils==0.5.1 # via -r requirements/edx/base.txt, super-csv
-edx-completion==3.2.1 # via -c requirements/edx/../constraints.txt, -r requirements/edx/base.txt
+edx-completion==3.2.3 # via -r requirements/edx/base.txt
edx-django-release-util==0.4.4 # via -r requirements/edx/base.txt
edx-django-sites-extensions==2.5.1 # via -r requirements/edx/base.txt
edx-django-utils==3.2.3 # via -r requirements/edx/base.txt, django-config-models, edx-drf-extensions, edx-enterprise, edx-rest-api-client, edx-when
--
GitLab