Merge pull request #8122 from edx/aamir-khan/ECOM-1590-populate-requirements-table-new

ECOM-1590: added the min_grade as a requirements

cms/envs/common.py

@@ -960,3 +960,13 @@ XBLOCK_SETTINGS = {
         "licensing_enabled": FEATURES.get("LICENSING", False)
     }
 }
+
+################################ Settings for Credit Course Requirements ################################
+# Initial delay used for retrying tasks.
+# Additional retries use longer delays.
+# Value is in seconds.
+CREDIT_TASK_DEFAULT_RETRY_DELAY = 30
+
+# Maximum number of retries per task for errors that are not related
+# to throttling.
+CREDIT_TASK_MAX_RETRIES = 5

lms/envs/common.py

@@ -2366,3 +2366,13 @@ PREVIEW_DOMAIN = 'preview'
 # Sets the maximum number of courses listed on the homepage
 # If set to None, all courses will be listed on the homepage
 HOMEPAGE_COURSE_MAX = None
+
+################################ Settings for Credit Course Requirements ################################
+# Initial delay used for retrying tasks.
+# Additional retries use longer delays.
+# Value is in seconds.
+CREDIT_TASK_DEFAULT_RETRY_DELAY = 30
+
+# Maximum number of retries per task for errors that are not related
+# to throttling.
+CREDIT_TASK_MAX_RETRIES = 5

openedx/core/djangoapps/credit/__init__.py

+""" Register signal handlers """
+
+from . import signals

openedx/core/djangoapps/credit/api.py

+""" Contains the APIs for course credit requirements """
+
+from .exceptions import InvalidCreditRequirements
+from .models import CreditCourse, CreditRequirement
+from openedx.core.djangoapps.credit.exceptions import InvalidCreditCourse
+
+
+def set_credit_requirements(course_key, requirements):
+    """ Add requirements to given course
+
+    Args:
+        course_key(CourseKey): The identifier for course
+        requirements(list): List of requirements to be added
+
+    Example:
+        >>> set_credit_requirements(
+                "course-v1-edX-DemoX-1T2015",
+                [
+                    {
+                        "namespace": "verification",
+                        "name": "verification",
+                        "criteria": {},
+                    },
+                    {
+                        "namespace": "reverification",
+                        "name": "midterm",
+                        "criteria": {},
+                    },
+                    {
+                        "namespace": "proctored_exam",
+                        "name": "final",
+                        "criteria": {},
+                    },
+                    {
+                        "namespace": "grade",
+                        "name": "grade",
+                        "criteria": {"min_grade": 0.8},
+                    },
+                ])
+
+    Raises:
+        InvalidCreditRequirements
+
+    Returns:
+        None
+    """
+
+    invalid_requirements = _validate_requirements(requirements)
+    if invalid_requirements:
+        invalid_requirements = ", ".join(invalid_requirements)
+        raise InvalidCreditRequirements(invalid_requirements)
+
+    try:
+        credit_course = CreditCourse.get_credit_course(course_key=course_key)
+    except CreditCourse.DoesNotExist:
+        raise InvalidCreditCourse()
+
+    old_requirements = CreditRequirement.get_course_requirements(course_key=course_key)
+    requirements_to_disable = _get_requirements_to_disable(old_requirements, requirements)
+    if requirements_to_disable:
+        CreditRequirement.disable_credit_requirements(requirements_to_disable)
+
+    for requirement in requirements:
+        CreditRequirement.add_or_update_course_requirement(credit_course, requirement)
+
+
+def get_credit_requirements(course_key, namespace=None):
+    """ Returns the requirements of a given course and namespace
+
+    Args:
+        course_key(CourseKey): The identifier for course
+        namespace(str): Namespace of requirements
+
+    Example:
+        >>> get_credit_requirements("course-v1-edX-DemoX-1T2015")
+                {
+                    requirements =
+                    [
+                        {
+                            "namespace": "verification",
+                            "name": "verification",
+                            "criteria": {},
+                        },
+                        {
+                            "namespace": "reverification",
+                            "name": "midterm",
+                            "criteria": {},
+                        },
+                        {
+                            "namespace": "proctored_exam",
+                            "name": "final",
+                            "criteria": {},
+                        },
+                        {
+                            "namespace": "grade",
+                            "name": "grade",
+                            "criteria": {"min_grade": 0.8},
+                        },
+                    ]
+                }
+
+    Returns:
+        Dict of requirements in the given namespace
+    """
+
+    requirements = CreditRequirement.get_course_requirements(course_key, namespace)
+    return [
+        {
+            "namespace": requirement.namespace,
+            "name": requirement.name,
+            "criteria": requirement.criteria
+        }
+        for requirement in requirements
+    ]
+
+
+def _get_requirements_to_disable(old_requirements, new_requirements):
+    """ Returns the ids of CreditRequirement to be disabled that are deleted from the courseware
+
+    Args:
+        old_requirements(QuerySet): QuerySet of CreditRequirement
+        new_requirements(list): List of requirements being added
+
+    Returns:
+        List of ids of CreditRequirement that are not in new_requirements
+    """
+    requirements_to_disable = []
+    for old_req in old_requirements:
+        found_flag = False
+        for req in new_requirements:
+            if req["namespace"] == old_req.namespace and req["name"] == old_req.name:
+                found_flag = True
+                break
+        if not found_flag:
+            requirements_to_disable.append(old_req.id)
+    return requirements_to_disable
+
+
+def _validate_requirements(requirements):
+    """ Validate the requirements
+
+    Args:
+        requirements(list): List of requirements
+
+    Returns:
+        List of strings of invalid requirements
+    """
+    invalid_requirements = []
+    for requirement in requirements:
+        invalid_params = []
+        if not requirement.get("namespace"):
+            invalid_params.append("namespace")
+        if not requirement.get("name"):
+            invalid_params.append("name")
+        if "criteria" not in requirement:
+            invalid_params.append("criteria")
+
+        if invalid_params:
+            invalid_requirements.append(
+                u"{requirement} has missing/invalid parameters: {params}".format(
+                    requirement=requirement,
+                    params=invalid_params,
+                )
+            )
+    return invalid_requirements

