ECOM-1911: Fixes to credit requirements in Studio.
* Exclude orphaned XBlocks from requirements because they have been deleted. * Sort ICRV requirements by start date, then by display name.
This commit is contained in:
committed by
Clinton Blackburn
parent
9e00e2b397
commit
65367a2ded
@@ -286,6 +286,7 @@ class CreditRequirement(TimeStampedModel):
|
||||
Model metadata.
|
||||
"""
|
||||
unique_together = ('namespace', 'name', 'course')
|
||||
ordering = ["order"]
|
||||
|
||||
@classmethod
|
||||
def add_or_update_course_requirement(cls, credit_course, requirement, order):
|
||||
@@ -337,7 +338,7 @@ class CreditRequirement(TimeStampedModel):
|
||||
|
||||
"""
|
||||
# order credit requirements according to their appearance in courseware
|
||||
requirements = CreditRequirement.objects.filter(course__course_key=course_key, active=True).order_by("-order")
|
||||
requirements = CreditRequirement.objects.filter(course__course_key=course_key, active=True)
|
||||
|
||||
if namespace is not None:
|
||||
requirements = requirements.filter(namespace=namespace)
|
||||
|
||||
@@ -2,6 +2,9 @@
|
||||
This file contains celery tasks for credit course views.
|
||||
"""
|
||||
|
||||
import datetime
|
||||
from pytz import UTC
|
||||
|
||||
from django.conf import settings
|
||||
|
||||
from celery import task
|
||||
@@ -13,17 +16,15 @@ 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 import ModuleStoreEnum
|
||||
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"
|
||||
}
|
||||
CREDIT_REQUIREMENT_XBLOCK_CATEGORIES = [
|
||||
"edx-reverification-block",
|
||||
]
|
||||
|
||||
|
||||
@@ -123,24 +124,62 @@ def _get_credit_course_requirement_xblocks(course_key): # pylint: disable=inval
|
||||
# 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:
|
||||
for category in CREDIT_REQUIREMENT_XBLOCK_CATEGORIES:
|
||||
requirements.extend([
|
||||
{
|
||||
"namespace": desc["requirement-namespace"],
|
||||
"namespace": block.get_credit_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"]}
|
||||
)
|
||||
for block in _get_xblocks(course_key, category)
|
||||
if _is_credit_requirement(block)
|
||||
])
|
||||
|
||||
return requirements
|
||||
|
||||
|
||||
def _is_in_course_tree(block):
|
||||
"""
|
||||
Check that the XBlock is in the course tree.
|
||||
|
||||
It's possible that the XBlock is not in the course tree
|
||||
if its parent has been deleted and is now an orphan.
|
||||
"""
|
||||
ancestor = block.get_parent()
|
||||
while ancestor is not None and ancestor.location.category != "course":
|
||||
ancestor = ancestor.get_parent()
|
||||
|
||||
return ancestor is not None
|
||||
|
||||
|
||||
def _get_xblocks(course_key, category):
|
||||
"""
|
||||
Retrieve all XBlocks in the course for a particular category.
|
||||
|
||||
Returns only XBlocks that are published and haven't been deleted.
|
||||
"""
|
||||
xblocks = [
|
||||
block for block in modulestore().get_items(
|
||||
course_key,
|
||||
qualifiers={"category": category},
|
||||
revision=ModuleStoreEnum.RevisionOption.published_only,
|
||||
)
|
||||
if _is_in_course_tree(block)
|
||||
]
|
||||
|
||||
# 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
|
||||
|
||||
|
||||
def _is_credit_requirement(xblock):
|
||||
"""
|
||||
Check if the given XBlock is a credit requirement.
|
||||
@@ -152,19 +191,19 @@ def _is_credit_requirement(xblock):
|
||||
True if XBlock is a credit requirement else 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
|
||||
required_methods = [
|
||||
"get_credit_requirement_namespace",
|
||||
"get_credit_requirement_name",
|
||||
"get_credit_requirement_display_name"
|
||||
]
|
||||
|
||||
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
|
||||
for method_name in required_methods:
|
||||
if not callable(getattr(xblock, method_name, None)):
|
||||
LOGGER.error(
|
||||
"XBlock %s is marked as a credit requirement but does not "
|
||||
"implement %s", unicode(xblock), method_name
|
||||
)
|
||||
return False
|
||||
|
||||
return True
|
||||
|
||||
|
||||
@@ -3,14 +3,16 @@ Tests for credit course tasks.
|
||||
"""
|
||||
|
||||
import mock
|
||||
from datetime import datetime
|
||||
from datetime import datetime, timedelta
|
||||
|
||||
from pytz import UTC
|
||||
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 on_course_publish
|
||||
from xmodule.modulestore import ModuleStoreEnum
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
|
||||
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls_range
|
||||
|
||||
from edx_proctoring.api import create_exam
|
||||
|
||||
@@ -30,22 +32,32 @@ class TestTaskExecution(ModuleStoreTestCase):
|
||||
"""
|
||||
raise InvalidCreditRequirements
|
||||
|
||||
def add_icrv_xblock(self):
|
||||
def add_icrv_xblock(self, related_assessment_name=None, start_date=None):
|
||||
""" Create the 'edx-reverification-block' in course tree """
|
||||
|
||||
section = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section')
|
||||
subsection = ItemFactory.create(parent=section, category='sequential', display_name='Test Subsection')
|
||||
vertical = ItemFactory.create(parent=subsection, category='vertical', display_name='Test Unit')
|
||||
ItemFactory.create(
|
||||
parent=vertical,
|
||||
block = ItemFactory.create(
|
||||
parent=self.vertical,
|
||||
category='edx-reverification-block',
|
||||
display_name='Test Verification Block'
|
||||
)
|
||||
|
||||
if related_assessment_name is not None:
|
||||
block.related_assessment = related_assessment_name
|
||||
|
||||
block.start = start_date
|
||||
|
||||
self.store.update_item(block, ModuleStoreEnum.UserID.test)
|
||||
|
||||
with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id):
|
||||
self.store.publish(block.location, ModuleStoreEnum.UserID.test)
|
||||
|
||||
return block
|
||||
|
||||
def setUp(self):
|
||||
super(TestTaskExecution, self).setUp()
|
||||
|
||||
self.course = CourseFactory.create(start=datetime(2015, 3, 1))
|
||||
self.section = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section')
|
||||
self.subsection = ItemFactory.create(parent=self.section, category='sequential', display_name='Test Subsection')
|
||||
self.vertical = ItemFactory.create(parent=self.subsection, category='vertical', display_name='Test Unit')
|
||||
|
||||
def test_task_adding_requirements_invalid_course(self):
|
||||
"""
|
||||
@@ -172,9 +184,65 @@ class TestTaskExecution(ModuleStoreTestCase):
|
||||
self.add_credit_course(self.course.id)
|
||||
self.add_icrv_xblock()
|
||||
|
||||
with check_mongo_calls(3):
|
||||
with check_mongo_calls_range(max_finds=7):
|
||||
on_course_publish(self.course.id)
|
||||
|
||||
def test_remove_icrv_requirement(self):
|
||||
self.add_credit_course(self.course.id)
|
||||
self.add_icrv_xblock()
|
||||
on_course_publish(self.course.id)
|
||||
|
||||
# There should be one ICRV requirement
|
||||
requirements = get_credit_requirements(self.course.id, namespace="reverification")
|
||||
self.assertEqual(len(requirements), 1)
|
||||
|
||||
# Delete the parent section containing the ICRV block
|
||||
with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id):
|
||||
self.store.delete_item(self.subsection.location, ModuleStoreEnum.UserID.test)
|
||||
|
||||
# Check that the ICRV block is no longer visible in the requirements
|
||||
on_course_publish(self.course.id)
|
||||
requirements = get_credit_requirements(self.course.id, namespace="reverification")
|
||||
self.assertEqual(len(requirements), 0)
|
||||
|
||||
def test_icrv_requirement_ordering(self):
|
||||
self.add_credit_course(self.course.id)
|
||||
|
||||
# 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, namespace="reverification")
|
||||
self.assertEqual(len(requirements), 2)
|
||||
self.assertEqual(requirements[0]["display_name"], "Midterm B")
|
||||
self.assertEqual(requirements[1]["display_name"], "Midterm A")
|
||||
|
||||
# Add two additional ICRV blocks that have no start date
|
||||
# and the same name.
|
||||
start = datetime.now(UTC)
|
||||
first_block = self.add_icrv_xblock(related_assessment_name="Midterm Start Date")
|
||||
|
||||
start = start + timedelta(days=1)
|
||||
second_block = self.add_icrv_xblock(related_assessment_name="Midterm Start Date")
|
||||
|
||||
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 the first 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())
|
||||
|
||||
@mock.patch(
|
||||
'openedx.core.djangoapps.credit.tasks.set_credit_requirements',
|
||||
mock.Mock(
|
||||
|
||||
Reference in New Issue
Block a user