Merge pull request #9447 from…

Merge pull request #9447 from edx/ahsan/ECOM-2118-Credit-Eligible-Assessments-Appear-in-Incorrect-Order

Credit-Eligible Assessments Appearing in Incorrect Order

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']
-    ]
-
-    log_msg = (
-        'Registering the following as \'proctored_exam\' credit requirements: {log_msg}'.format(
-            log_msg=requirements
+    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
+
+            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

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'
-        ]
+        on_course_publish(self.course.id)
 
-        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.