From f2a1984f1a47b6fe233b262025a070d6ff4afdd0 Mon Sep 17 00:00:00 2001 From: sanfordstudent Date: Thu, 21 Jul 2016 17:04:07 -0400 Subject: [PATCH] Revert "genericizing transformer" (#13063) This reverts commit 4e65b1f14b70c7cf5daa07a8a36d83fbbcb8f9fc. --- common/lib/xmodule/xmodule/seq_module.py | 8 - lms/djangoapps/course_api/blocks/api.py | 4 +- .../blocks/transformers/milestones.py | 74 ------- .../blocks/transformers/proctored_exam.py | 60 ++++++ .../transformers/tests/test_milestones.py | 197 ------------------ .../transformers/tests/test_proctored_exam.py | 173 +++++++++++++++ setup.py | 2 +- 7 files changed, 236 insertions(+), 282 deletions(-) delete mode 100644 lms/djangoapps/course_api/blocks/transformers/milestones.py create mode 100644 lms/djangoapps/course_api/blocks/transformers/proctored_exam.py delete mode 100644 lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py create mode 100644 lms/djangoapps/course_api/blocks/transformers/tests/test_proctored_exam.py diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 52c1742ecd..11933b0930 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -117,14 +117,6 @@ class ProctoringFields(object): scope=Scope.settings, ) - @property - def is_timed_exam(self): - """ - Alias the permutation of above fields that corresponds to un-proctored timed exams - to the more clearly-named is_timed_exam - """ - return not self.is_proctored_enabled and not self.is_practice_exam and self.is_time_limited - @property def is_proctored_exam(self): """ Alias the is_proctored_enabled field to the more legible is_proctored_exam """ diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index eb7ffb4fd7..58371b5c3d 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -7,7 +7,7 @@ from lms.djangoapps.course_blocks.transformers.hidden_content import HiddenConte from openedx.core.lib.block_structure.transformers import BlockStructureTransformers from .transformers.blocks_api import BlocksAPITransformer -from .transformers.milestones import MilestonesTransformer +from .transformers.proctored_exam import ProctoredExamTransformer from .serializers import BlockSerializer, BlockDictSerializer @@ -52,7 +52,7 @@ def get_blocks( # create ordered list of transformers, adding BlocksAPITransformer at end. transformers = BlockStructureTransformers() if user is not None: - transformers += COURSE_BLOCK_ACCESS_TRANSFORMERS + [MilestonesTransformer(), HiddenContentTransformer()] + transformers += COURSE_BLOCK_ACCESS_TRANSFORMERS + [ProctoredExamTransformer(), HiddenContentTransformer()] transformers += [ BlocksAPITransformer( block_counts, diff --git a/lms/djangoapps/course_api/blocks/transformers/milestones.py b/lms/djangoapps/course_api/blocks/transformers/milestones.py deleted file mode 100644 index 33ce6a9680..0000000000 --- a/lms/djangoapps/course_api/blocks/transformers/milestones.py +++ /dev/null @@ -1,74 +0,0 @@ -""" -Milestones Transformer -""" - -from django.conf import settings - -from openedx.core.lib.block_structure.transformer import BlockStructureTransformer, FilteringTransformerMixin -from util import milestones_helpers - - -class MilestonesTransformer(FilteringTransformerMixin, BlockStructureTransformer): - """ - Excludes all special exams (timed, proctored, practice proctored) from the student view. - Excludes all blocks with unfulfilled milestones from the student view. - """ - VERSION = 1 - - @classmethod - def name(cls): - return "milestones" - - @classmethod - def collect(cls, block_structure): - """ - Computes any information for each XBlock that's necessary to execute - this transformer's transform method. - - Arguments: - block_structure (BlockStructureCollectedData) - """ - block_structure.request_xblock_fields('is_proctored_enabled') - block_structure.request_xblock_fields('is_practice_exam') - block_structure.request_xblock_fields('is_timed_exam') - - def transform_block_filters(self, usage_info, block_structure): - if usage_info.has_staff_access: - return [block_structure.create_universal_filter()] - - def user_gated_from_block(block_key): - """ - Checks whether the user is gated from accessing this block, first via special exams, - then via a general milestones check. - """ - return ( - settings.FEATURES.get('ENABLE_SPECIAL_EXAMS', False) and - self.is_special_exam(block_key, block_structure) - ) or self.has_pending_milestones_for_user(block_key, usage_info) - - return [block_structure.create_removal_filter(user_gated_from_block)] - - @staticmethod - def is_special_exam(block_key, block_structure): - """ - Test whether the block is a special exam. These exams are always excluded - from the student view. - """ - return ( - block_structure.get_xblock_field(block_key, 'is_proctored_enabled') or - block_structure.get_xblock_field(block_key, 'is_practice_exam') or - block_structure.get_xblock_field(block_key, 'is_timed_exam') - ) - - @staticmethod - def has_pending_milestones_for_user(block_key, usage_info): - """ - Test whether the current user has any unfulfilled milestones preventing - them from accessing this block. - """ - return bool(milestones_helpers.get_course_content_milestones( - unicode(block_key.course_key), - unicode(block_key), - 'requires', - usage_info.user.id - )) diff --git a/lms/djangoapps/course_api/blocks/transformers/proctored_exam.py b/lms/djangoapps/course_api/blocks/transformers/proctored_exam.py new file mode 100644 index 0000000000..b59ff417be --- /dev/null +++ b/lms/djangoapps/course_api/blocks/transformers/proctored_exam.py @@ -0,0 +1,60 @@ +""" +Proctored Exams Transformer +""" + +from django.conf import settings + +from edx_proctoring.api import get_attempt_status_summary +from edx_proctoring.models import ProctoredExamStudentAttemptStatus +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer, FilteringTransformerMixin + + +class ProctoredExamTransformer(FilteringTransformerMixin, BlockStructureTransformer): + """ + Exclude proctored exams unless the user is not a verified student or has + declined taking the exam. + """ + VERSION = 1 + BLOCK_HAS_PROCTORED_EXAM = 'has_proctored_exam' + + @classmethod + def name(cls): + return "proctored_exam" + + @classmethod + def collect(cls, block_structure): + """ + Computes any information for each XBlock that's necessary to execute + this transformer's transform method. + + Arguments: + block_structure (BlockStructureCollectedData) + """ + block_structure.request_xblock_fields('is_proctored_enabled') + block_structure.request_xblock_fields('is_practice_exam') + + def transform_block_filters(self, usage_info, block_structure): + if not settings.FEATURES.get('ENABLE_PROCTORED_EXAMS', False): + return [block_structure.create_universal_filter()] + + def is_proctored_exam_for_user(block_key): + """ + Test whether the block is a proctored exam for the user in + question. + """ + if ( + block_key.block_type == 'sequential' and ( + block_structure.get_xblock_field(block_key, 'is_proctored_enabled') or + block_structure.get_xblock_field(block_key, 'is_practice_exam') + ) + ): + # This section is an exam. It should be excluded unless the + # user is not a verified student or has declined taking the exam. + user_exam_summary = get_attempt_status_summary( + usage_info.user.id, + unicode(block_key.course_key), + unicode(block_key), + ) + return user_exam_summary and user_exam_summary['status'] != ProctoredExamStudentAttemptStatus.declined + + return [block_structure.create_removal_filter(is_proctored_exam_for_user)] diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py deleted file mode 100644 index 1fc8d370b3..0000000000 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py +++ /dev/null @@ -1,197 +0,0 @@ -""" -Tests for ProctoredExamTransformer. -""" -from mock import patch, Mock -from nose.plugins.attrib import attr - -import ddt -from gating import api as lms_gating_api -from lms.djangoapps.course_blocks.transformers.tests.helpers import CourseStructureTestCase -from milestones.tests.utils import MilestonesTestCaseMixin -from opaque_keys.edx.keys import UsageKey -from openedx.core.lib.gating import api as gating_api -from student.tests.factories import CourseEnrollmentFactory - -from ..milestones import MilestonesTransformer -from ...api import get_course_blocks - - -@attr('shard_3') -@ddt.ddt -@patch.dict('django.conf.settings.FEATURES', {'ENABLE_SPECIAL_EXAMS': True, 'MILESTONES_APP': True}) -class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseMixin): - """ - Test behavior of ProctoredExamTransformer - """ - TRANSFORMER_CLASS_TO_TEST = MilestonesTransformer - - def setUp(self): - """ - Setup course structure and create user for split test transformer test. - """ - super(MilestonesTransformerTestCase, self).setUp() - - # Build course. - self.course_hierarchy = self.get_course_hierarchy() - self.blocks = self.build_course(self.course_hierarchy) - self.course = self.blocks['course'] - - # Enroll user in course. - CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) - - def setup_gated_section(self, gated_block, gating_block): - """ - Test helper to create a gating requirement. - Args: - gated_block: The block that should be inaccessible until gating_block is completed - gating_block: The block that must be completed before access is granted - """ - gating_api.add_prerequisite(self.course.id, unicode(gating_block.location)) - gating_api.set_required_content(self.course.id, gated_block.location, gating_block.location, 100) - - ALL_BLOCKS = ( - 'course', 'A', 'B', 'C', 'ProctoredExam', 'D', 'E', 'PracticeExam', 'F', 'G', 'H', 'I', 'TimedExam', 'J', 'K' - ) - - # The special exams (proctored, practice, timed) should never be visible to students - ALL_BLOCKS_EXCEPT_SPECIAL = ('course', 'A', 'B', 'C', 'H', 'I') - - def get_course_hierarchy(self): - """ - Get a course hierarchy to test with. - """ - - # course - # / / \ \ \ - # / / \ \ \ - # A ProctoredExam PracticeExam H TimedExam - # / \ / \ / \ | / \ - # / \ / \ / \ | / \ - # B C D E F G I J K - # - return [ - { - 'org': 'MilestonesTransformer', - 'course': 'PE101F', - 'run': 'test_run', - '#type': 'course', - '#ref': 'course', - }, - { - '#type': 'sequential', - '#ref': 'A', - '#children': [ - {'#type': 'vertical', '#ref': 'B'}, - {'#type': 'vertical', '#ref': 'C'}, - ], - }, - { - '#type': 'sequential', - '#ref': 'ProctoredExam', - 'is_time_limited': True, - 'is_proctored_enabled': True, - 'is_practice_exam': False, - '#children': [ - {'#type': 'vertical', '#ref': 'D'}, - {'#type': 'vertical', '#ref': 'E'}, - ], - }, - { - '#type': 'sequential', - '#ref': 'PracticeExam', - 'is_time_limited': True, - 'is_proctored_enabled': True, - 'is_practice_exam': True, - '#children': [ - {'#type': 'vertical', '#ref': 'F'}, - {'#type': 'vertical', '#ref': 'G'}, - ], - }, - { - '#type': 'sequential', - '#ref': 'H', - '#children': [ - {'#type': 'vertical', '#ref': 'I'}, - ], - }, - { - '#type': 'sequential', - '#ref': 'TimedExam', - 'is_time_limited': True, - 'is_proctored_enabled': False, - 'is_practice_exam': False, - '#children': [ - {'#type': 'vertical', '#ref': 'J'}, - {'#type': 'vertical', '#ref': 'K'}, - ], - }, - ] - - def test_special_exams_not_visible_to_non_staff(self): - self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL) - - @ddt.data( - ( - 'H', - 'A', - 'B', - ('course', 'A', 'B', 'C',) - ), - ( - 'H', - 'ProctoredExam', - 'D', - ('course', 'A', 'B', 'C'), - ), - ) - @ddt.unpack - def test_gated(self, gated_block_ref, gating_block_ref, gating_block_child, expected_blocks_before_completion): - """ - First, checks that a student cannot see the gated block when it is gated by the gating block and no - attempt has been made to complete the gating block. - Then, checks that the student can see the gated block after the gating block has been completed. - - expected_blocks_before_completion is the set of blocks we expect to be visible to the student - before the student has completed the gating block. - - The test data includes one special exam and one non-special block as the gating blocks. - """ - self.course.enable_subsection_gating = True - self.setup_gated_section(self.blocks[gated_block_ref], self.blocks[gating_block_ref]) - self.get_blocks_and_check_against_expected(self.user, expected_blocks_before_completion) - - # mock the api that the lms gating api calls to get the score for each block to always return 1 (ie 100%) - with patch('courseware.grades.get_module_score', Mock(return_value=1)): - - # this call triggers reevaluation of prerequisites fulfilled by the parent of the - # block passed in, so we pass in a child of the gating block - lms_gating_api.evaluate_prerequisite( - self.course, - UsageKey.from_string(unicode(self.blocks[gating_block_child].location)), - self.user.id) - - self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL) - - def test_staff_access(self): - """ - Ensures that staff can always access all blocks in the course, - regardless of gating or proctoring. - """ - expected_blocks = self.ALL_BLOCKS - self.setup_gated_section(self.blocks['H'], self.blocks['A']) - self.get_blocks_and_check_against_expected(self.staff, expected_blocks) - - def get_blocks_and_check_against_expected(self, user, expected_blocks): - """ - Calls the course API as the specified user and checks the - output against a specified set of expected blocks. - """ - block_structure = get_course_blocks( - user, - self.course.location, - self.transformers, - ) - self.assertEqual( - set(block_structure.get_block_keys()), - set(self.get_block_key_set(self.blocks, *expected_blocks)), - ) diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_proctored_exam.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_proctored_exam.py new file mode 100644 index 0000000000..ef6111fa3f --- /dev/null +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_proctored_exam.py @@ -0,0 +1,173 @@ +""" +Tests for ProctoredExamTransformer. +""" +from mock import patch +from nose.plugins.attrib import attr + +import ddt +from edx_proctoring.api import ( + create_exam, + create_exam_attempt, + update_attempt_status +) +from edx_proctoring.models import ProctoredExamStudentAttemptStatus +from edx_proctoring.runtime import set_runtime_service +from edx_proctoring.tests.test_services import MockCreditService +from lms.djangoapps.course_blocks.transformers.tests.helpers import CourseStructureTestCase +from student.tests.factories import CourseEnrollmentFactory + +from ..proctored_exam import ProctoredExamTransformer +from ...api import get_course_blocks + + +@attr('shard_3') +@ddt.ddt +@patch.dict('django.conf.settings.FEATURES', {'ENABLE_PROCTORED_EXAMS': True}) +class ProctoredExamTransformerTestCase(CourseStructureTestCase): + """ + Test behavior of ProctoredExamTransformer + """ + TRANSFORMER_CLASS_TO_TEST = ProctoredExamTransformer + + def setUp(self): + """ + Setup course structure and create user for split test transformer test. + """ + super(ProctoredExamTransformerTestCase, self).setUp() + + # Set up proctored exam + + # Build course. + self.course_hierarchy = self.get_course_hierarchy() + self.blocks = self.build_course(self.course_hierarchy) + self.course = self.blocks['course'] + + # Enroll user in course. + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) + + def setup_proctored_exam(self, block, attempt_status, user_id): + """ + Test helper to configure the given block as a proctored exam. + """ + exam_id = create_exam( + course_id=unicode(block.location.course_key), + content_id=unicode(block.location), + exam_name='foo', + time_limit_mins=10, + is_proctored=True, + is_practice_exam=block.is_practice_exam, + ) + + set_runtime_service( + 'credit', + MockCreditService(enrollment_mode='verified') + ) + + create_exam_attempt(exam_id, user_id, taking_as_proctored=True) + update_attempt_status(exam_id, user_id, attempt_status) + + ALL_BLOCKS = ('course', 'A', 'B', 'C', 'TimedExam', 'D', 'E', 'PracticeExam', 'F', 'G') + + def get_course_hierarchy(self): + """ + Get a course hierarchy to test with. + """ + + # course + # / | \ + # / | \ + # A Exam1 Exam2 + # / \ / \ / \ + # / \ / \ / \ + # B C D E F G + # + return [ + { + 'org': 'ProctoredExamTransformer', + 'course': 'PE101F', + 'run': 'test_run', + '#type': 'course', + '#ref': 'course', + }, + { + '#type': 'sequential', + '#ref': 'A', + '#children': [ + {'#type': 'vertical', '#ref': 'B'}, + {'#type': 'vertical', '#ref': 'C'}, + ], + }, + { + '#type': 'sequential', + '#ref': 'TimedExam', + 'is_time_limited': True, + 'is_proctored_enabled': True, + 'is_practice_exam': False, + '#children': [ + {'#type': 'vertical', '#ref': 'D'}, + {'#type': 'vertical', '#ref': 'E'}, + ], + }, + { + '#type': 'sequential', + '#ref': 'PracticeExam', + 'is_time_limited': True, + 'is_proctored_enabled': True, + 'is_practice_exam': True, + '#children': [ + {'#type': 'vertical', '#ref': 'F'}, + {'#type': 'vertical', '#ref': 'G'}, + ], + }, + ] + + def test_exam_not_created(self): + block_structure = get_course_blocks( + self.user, + self.course.location, + self.transformers, + ) + self.assertEqual( + set(block_structure.get_block_keys()), + set(self.get_block_key_set(self.blocks, *self.ALL_BLOCKS)), + ) + + @ddt.data( + ( + 'TimedExam', + ProctoredExamStudentAttemptStatus.declined, + ALL_BLOCKS, + ), + ( + 'TimedExam', + ProctoredExamStudentAttemptStatus.submitted, + ('course', 'A', 'B', 'C', 'PracticeExam', 'F', 'G'), + ), + ( + 'TimedExam', + ProctoredExamStudentAttemptStatus.rejected, + ('course', 'A', 'B', 'C', 'PracticeExam', 'F', 'G'), + ), + ( + 'PracticeExam', + ProctoredExamStudentAttemptStatus.declined, + ALL_BLOCKS, + ), + ( + 'PracticeExam', + ProctoredExamStudentAttemptStatus.rejected, + ('course', 'A', 'B', 'C', 'TimedExam', 'D', 'E'), + ), + ) + @ddt.unpack + def test_exam_created(self, exam_ref, attempt_status, expected_blocks): + self.setup_proctored_exam(self.blocks[exam_ref], attempt_status, self.user.id) + block_structure = get_course_blocks( + self.user, + self.course.location, + self.transformers, + ) + self.assertEqual( + set(block_structure.get_block_keys()), + set(self.get_block_key_set(self.blocks, *expected_blocks)), + ) diff --git a/setup.py b/setup.py index c75a05ce51..5e72d514d6 100644 --- a/setup.py +++ b/setup.py @@ -51,7 +51,7 @@ setup( "visibility = lms.djangoapps.course_blocks.transformers.visibility:VisibilityTransformer", "hidden_content = lms.djangoapps.course_blocks.transformers.hidden_content:HiddenContentTransformer", "course_blocks_api = lms.djangoapps.course_api.blocks.transformers.blocks_api:BlocksAPITransformer", - "milestones = lms.djangoapps.course_api.blocks.transformers.milestones:MilestonesTransformer", + "proctored_exam = lms.djangoapps.course_api.blocks.transformers.proctored_exam:ProctoredExamTransformer", "grades = lms.djangoapps.courseware.transformers.grades:GradesTransformer", ], }