openedx/core/djangoapps/credit/exceptions.py

+""" This module contains the exceptions raised in credit course requirements """
+
+
+class InvalidCreditRequirements(Exception):
+    """ The exception occurs when the requirement dictionary has invalid format. """
+    pass
+
+
+class InvalidCreditCourse(Exception):
+    """ The exception occurs when the the course is not marked as a Credit Course. """
+    pass

openedx/core/djangoapps/credit/models.py

@@ -9,9 +9,11 @@ successful completion of a course on EdX
 import logging
 
 from django.db import models
+
+from jsonfield.fields import JSONField
 from model_utils.models import TimeStampedModel
 from xmodule_django.models import CourseKeyField
-from jsonfield.fields import JSONField
+
 
 log = logging.getLogger(__name__)
 
@@ -22,6 +24,33 @@ class CreditCourse(models.Model):
     course_key = CourseKeyField(max_length=255, db_index=True, unique=True)
     enabled = models.BooleanField(default=False)
 
+    @classmethod
+    def is_credit_course(cls, course_key):
+        """ Check that given course is credit or not
+
+        Args:
+            course_key(CourseKey): The course identifier
+
+        Returns:
+            Bool True if the course is marked credit else False
+        """
+        return cls.objects.filter(course_key=course_key, enabled=True).exists()
+
+    @classmethod
+    def get_credit_course(cls, course_key):
+        """ Get the credit course if exists for the given course_key
+
+        Args:
+            course_key(CourseKey): The course identifier
+
+        Raises:
+            DoesNotExist if no CreditCourse exists for the given course key.
+
+        Returns:
+            CreditCourse if one exists for the given course key.
+        """
+        return cls.objects.get(course_key=course_key, enabled=True)
+
 
 class CreditProvider(TimeStampedModel):
     """This model represents an institution that can grant credit for a course.
@@ -37,21 +66,73 @@ class CreditRequirement(TimeStampedModel):
     """This model represents a credit requirement.
 
     Each requirement is uniquely identified by a `namespace` and a `name`. CreditRequirements
