-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Add block completion value as optional field in course_blocks.api. #16674
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
nasthagiri
merged 2 commits into
openedx:master
from
open-craft:tomaszgy/completion_in_blocks_api
Dec 13, 2017
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
86 changes: 86 additions & 0 deletions
86
lms/djangoapps/course_api/blocks/transformers/block_completion.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| """ | ||
| Block Completion Transformer | ||
| """ | ||
|
|
||
| from xblock.completable import XBlockCompletionMode as CompletionMode | ||
|
|
||
| from lms.djangoapps.completion.models import BlockCompletion | ||
| from openedx.core.djangoapps.content.block_structure.transformer import BlockStructureTransformer | ||
|
|
||
|
|
||
| class BlockCompletionTransformer(BlockStructureTransformer): | ||
| """ | ||
| Keep track of the completion of each block within the block structure. | ||
| """ | ||
| READ_VERSION = 0 | ||
| WRITE_VERSION = 1 | ||
| COMPLETION = 'completion' | ||
|
|
||
| def __init__(self): | ||
| super(BlockCompletionTransformer, self).__init__() | ||
|
|
||
| @classmethod | ||
| def name(cls): | ||
| return "blocks_api:completion" | ||
|
|
||
| @classmethod | ||
| def get_block_completion(cls, block_structure, block_key): | ||
| """ | ||
| Return the precalculated completion of a block within the block_structure: | ||
|
|
||
| Arguments: | ||
| block_structure: a BlockStructure instance | ||
| block_key: the key of the block whose completion we want to know | ||
|
|
||
| Returns: | ||
| block_completion: float or None | ||
| """ | ||
| return block_structure.get_transformer_block_field( | ||
| block_key, | ||
| cls, | ||
| cls.COMPLETION, | ||
| ) | ||
|
|
||
| @classmethod | ||
| def collect(cls, block_structure): | ||
| block_structure.request_xblock_fields('completion_mode') | ||
|
|
||
| def transform(self, usage_info, block_structure): | ||
| """ | ||
| Mutates block_structure adding extra field which contains block's completion. | ||
| """ | ||
| def _is_block_an_aggregator_or_excluded(block_key): | ||
| """ | ||
| Checks whether block's completion method | ||
| is of `AGGREGATOR` or `EXCLUDED` type. | ||
| """ | ||
| completion_mode = block_structure.get_xblock_field( | ||
| block_key, 'completion_mode' | ||
| ) | ||
|
|
||
| return completion_mode in (CompletionMode.AGGREGATOR, CompletionMode.EXCLUDED) | ||
|
|
||
| completions = BlockCompletion.objects.filter( | ||
| user=usage_info.user, | ||
| course_key=usage_info.course_key, | ||
| ).values_list( | ||
| 'block_key', | ||
| 'completion', | ||
| ) | ||
|
|
||
| completions_dict = { | ||
| block.map_into_course(usage_info.course_key): completion | ||
| for block, completion in completions | ||
| } | ||
|
|
||
| for block_key in block_structure.topological_traversal(): | ||
| if _is_block_an_aggregator_or_excluded(block_key): | ||
| completion_value = None | ||
| elif block_key in completions_dict: | ||
| completion_value = completions_dict[block_key] | ||
| else: | ||
| completion_value = 0.0 | ||
|
|
||
| block_structure.set_transformer_block_field( | ||
| block_key, self, self.COMPLETION, completion_value | ||
| ) |
118 changes: 118 additions & 0 deletions
118
lms/djangoapps/course_api/blocks/transformers/tests/test_block_completion.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| """ | ||
| Tests for BlockCompletionTransformer. | ||
| """ | ||
| from xblock.core import XBlock | ||
| from xblock.completable import CompletableXBlockMixin, XBlockCompletionMode | ||
|
|
||
| from lms.djangoapps.completion.models import BlockCompletion | ||
| from lms.djangoapps.completion.test_utils import CompletionWaffleTestMixin | ||
| from lms.djangoapps.course_api.blocks.transformers.block_completion import BlockCompletionTransformer | ||
| from lms.djangoapps.course_blocks.transformers.tests.helpers import ModuleStoreTestCase, TransformerRegistryTestMixin | ||
| from student.tests.factories import UserFactory | ||
| from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory | ||
|
|
||
|
|
||
| from ...api import get_course_blocks | ||
|
|
||
|
|
||
| class StubAggregatorXBlock(XBlock): | ||
| """ | ||
| XBlock to test behaviour of BlockCompletionTransformer | ||
| when transforming aggregator XBlock. | ||
| """ | ||
| completion_mode = XBlockCompletionMode.AGGREGATOR | ||
|
|
||
|
|
||
| class StubExcludedXBlock(XBlock): | ||
| """ | ||
| XBlock to test behaviour of BlockCompletionTransformer | ||
| when transforming excluded XBlock. | ||
| """ | ||
| completion_mode = XBlockCompletionMode.EXCLUDED | ||
|
|
||
|
|
||
| class StubCompletableXBlock(XBlock, CompletableXBlockMixin): | ||
| """ | ||
| XBlock to test behaviour of BlockCompletionTransformer | ||
| when transforming completable XBlock. | ||
| """ | ||
| pass | ||
|
|
||
|
|
||
| class BlockCompletionTransformerTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase, CompletionWaffleTestMixin): | ||
| """ | ||
| Tests behaviour of BlockCompletionTransformer | ||
| """ | ||
| TRANSFORMER_CLASS_TO_TEST = BlockCompletionTransformer | ||
| COMPLETION_TEST_VALUE = 0.4 | ||
|
|
||
| def setUp(self): | ||
| super(BlockCompletionTransformerTestCase, self).setUp() | ||
| self.user = UserFactory.create(password='test') | ||
| self.override_waffle_switch(True) | ||
|
|
||
| @XBlock.register_temp_plugin(StubAggregatorXBlock, identifier='aggregator') | ||
| def test_transform_gives_none_for_aggregator(self): | ||
| course = CourseFactory.create() | ||
| block = ItemFactory.create(category='aggregator', parent=course) | ||
| block_structure = get_course_blocks( | ||
| self.user, course.location, self.transformers | ||
| ) | ||
|
|
||
| self._assert_block_has_proper_completion_value( | ||
| block_structure, block.location, None | ||
| ) | ||
|
|
||
| @XBlock.register_temp_plugin(StubExcludedXBlock, identifier='excluded') | ||
| def test_transform_gives_none_for_excluded(self): | ||
| course = CourseFactory.create() | ||
| block = ItemFactory.create(category='excluded', parent=course) | ||
| block_structure = get_course_blocks( | ||
| self.user, course.location, self.transformers | ||
| ) | ||
|
|
||
| self._assert_block_has_proper_completion_value( | ||
| block_structure, block.location, None | ||
| ) | ||
|
|
||
| @XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp') | ||
| def test_transform_gives_value_for_completable(self): | ||
| course = CourseFactory.create() | ||
| block = ItemFactory.create(category='comp', parent=course) | ||
| BlockCompletion.objects.submit_completion( | ||
| user=self.user, | ||
| course_key=block.location.course_key, | ||
| block_key=block.location, | ||
| completion=self.COMPLETION_TEST_VALUE, | ||
| ) | ||
| block_structure = get_course_blocks( | ||
| self.user, course.location, self.transformers | ||
| ) | ||
|
|
||
| self._assert_block_has_proper_completion_value( | ||
| block_structure, block.location, self.COMPLETION_TEST_VALUE | ||
| ) | ||
|
|
||
| def test_transform_gives_zero_for_ordinary_block(self): | ||
| course = CourseFactory.create() | ||
| block = ItemFactory.create(category='html', parent=course) | ||
| block_structure = get_course_blocks( | ||
| self.user, course.location, self.transformers | ||
| ) | ||
|
|
||
| self._assert_block_has_proper_completion_value( | ||
| block_structure, block.location, 0.0 | ||
| ) | ||
|
|
||
| def _assert_block_has_proper_completion_value( | ||
| self, block_structure, block_key, expected_value | ||
| ): | ||
| """ | ||
| Checks whether block's completion has expected value. | ||
| """ | ||
| block_data = block_structure.get_transformer_block_data( | ||
| block_key, self.TRANSFORMER_CLASS_TO_TEST | ||
| ) | ||
| completion_value = block_data.fields['completion'] | ||
|
|
||
| self.assertEqual(completion_value, expected_value) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Future note to self (and others): It would be great if we can make this pluggable, so new Transformers don't need to manually update this list - and can be automatically extracted from registered transformers.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nasthagiri I remember implementing a registry for something once, and I found two basic ways of doing it: The first is by creating a registry class with a
register()decorator function:The other uses a metaclass defined on the superclass. Then when the class gets constructed, it is automatically registered in the metaclass. Django models use this method.
The major differences are:
class Registerable(Registry):
register = False
The downside of allowing programmers to opt out (or more accurately opt not to opt-in) is that you also make it possible to forget to opt-in.