From 5ae5a531f43f2647de3d47dae43a8b58e3688611 Mon Sep 17 00:00:00 2001 From: Amir Qayyum Khan Date: Tue, 13 Mar 2018 19:01:28 +0500 Subject: [PATCH] Fixed override due date by limiting scope to selected users Added selected user tests updated my email --- AUTHORS | 2 +- lms/djangoapps/course_api/blocks/api.py | 2 +- .../blocks/tests/test_serializers.py | 2 +- lms/djangoapps/course_blocks/api.py | 11 +++- .../transformers/load_override_data.py | 17 +++-- .../tests/test_load_override_data.py | 66 ++++++++++++++++++- 6 files changed, 86 insertions(+), 14 deletions(-) diff --git a/AUTHORS b/AUTHORS index 5bee98e40e..d4506f9957 100644 --- a/AUTHORS +++ b/AUTHORS @@ -208,7 +208,7 @@ Phil McGachey Sri Harsha Pamu Cris Ewing Carlos de La Guardia -Amir Qayyum Khan +Amir Qayyum Khan Jolyon Bloomfield Kyle McCormick Jim Cai diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index b8ab703056..dbac1db1f7 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -59,7 +59,7 @@ def get_blocks( include_gated_sections = 'show_gated_sections' in requested_fields if user is not None: - transformers += course_blocks_api.get_course_block_access_transformers() + transformers += course_blocks_api.get_course_block_access_transformers(user) transformers += [MilestonesAndSpecialExamsTransformer( include_special_exams=include_special_exams, include_gated_sections=include_gated_sections)] diff --git a/lms/djangoapps/course_api/blocks/tests/test_serializers.py b/lms/djangoapps/course_api/blocks/tests/test_serializers.py index 9ed2b4c1a0..3fbb46e83f 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_serializers.py +++ b/lms/djangoapps/course_api/blocks/tests/test_serializers.py @@ -44,7 +44,7 @@ class TestBlockSerializerBase(SharedModuleStoreTestCase): requested_student_view_data=['video'], ) self.transformers = BlockStructureTransformers( - get_course_block_access_transformers() + [blocks_api_transformer] + get_course_block_access_transformers(self.user) + [blocks_api_transformer] ) self.block_structure = get_course_blocks( self.user, diff --git a/lms/djangoapps/course_blocks/api.py b/lms/djangoapps/course_blocks/api.py index a2dda446e0..0ed78febfa 100644 --- a/lms/djangoapps/course_blocks/api.py +++ b/lms/djangoapps/course_blocks/api.py @@ -21,10 +21,15 @@ def has_individual_student_override_provider(): return INDIVIDUAL_STUDENT_OVERRIDE_PROVIDER in getattr(settings, 'FIELD_OVERRIDE_PROVIDERS', ()) -def get_course_block_access_transformers(): +def get_course_block_access_transformers(user): """ Default list of transformers for manipulating course block structures based on the user's access to the course blocks. + + Arguments: + user (django.contrib.auth.models.User) - User object for + which the block structure is to be transformed. + """ course_block_access_transformers = [ library_content.ContentLibraryTransformer(), @@ -33,7 +38,7 @@ def get_course_block_access_transformers(): visibility.VisibilityTransformer(), ] if has_individual_student_override_provider(): - course_block_access_transformers += [load_override_data.OverrideDataTransformer()] + course_block_access_transformers += [load_override_data.OverrideDataTransformer(user)] return course_block_access_transformers @@ -75,7 +80,7 @@ def get_course_blocks( access. """ if not transformers: - transformers = BlockStructureTransformers(get_course_block_access_transformers()) + transformers = BlockStructureTransformers(get_course_block_access_transformers(user)) transformers.usage_info = CourseUsageInfo(starting_block_usage_key.course_key, user) return get_block_structure_manager(starting_block_usage_key.course_key).get_transformed( diff --git a/lms/djangoapps/course_blocks/transformers/load_override_data.py b/lms/djangoapps/course_blocks/transformers/load_override_data.py index 19662097ad..7ab6eec772 100644 --- a/lms/djangoapps/course_blocks/transformers/load_override_data.py +++ b/lms/djangoapps/course_blocks/transformers/load_override_data.py @@ -15,22 +15,24 @@ REQUESTED_FIELDS = [ ] -def _get_override_query(course_key, location_list): +def _get_override_query(course_key, location_list, user_id): """ returns queryset containing override data. Args: course_key (CourseLocator): Course locator object location_list (List): List of usage key of all blocks + user_id (int): User id """ return StudentFieldOverride.objects.filter( course_id=course_key, location__in=location_list, - field__in=REQUESTED_FIELDS + field__in=REQUESTED_FIELDS, + student__id=user_id ) -def override_xblock_fields(course_key, location_list, block_structure): +def override_xblock_fields(course_key, location_list, block_structure, user_id): """ loads override data of block @@ -38,8 +40,9 @@ def override_xblock_fields(course_key, location_list, block_structure): course_key (CourseLocator): course locator object location_list (List): list of usage key of all blocks block_structure (BlockStructure): block structure class + user_id (int): User id """ - query = _get_override_query(course_key, location_list) + query = _get_override_query(course_key, location_list, user_id) for student_field_override in query: value = json.loads(student_field_override.value) field = student_field_override.field @@ -57,6 +60,9 @@ class OverrideDataTransformer(BlockStructureTransformer): WRITE_VERSION = 1 READ_VERSION = 1 + def __init__(self, user): + self.user = user + @classmethod def name(cls): """ @@ -80,5 +86,6 @@ class OverrideDataTransformer(BlockStructureTransformer): override_xblock_fields( usage_info.course_key, block_structure.topological_traversal(), - block_structure + block_structure, + self.user.id ) diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_load_override_data.py b/lms/djangoapps/course_blocks/transformers/tests/test_load_override_data.py index e1966891e7..32c757c7d9 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_load_override_data.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_load_override_data.py @@ -35,7 +35,8 @@ class TestOverrideDataTransformer(ModuleStoreTestCase): @classmethod def setUpClass(cls): super(TestOverrideDataTransformer, cls).setUpClass() - cls.learner = UserFactory.create(password="test") + cls.learner = UserFactory.create() + cls.learner2 = UserFactory.create() def setUp(self): super(TestOverrideDataTransformer, self).setUp() @@ -49,9 +50,16 @@ class TestOverrideDataTransformer(ModuleStoreTestCase): self.learner.id, subsection.location, 'html', 'new_component' ) CourseEnrollmentFactory.create(user=self.learner, course_id=self.course_key, is_active=True) + self.block = self.store.create_child( + self.learner2.id, subsection.location, 'html', 'new_component' + ) + CourseEnrollmentFactory.create(user=self.learner2, course_id=self.course_key, is_active=True) @ddt.data(*REQUESTED_FIELDS) def test_transform(self, field): + """ + assert that data transformed + """ override_field_for_user( self.learner, self.block, @@ -63,7 +71,7 @@ class TestOverrideDataTransformer(ModuleStoreTestCase): OverrideDataTransformer.collect(self.block_structure) # transform phase - OverrideDataTransformer().transform( + OverrideDataTransformer(self.learner).transform( usage_info=self.course_usage_key, block_structure=self.block_structure, ) @@ -71,6 +79,32 @@ class TestOverrideDataTransformer(ModuleStoreTestCase): # verify overridden data assert get_override_for_user(self.learner, self.block, field) == expected_overrides.get(field) + @ddt.data(*REQUESTED_FIELDS) + def test_transform_for_selected_learner(self, field): + """ + assert that data transformed for selected learner + """ + override_field_for_user( + self.learner, + self.block, + field, + expected_overrides.get(field) + ) + + # collect phase + OverrideDataTransformer.collect(self.block_structure) + + # transform phase + OverrideDataTransformer(self.learner).transform( + usage_info=self.course_usage_key, + block_structure=self.block_structure, + ) + + # verify learner has overridden data + assert get_override_for_user(self.learner, self.block, field) == expected_overrides.get(field) + # other learner2 dont have overridden data + assert get_override_for_user(self.learner2, self.block, field) is None + def test_transform_all_fields(self): """Test overriding of all fields""" for field in REQUESTED_FIELDS: @@ -85,7 +119,7 @@ class TestOverrideDataTransformer(ModuleStoreTestCase): OverrideDataTransformer.collect(self.block_structure) # transform phase - OverrideDataTransformer().transform( + OverrideDataTransformer(self.learner).transform( usage_info=self.course_usage_key, block_structure=self.block_structure, ) @@ -93,3 +127,29 @@ class TestOverrideDataTransformer(ModuleStoreTestCase): # verify overridden data for field in REQUESTED_FIELDS: assert get_override_for_user(self.learner, self.block, field) == expected_overrides.get(field) + + def test_transform_all_fields_for_selected_learner(self): + """Test overriding of all fields""" + for field in REQUESTED_FIELDS: + override_field_for_user( + self.learner, + self.block, + field, + expected_overrides.get(field) + ) + + # collect phase + OverrideDataTransformer.collect(self.block_structure) + + # transform phase + OverrideDataTransformer(self.learner).transform( + usage_info=self.course_usage_key, + block_structure=self.block_structure, + ) + + # verify overridden data + for field in REQUESTED_FIELDS: + # learner has overridden data + assert get_override_for_user(self.learner, self.block, field) == expected_overrides.get(field) + # other learner2 dont have overridden data + assert get_override_for_user(self.learner2, self.block, field) is None