-    also include a `configuration` dictionary, the format of which varies by the type of requirement.
-    The configuration dictionary provides additional information clients may need to determine
+    also include a `criteria` dictionary, the format of which varies by the type of requirement.
+    The criteria dictionary provides additional information clients may need to determine
     whether a user has satisfied the requirement.
     """
 
     course = models.ForeignKey(CreditCourse, related_name="credit_requirements")
     namespace = models.CharField(max_length=255)
     name = models.CharField(max_length=255)
-    configuration = JSONField()
+    criteria = JSONField()
     active = models.BooleanField(default=True)
 
     class Meta(object):
         """Model metadata"""
         unique_together = ('namespace', 'name', 'course')
 
+    @classmethod
+    def add_or_update_course_requirement(cls, credit_course, requirement):
+        """ Add requirement to a given course
+        Args:
+            credit_course(CreditCourse): The identifier for credit course course
+            requirement(dict): requirement dict to be added
+
+        Returns:
+            (CreditRequirement, created) tuple
+        """
+
+        credit_requirement, created = cls.objects.get_or_create(
+            course=credit_course,
+            namespace=requirement["namespace"],
+            name=requirement["name"],
+            defaults={"criteria": requirement["criteria"], "active": True}
+        )
+        if not created:
+            credit_requirement.criteria = requirement["criteria"]
+            credit_requirement.active = True
+            credit_requirement.save()
+
+        return credit_requirement, created
+
+    @classmethod
+    def get_course_requirements(cls, course_key, namespace=None):
+        """ Get credit requirements of a given course
+
+        Args:
+            course_key(CourseKey): The identifier for a course
+            namespace(str): namespace of credit course requirements
+
+        Returns:
+            QuerySet of CreditRequirement model
+        """
+        requirements = CreditRequirement.objects.filter(course__course_key=course_key, active=True)
+        if namespace:
+            requirements = requirements.filter(namespace=namespace)
+        return requirements
+
+    @classmethod
+    def disable_credit_requirements(cls, requirement_ids):
+        """ Mark the given requirements inactive
+
+        Args:
+            requirement_ids(list): List of ids
+
+        Returns:
+            None
+        """
+        cls.objects.filter(id__in=requirement_ids).update(active=False)
+
 
 class CreditRequirementStatus(TimeStampedModel):
     """This model represents the status of each requirement."""

openedx/core/djangoapps/credit/signals.py

+"""This file contains receivers of course publication signals."""
+
+from django.dispatch import receiver
+
+from xmodule.modulestore.django import SignalHandler
+
+
+@receiver(SignalHandler.course_published)
+def listen_for_course_publish(sender, course_key, **kwargs):  # pylint: disable=unused-argument
+    """
+    Receives signal and kicks off celery task to update the course requirements
+    """
+
+    # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded
+    from .tasks import update_course_requirements
+
+    update_course_requirements.delay(unicode(course_key))

openedx/core/djangoapps/credit/tasks.py

+""" This file contains celery tasks for credit course views """
+
+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 .api import set_credit_requirements
+from openedx.core.djangoapps.credit.exceptions import InvalidCreditRequirements
+from openedx.core.djangoapps.credit.models import CreditCourse
+from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.exceptions import ItemNotFoundError
+
+
+LOGGER = get_task_logger(__name__)
+
+
+# pylint: disable=not-callable
+@task(default_retry_delay=settings.CREDIT_TASK_DEFAULT_RETRY_DELAY, max_retries=settings.CREDIT_TASK_MAX_RETRIES)
+def update_course_requirements(course_id):
+    """ Updates course requirements table for a course.
+
+     Args:
+        course_id(str): A string representation of course identifier
+
+    Returns:
+        None
+    """
+    try:
+        course_key = CourseKey.from_string(course_id)
+        is_credit_course = CreditCourse.is_credit_course(course_key)
+        if is_credit_course:
+            course = modulestore().get_course(course_key)
+            requirements = [
+                {
+                    "namespace": "grade",
+                    "name": "grade",
+                    "criteria": {
+                        "min_grade": get_min_grade_for_credit(course)
+                    }
+                }
+            ]
+            set_credit_requirements(course_key, requirements)
+    except (InvalidKeyError, ItemNotFoundError, InvalidCreditRequirements) as exc:
+        LOGGER.error('Error on adding the requirements for course %s - %s', course_id, unicode(exc))
+        raise update_course_requirements.retry(args=[course_id], exc=exc)
+    else:
+        LOGGER.info('Requirements added for course %s', course_id)
+
+
+def get_min_grade_for_credit(course):
+    """ Returns the min_grade for the credit requirements """
+    return getattr(course, "min_grade", 0.8)

openedx/core/djangoapps/credit/tests/test_api.py

+""" Tests for credit course api """
+import ddt
+
+from opaque_keys.edx.keys import CourseKey
+from openedx.core.djangoapps.credit.api import (
+    get_credit_requirements, set_credit_requirements, _get_requirements_to_disable
+)
+from openedx.core.djangoapps.credit.exceptions import InvalidCreditRequirements, InvalidCreditCourse
+from openedx.core.djangoapps.credit.models import CreditCourse, CreditRequirement
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+
+
+@ddt.ddt
+class ApiTestCases(ModuleStoreTestCase):
+    """ Tests for credit course api """
+
+    def setUp(self, **kwargs):
+        super(ApiTestCases, self).setUp()
+        self.course_key = CourseKey.from_string("edX/DemoX/Demo_Course")
+
+    @ddt.data(
+        [
+            {
+                "namespace": "grade",
+                "criteria": {
+                    "min_grade": 0.8
+                }
+            }
+        ],
+        [
+            {
+                "name": "grade",
+                "criteria": {
+                    "min_grade": 0.8
+                }
+            }
+        ],
+        [
+            {
+                "namespace": "grade",
+                "name": "grade",
+            }
+        ]
+    )
+    def test_set_credit_requirements_invalid_requirements(self, requirements):
+        self.add_credit_course()
+        with self.assertRaises(InvalidCreditRequirements):
+            set_credit_requirements(self.course_key, requirements)
+
+    def test_set_credit_requirements_invalid_course(self):
+        requirements = [
+            {
+                "namespace": "grade",
+                "name": "grade",
+                "criteria": {}
+            }
+        ]
+        with self.assertRaises(InvalidCreditCourse):
+            set_credit_requirements(self.course_key, requirements)
+        self.add_credit_course(enabled=False)
+        with self.assertRaises(InvalidCreditCourse):
+            set_credit_requirements(self.course_key, requirements)
+
+    def test_set_get_credit_requirements(self):
+        self.add_credit_course()
+        requirements = [
+            {
+                "namespace": "grade",
+                "name": "grade",
+                "criteria": {
+                    "min_grade": 0.8
+                }
+            },
+            {
+                "namespace": "grade",
+                "name": "grade",
+                "criteria": {
+                    "min_grade": 0.8
+                }
+            }
+        ]
+        set_credit_requirements(self.course_key, requirements)
+        self.assertEqual(len(get_credit_requirements(self.course_key)), 1)
+
+    def test_disable_credit_requirements(self):
+        self.add_credit_course()
+        requirements = [
+            {
+                "namespace": "grade",
+                "name": "grade",
+                "criteria": {
+                    "min_grade": 0.8
+                }
+            },
+            {
+                "namespace": "grade",
+                "name": "grade",
+                "criteria": {
+                    "min_grade": 0.8
+                }
+            }
+        ]
+        set_credit_requirements(self.course_key, requirements)
+        self.assertEqual(len(get_credit_requirements(self.course_key)), 1)
+
+        requirements = [
+            {
+                "namespace": "reverification",
+                "name": "midterm",
+                "criteria": {}
+            }
+        ]
+        set_credit_requirements(self.course_key, requirements)
+        self.assertEqual(len(get_credit_requirements(self.course_key)), 1)
+        grade_req = CreditRequirement.objects.filter(namespace="grade", name="grade")
+        self.assertEqual(len(grade_req), 1)
+        self.assertEqual(grade_req[0].active, False)
+
+    def test_requirements_to_disable(self):
+        self.add_credit_course()
+        requirements = [
+            {
+                "namespace": "grade",
+                "name": "grade",
+                "criteria": {
+                    "min_grade": 0.8
+                }
+            },
+            {
+                "namespace": "grade",
+                "name": "grade",
+                "criteria": {
+                    "min_grade": 0.8
+                }
+            }
+        ]
+
+        set_credit_requirements(self.course_key, requirements)
+        old_requirements = CreditRequirement.get_course_requirements(self.course_key)
+        self.assertEqual(len(old_requirements), 1)
+
+        requirements = [
+            {
+                "namespace": "reverification",
+                "name": "midterm",
+                "criteria": {}
+            }
+        ]
+        requirements_to_disabled = _get_requirements_to_disable(old_requirements, requirements)
+        self.assertEqual(len(requirements_to_disabled), 1)
+        self.assertEqual(requirements_to_disabled[0], old_requirements[0].id)
+
+        requirements = [
+            {
+                "namespace": "grade",
+                "name": "grade",
+                "criteria": {
+                    "min_grade": 0.8
+                }
+            },
+            {
+                "namespace": "reverification",
+                "name": "midterm",
+                "criteria": {}
+            }
+        ]
+        requirements_to_disabled = _get_requirements_to_disable(old_requirements, requirements)
+        self.assertEqual(len(requirements_to_disabled), 0)
+
+    def add_credit_course(self, enabled=True):
+        """ Mark the course as a credit """
+
+        credit_course = CreditCourse(course_key=self.course_key, enabled=enabled)
+        credit_course.save()
+        return credit_course

openedx/core/djangoapps/credit/tests/test_models.py

+""" Tests for credit course models """
+
+import ddt
+
+from opaque_keys.edx.keys import CourseKey
+from openedx.core.djangoapps.credit.models import CreditCourse, CreditRequirement
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+
+
+@ddt.ddt
+class ModelTestCases(ModuleStoreTestCase):
+    """ Tests for credit course models """
+
+    def setUp(self, **kwargs):
+        super(ModelTestCases, self).setUp()
+        self.course_key = CourseKey.from_string("edX/DemoX/Demo_Course")
+
+    @ddt.data(False, True)
+    def test_is_credit_course(self, is_credit):
+        CreditCourse(course_key=self.course_key, enabled=is_credit).save()
+        if is_credit:
+            self.assertTrue(CreditCourse.is_credit_course(self.course_key))
+        else:
+            self.assertFalse(CreditCourse.is_credit_course(self.course_key))
+
+    def test_get_course_requirements(self):
+        credit_course = self.add_credit_course()
+        requirement = {
+            "namespace": "grade",
+            "name": "grade",
+            "criteria": {
+                "min_grade": 0.8
+            }
+        }
+        credit_req, created = CreditRequirement.add_or_update_course_requirement(credit_course, requirement)
+        self.assertEqual(credit_course, credit_req.course)
+        self.assertEqual(created, True)
+        requirements = CreditRequirement.get_course_requirements(self.course_key)
+        self.assertEqual(len(requirements), 1)
+
+    def test_add_course_requirement_namespace(self):
+        credit_course = self.add_credit_course()
+        requirement = {
+            "namespace": "grade",
+            "name": "grade",
+            "criteria": {
+                "min_grade": 0.8
+            }
+        }
+        credit_req, created = CreditRequirement.add_or_update_course_requirement(credit_course, requirement)
+        self.assertEqual(credit_course, credit_req.course)
+        self.assertEqual(created, True)
+
+        requirement = {
+            "namespace": "icrv",
+            "name": "midterm",
+            "criteria": ""
+        }
+        credit_req, created = CreditRequirement.add_or_update_course_requirement(credit_course, requirement)
+        self.assertEqual(credit_course, credit_req.course)
+        self.assertEqual(created, True)
+
+        requirements = CreditRequirement.get_course_requirements(self.course_key)
+        self.assertEqual(len(requirements), 2)
+        requirements = CreditRequirement.get_course_requirements(self.course_key, namespace="grade")
+        self.assertEqual(len(requirements), 1)
+
+    def add_credit_course(self):
+        """ Add the course as a credit
+
+        Returns:
+            CreditCourse object
+        """
+        credit_course = CreditCourse(course_key=self.course_key, enabled=True)
+        credit_course.save()
+        return credit_course

openedx/core/djangoapps/credit/tests/test_tasks.py

+""" Tests for credit course tasks """
+
+import mock
+from datetime import datetime
+
+from openedx.core.djangoapps.credit.api import get_credit_requirements
+from openedx.core.djangoapps.credit.exceptions import InvalidCreditRequirements
+from openedx.core.djangoapps.credit.models import CreditCourse
+from openedx.core.djangoapps.credit.signals import listen_for_course_publish
+from xmodule.modulestore.django import SignalHandler
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+
+
+class TestTaskExecution(ModuleStoreTestCase):
+    """
+    Set of tests to ensure that the task code will do the right thing when
+    executed directly. The test course gets created without the listeners
+    being present, which allows us to ensure that when the listener is
+    executed, it is done as expected.
+    """
+
+    def mocked_set_credit_requirements(course_key, requirements):  # pylint: disable=no-self-argument, unused-argument
+        """
+        Used as a side effect when mocking `verify_student.ssencrypt.has_valid_signature`.
+        """
+        raise InvalidCreditRequirements
+
+    def setUp(self):
+        super(TestTaskExecution, self).setUp()
+
+        SignalHandler.course_published.disconnect(listen_for_course_publish)
+        self.course = CourseFactory.create(start=datetime(2015, 3, 1))
+
+    def test_task_adding_requirements_invalid_course(self):
+        """
+        Make sure that the receiver correctly fires off the task when
+        invoked by signal
+        """
+        requirements = get_credit_requirements(self.course.id)
+        self.assertEqual(len(requirements), 0)
+        listen_for_course_publish(self, self.course.id)
+
+        requirements = get_credit_requirements(self.course.id)
+        self.assertEqual(len(requirements), 0)
+
+    def test_task_adding_requirements(self):
+        """
+        Make sure that the receiver correctly fires off the task when
+        invoked by signal
+        """
+        self.add_credit_course(self.course.id)
+        requirements = get_credit_requirements(self.course.id)
+        self.assertEqual(len(requirements), 0)
+        listen_for_course_publish(self, self.course.id)
+
+        requirements = get_credit_requirements(self.course.id)
+        self.assertEqual(len(requirements), 1)
+
+    @mock.patch(
+        'openedx.core.djangoapps.credit.tasks.set_credit_requirements',
+        mock.Mock(
+            side_effect=mocked_set_credit_requirements
+        )
+    )
+    def test_retry(self):
+        """
+        Make sure that the receiver correctly fires off the task when
+        invoked by signal
+        """
+        self.add_credit_course(self.course.id)
+        requirements = get_credit_requirements(self.course.id)
+        self.assertEqual(len(requirements), 0)
+        listen_for_course_publish(self, self.course.id)
+
+        requirements = get_credit_requirements(self.course.id)
+        self.assertEqual(len(requirements), 0)
+
+    def add_credit_course(self, course_key):
+        """ Add the course as a credit
+
+        Args:
+            course_key(CourseKey): identifier for the course
+
+        Returns:
+            CreditCourse object added
+        """
+        credit_course = CreditCourse(course_key=course_key, enabled=True)
+        credit_course.save()
+        return credit_course