diff --git a/cms/envs/common.py b/cms/envs/common.py index 63497dc6fd..39a3008c30 100644 --- a/cms/envs/common.py +++ b/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 diff --git a/lms/envs/common.py b/lms/envs/common.py index 01492633f0..ecb8c94bcb 100644 --- a/lms/envs/common.py +++ b/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 diff --git a/openedx/core/djangoapps/credit/__init__.py b/openedx/core/djangoapps/credit/__init__.py index e69de29bb2..e5ef8a0674 100644 --- a/openedx/core/djangoapps/credit/__init__.py +++ b/openedx/core/djangoapps/credit/__init__.py @@ -0,0 +1,3 @@ +""" Register signal handlers """ + +from . import signals diff --git a/openedx/core/djangoapps/credit/api.py b/openedx/core/djangoapps/credit/api.py new file mode 100644 index 0000000000..f29ede92b2 --- /dev/null +++ b/openedx/core/djangoapps/credit/api.py @@ -0,0 +1,165 @@ +""" 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 diff --git a/openedx/core/djangoapps/credit/exceptions.py b/openedx/core/djangoapps/credit/exceptions.py new file mode 100644 index 0000000000..c7cba03f42 --- /dev/null +++ b/openedx/core/djangoapps/credit/exceptions.py @@ -0,0 +1,11 @@ +""" 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 diff --git a/openedx/core/djangoapps/credit/models.py b/openedx/core/djangoapps/credit/models.py index 8da5ec03f8..d3b7403603 100644 --- a/openedx/core/djangoapps/credit/models.py +++ b/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.""" diff --git a/openedx/core/djangoapps/credit/signals.py b/openedx/core/djangoapps/credit/signals.py new file mode 100644 index 0000000000..9cd9778c6b --- /dev/null +++ b/openedx/core/djangoapps/credit/signals.py @@ -0,0 +1,17 @@ +"""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)) diff --git a/openedx/core/djangoapps/credit/tasks.py b/openedx/core/djangoapps/credit/tasks.py new file mode 100644 index 0000000000..d5e5a5c634 --- /dev/null +++ b/openedx/core/djangoapps/credit/tasks.py @@ -0,0 +1,55 @@ +""" 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) diff --git a/openedx/core/djangoapps/credit/tests/__init__.py b/openedx/core/djangoapps/credit/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/credit/tests/test_api.py b/openedx/core/djangoapps/credit/tests/test_api.py new file mode 100644 index 0000000000..dadd5a4493 --- /dev/null +++ b/openedx/core/djangoapps/credit/tests/test_api.py @@ -0,0 +1,175 @@ +""" 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 diff --git a/openedx/core/djangoapps/credit/tests/test_models.py b/openedx/core/djangoapps/credit/tests/test_models.py new file mode 100644 index 0000000000..fccdc05566 --- /dev/null +++ b/openedx/core/djangoapps/credit/tests/test_models.py @@ -0,0 +1,76 @@ +""" 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 diff --git a/openedx/core/djangoapps/credit/tests/test_tasks.py b/openedx/core/djangoapps/credit/tests/test_tasks.py new file mode 100644 index 0000000000..f1b14d6f3f --- /dev/null +++ b/openedx/core/djangoapps/credit/tests/test_tasks.py @@ -0,0 +1,90 @@ +""" 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