diff --git a/openedx/core/djangoapps/credit/tasks.py b/openedx/core/djangoapps/credit/tasks.py index 51c2af2874..1c0ba2468d 100644 --- a/openedx/core/djangoapps/credit/tasks.py +++ b/openedx/core/djangoapps/credit/tasks.py @@ -10,13 +10,12 @@ from django.conf import settings from celery import task from celery.utils.log import get_task_logger from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.keys import CourseKey, UsageKey -from .api import set_credit_requirements +from openedx.core.djangoapps.credit.api import set_credit_requirements from openedx.core.djangoapps.credit.exceptions import InvalidCreditRequirements from openedx.core.djangoapps.credit.models import CreditCourse from openedx.core.djangoapps.credit.utils import get_course_blocks -from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError @@ -75,8 +74,15 @@ def _get_course_credit_requirements(course_key): credit_xblock_requirements = _get_credit_course_requirement_xblocks(course_key) min_grade_requirement = _get_min_grade_requirement(course_key) proctored_exams_requirements = _get_proctoring_requirements(course_key) + block_requirements = credit_xblock_requirements + proctored_exams_requirements + # sort credit requirements list based on start date and put all the + # requirements with no start date at the end of requirement list. + sorted_block_requirements = sorted( + block_requirements, key=lambda x: (x['start_date'] is None, x['start_date'], x['display_name']) + ) + credit_requirements = ( - min_grade_requirement + credit_xblock_requirements + proctored_exams_requirements + min_grade_requirement + sorted_block_requirements ) return credit_requirements @@ -131,6 +137,7 @@ def _get_credit_course_requirement_xblocks(course_key): # pylint: disable=inval "namespace": block.get_credit_requirement_namespace(), "name": block.get_credit_requirement_name(), "display_name": block.get_credit_requirement_display_name(), + 'start_date': block.start, "criteria": {}, } for block in _get_xblocks(course_key, category) @@ -148,15 +155,6 @@ def _get_xblocks(course_key, category): """ xblocks = get_course_blocks(course_key, category) - # Secondary sort on credit requirement name - xblocks = sorted(xblocks, key=lambda block: block.get_credit_requirement_display_name()) - - # Primary sort on start date - xblocks = sorted(xblocks, key=lambda block: ( - block.start if block.start is not None - else datetime.datetime(datetime.MINYEAR, 1, 1).replace(tzinfo=UTC) - )) - return xblocks @@ -208,23 +206,32 @@ def _get_proctoring_requirements(course_key): # process from edx_proctoring.api import get_all_exams_for_course - requirements = [ - { - 'namespace': 'proctored_exam', - 'name': exam['content_id'], - 'display_name': exam['exam_name'], - 'criteria': {}, - } - for exam in get_all_exams_for_course(unicode(course_key)) - # practice exams do not count towards eligibility - if exam['is_proctored'] and exam['is_active'] and not exam['is_practice_exam'] - ] + requirements = [] + for exam in get_all_exams_for_course(unicode(course_key)): + if exam['is_proctored'] and exam['is_active'] and not exam['is_practice_exam']: + try: + usage_key = UsageKey.from_string(exam['content_id']) + proctor_block = modulestore().get_item(usage_key) + except (InvalidKeyError, ItemNotFoundError): + LOGGER.info("Invalid content_id '%s' for proctored block '%s'", exam['content_id'], exam['exam_name']) + proctor_block = None - log_msg = ( - 'Registering the following as \'proctored_exam\' credit requirements: {log_msg}'.format( - log_msg=requirements + if proctor_block: + requirements.append( + { + 'namespace': 'proctored_exam', + 'name': exam['content_id'], + 'display_name': exam['exam_name'], + 'start_date': proctor_block.start if proctor_block.start else None, + 'criteria': {}, + }) + + if requirements: + log_msg = ( + 'Registering the following as \'proctored_exam\' credit requirements: {log_msg}'.format( + log_msg=requirements + ) ) - ) - LOGGER.info(log_msg) + LOGGER.info(log_msg) return requirements diff --git a/openedx/core/djangoapps/credit/tests/test_tasks.py b/openedx/core/djangoapps/credit/tests/test_tasks.py index 70f838d7c7..ca41700c0f 100644 --- a/openedx/core/djangoapps/credit/tests/test_tasks.py +++ b/openedx/core/djangoapps/credit/tests/test_tasks.py @@ -103,9 +103,10 @@ class TestTaskExecution(ModuleStoreTestCase): """ self.add_credit_course(self.course.id) + create_exam( course_id=unicode(self.course.id), - content_id='foo', + content_id=unicode(self.subsection.location), exam_name='A Proctored Exam', time_limit_mins=10, is_proctored=True, @@ -114,20 +115,15 @@ class TestTaskExecution(ModuleStoreTestCase): requirements = get_credit_requirements(self.course.id) self.assertEqual(len(requirements), 0) + on_course_publish(self.course.id) - # just inspect the proctored exam requirement - requirements = [ - requirement - for requirement in get_credit_requirements(self.course.id) - if requirement['namespace'] == 'proctored_exam' - ] - - self.assertEqual(len(requirements), 1) - self.assertEqual(requirements[0]['namespace'], 'proctored_exam') - self.assertEqual(requirements[0]['name'], 'foo') - self.assertEqual(requirements[0]['display_name'], 'A Proctored Exam') - self.assertEqual(requirements[0]['criteria'], {}) + requirements = get_credit_requirements(self.course.id) + self.assertEqual(len(requirements), 2) + self.assertEqual(requirements[1]['namespace'], 'proctored_exam') + self.assertEqual(requirements[1]['name'], unicode(self.subsection.location)) + self.assertEqual(requirements[1]['display_name'], 'A Proctored Exam') + self.assertEqual(requirements[1]['criteria'], {}) def test_proctored_exam_filtering(self): """ @@ -257,15 +253,17 @@ class TestTaskExecution(ModuleStoreTestCase): on_course_publish(self.course.id) requirements = get_credit_requirements(self.course.id, namespace="reverification") self.assertEqual(len(requirements), 4) - self.assertEqual(requirements[0]["display_name"], "Midterm Start Date") - self.assertEqual(requirements[1]["display_name"], "Midterm Start Date") - self.assertEqual(requirements[2]["display_name"], "Midterm B") - self.assertEqual(requirements[3]["display_name"], "Midterm A") + # Since we are now primarily sorting on start_date and display_name if + # start_date is present otherwise we are just sorting on display_name. + self.assertEqual(requirements[0]["display_name"], "Midterm B") + self.assertEqual(requirements[1]["display_name"], "Midterm A") + self.assertEqual(requirements[2]["display_name"], "Midterm Start Date") + self.assertEqual(requirements[3]["display_name"], "Midterm Start Date") - # Since the first two requirements have the same display name, + # Since the last two requirements have the same display name, # we need to also check that their internal names (locations) are the same. - self.assertEqual(requirements[0]["name"], first_block.get_credit_requirement_name()) - self.assertEqual(requirements[1]["name"], second_block.get_credit_requirement_name()) + self.assertEqual(requirements[2]["name"], first_block.get_credit_requirement_name()) + self.assertEqual(requirements[3]["name"], second_block.get_credit_requirement_name()) @mock.patch( 'openedx.core.djangoapps.credit.tasks.set_credit_requirements', @@ -288,6 +286,52 @@ class TestTaskExecution(ModuleStoreTestCase): requirements = get_credit_requirements(self.course.id) self.assertEqual(len(requirements), 0) + def test_credit_requirement_blocks_ordering(self): + """ + Test ordering of the proctoring and ICRV blocks are in proper order. + """ + + self.add_credit_course(self.course.id) + subsection = ItemFactory.create(parent=self.section, category='sequential', display_name='Dummy Subsection') + create_exam( + course_id=unicode(self.course.id), + content_id=unicode(subsection.location), + exam_name='A Proctored Exam', + time_limit_mins=10, + is_proctored=True, + is_active=True + ) + + requirements = get_credit_requirements(self.course.id) + self.assertEqual(len(requirements), 0) + on_course_publish(self.course.id) + + requirements = get_credit_requirements(self.course.id) + self.assertEqual(len(requirements), 2) + self.assertEqual(requirements[1]['namespace'], 'proctored_exam') + self.assertEqual(requirements[1]['name'], unicode(subsection.location)) + self.assertEqual(requirements[1]['display_name'], 'A Proctored Exam') + self.assertEqual(requirements[1]['criteria'], {}) + + # Create multiple ICRV blocks + start = datetime.now(UTC) + self.add_icrv_xblock(related_assessment_name="Midterm A", start_date=start) + + start = start - timedelta(days=1) + self.add_icrv_xblock(related_assessment_name="Midterm B", start_date=start) + + # Primary sort is based on start date + on_course_publish(self.course.id) + requirements = get_credit_requirements(self.course.id) + # grade requirement is added on publish of the requirements + self.assertEqual(len(requirements), 4) + # check requirements are added in the desired order + # 1st Minimum grade then the blocks with start date than other blocks + self.assertEqual(requirements[0]["display_name"], "Minimum Grade") + self.assertEqual(requirements[1]["display_name"], "A Proctored Exam") + self.assertEqual(requirements[2]["display_name"], "Midterm B") + self.assertEqual(requirements[3]["display_name"], "Midterm A") + def add_credit_course(self, course_key): """Add the course as a credit.