diff --git a/openedx/core/djangoapps/credit/signals.py b/openedx/core/djangoapps/credit/signals.py index 6c7886857a..4480f10bed 100644 --- a/openedx/core/djangoapps/credit/signals.py +++ b/openedx/core/djangoapps/credit/signals.py @@ -2,6 +2,8 @@ This file contains receivers of course publication signals. """ +import logging + from django.dispatch import receiver from django.utils import timezone from opaque_keys.edx.keys import CourseKey @@ -10,6 +12,9 @@ from xmodule.modulestore.django import SignalHandler from openedx.core.djangoapps.signals.signals import GRADES_UPDATED +log = logging.getLogger(__name__) + + @receiver(SignalHandler.course_published) def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument """Receive 'course_published' signal and kick off a celery task to update @@ -18,9 +23,11 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable= # Import here, because signal is registered at startup, but items in tasks # are not yet able to be loaded - from .tasks import update_credit_course_requirements + from openedx.core.djangoapps.credit import api, tasks - update_credit_course_requirements.delay(unicode(course_key)) + if api.is_credit_course(course_key): + tasks.update_credit_course_requirements.delay(unicode(course_key)) + log.info(u'Added task to update credit requirements for course "%s" to the task queue', course_key) @receiver(GRADES_UPDATED) diff --git a/openedx/core/djangoapps/credit/tasks.py b/openedx/core/djangoapps/credit/tasks.py index e23af25731..82efad96b0 100644 --- a/openedx/core/djangoapps/credit/tasks.py +++ b/openedx/core/djangoapps/credit/tasks.py @@ -18,10 +18,20 @@ from xmodule.modulestore.exceptions import ItemNotFoundError LOGGER = get_task_logger(__name__) +# XBlocks that can be added as credit requirements +CREDIT_REQUIREMENT_XBLOCKS = [ + { + "category": "edx-reverification-block", + "requirement-namespace": "reverification" + } +] + + # pylint: disable=not-callable @task(default_retry_delay=settings.CREDIT_TASK_DEFAULT_RETRY_DELAY, max_retries=settings.CREDIT_TASK_MAX_RETRIES) def update_credit_course_requirements(course_id): # pylint: disable=invalid-name - """Updates course requirements table for a course. + """ + Updates course requirements table for a course. Args: course_id(str): A string representation of course identifier @@ -34,8 +44,7 @@ def update_credit_course_requirements(course_id): # pylint: disable=invalid-na 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 = _get_course_credit_requirements(course) + requirements = _get_course_credit_requirements(course_key) 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)) @@ -44,41 +53,40 @@ def update_credit_course_requirements(course_id): # pylint: disable=invalid-na LOGGER.info('Requirements added for course %s', course_id) -def _get_course_credit_requirements(course): - """Returns the list of credit requirements for the given course. +def _get_course_credit_requirements(course_key): + """ + Returns the list of credit requirements for the given course. It returns the minimum_grade_credit and also the ICRV checkpoints if any were added in the course Args: - course(Course): The course object + course_key (CourseKey): Identifier for the course. Returns: - List of minimum_grade_credit and ICRV requirements + List of credit requirements (dictionaries) """ - credit_xblock_requirements = _get_credit_course_requirement_xblocks(course) - min_grade_requirement = _get_min_grade_requirement(course) + credit_xblock_requirements = _get_credit_course_requirement_xblocks(course_key) + min_grade_requirement = _get_min_grade_requirement(course_key) credit_requirements = min_grade_requirement + credit_xblock_requirements return credit_requirements -def _get_min_grade_requirement(course): - """Get list of 'minimum_grade_credit' requirement for the given course. +def _get_min_grade_requirement(course_key): + """ + Get list of 'minimum_grade_credit' requirement for the given course. Args: - course(Course): The course object - - Raises: - AttributeError if the course has not 'minimum_grade_credit' attribute + course_key (CourseKey): Identifier for the course. Returns: The list of minimum_grade_credit requirements """ - requirement = [] + course = modulestore().get_course(course_key, depth=0) try: - requirement = [ + return [ { "namespace": "grade", "name": "grade", @@ -90,41 +98,46 @@ def _get_min_grade_requirement(course): ] except AttributeError: LOGGER.error("The course %s does not has minimum_grade_credit attribute", unicode(course.id)) - return requirement + else: + return [] -def _get_credit_course_requirement_xblocks(course): # pylint: disable=invalid-name +def _get_credit_course_requirement_xblocks(course_key): # pylint: disable=invalid-name """Generate a course structure dictionary for the specified course. Args: - course(Course): The course object + course_key (CourseKey): Identifier for the course. Returns: The list of credit requirements xblocks dicts """ - blocks_stack = [course] - requirements_blocks = [] - while blocks_stack: - curr_block = blocks_stack.pop() - children = curr_block.get_children() if curr_block.has_children else [] - if _is_credit_requirement(curr_block): - block = { - "namespace": curr_block.get_credit_requirement_namespace(), - "name": curr_block.get_credit_requirement_name(), - "display_name": curr_block.get_credit_requirement_display_name(), - "criteria": "", - } - requirements_blocks.append(block) + requirements = [] - # Add the children of current block to the stack so that we can - # traverse them as well. - blocks_stack.extend(children) - return requirements_blocks + # Retrieve all XBlocks from the course that we know to be credit requirements. + # For performance reasons, we look these up by their "category" to avoid + # loading and searching the entire course tree. + for desc in CREDIT_REQUIREMENT_XBLOCKS: + requirements.extend([ + { + "namespace": desc["requirement-namespace"], + "name": block.get_credit_requirement_name(), + "display_name": block.get_credit_requirement_display_name(), + "criteria": {}, + } + for block in modulestore().get_items( + course_key, + qualifiers={"category": desc["category"]} + ) + if _is_credit_requirement(block) + ]) + + return requirements def _is_credit_requirement(xblock): - """Check if the given XBlock is a credit requirement. + """ + Check if the given XBlock is a credit requirement. Args: xblock(XBlock): The given XBlock object @@ -133,21 +146,18 @@ def _is_credit_requirement(xblock): True if XBlock is a credit requirement else False """ - is_credit_requirement = False + if not callable(getattr(xblock, "get_credit_requirement_namespace", None)): + LOGGER.error( + "XBlock %s is marked as a credit requirement but does not " + "implement get_credit_requirement_namespace()", unicode(xblock) + ) + return False - if callable(getattr(xblock, "is_course_credit_requirement", None)): - is_credit_requirement = xblock.is_course_credit_requirement() - if is_credit_requirement: - if not callable(getattr(xblock, "get_credit_requirement_namespace", None)): - is_credit_requirement = False - LOGGER.error( - "XBlock %s is marked as a credit requirement but does not " - "implement get_credit_requirement_namespace()", unicode(xblock) - ) - if not callable(getattr(xblock, "get_credit_requirement_name", None)): - is_credit_requirement = False - LOGGER.error( - "XBlock %s is marked as a credit requirement but does not " - "implement get_credit_requirement_name()", unicode(xblock) - ) - return is_credit_requirement + if not callable(getattr(xblock, "get_credit_requirement_name", None)): + LOGGER.error( + "XBlock %s is marked as a credit requirement but does not " + "implement get_credit_requirement_name()", unicode(xblock) + ) + return False + + return True diff --git a/openedx/core/djangoapps/credit/tests/test_tasks.py b/openedx/core/djangoapps/credit/tests/test_tasks.py index cef3605920..2ddcabfbd8 100644 --- a/openedx/core/djangoapps/credit/tests/test_tasks.py +++ b/openedx/core/djangoapps/credit/tests/test_tasks.py @@ -11,7 +11,7 @@ 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, ItemFactory +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls class TestTaskExecution(ModuleStoreTestCase): @@ -85,6 +85,13 @@ class TestTaskExecution(ModuleStoreTestCase): requirements = get_credit_requirements(self.course.id) self.assertEqual(len(requirements), 2) + def test_query_counts(self): + self.add_credit_course(self.course.id) + self.add_icrv_xblock() + + with check_mongo_calls(3): + listen_for_course_publish(self, self.course.id) + @mock.patch( 'openedx.core.djangoapps.credit.tasks.set_credit_requirements', mock.Mock(