Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: Add backfill course tabs management command

Browse Source 
Previously, course tabs would only be created once and never try to
update the default tabs again. This leads to an issue if you ever want
to add a new tab. With this command, you can now update the default tabs
for all existing courses and new courses will pick it up upon creation
when CourseTabList.initialize_default is called.
This commit is contained in:
Dillon Dumesnil
2022-03-09 12:27:18 -05:00
parent 50f348b395
commit 61b93d953d
13 changed files with 304 additions and 189 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
cms/djangoapps/contentstore/management/commands/backfill_course_tabs.py Normal file
View File

@@ -0,0 +1,53 @@
"""
Management command to backfill default course tabs for all courses. This command is essentially for
when a new default tab is added and we need to update all existing courses. Any new courses will pick
up the new tab automatically via course creation and the CourseTabList.initialize_default method.
People updating to Nutmeg release should run this command as part of the upgrade process.
This should be invoked from the Studio process.
"""
import logging
from django.core.management.base import BaseCommand
from xmodule.tabs import CourseTabList
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
logger = logging.getLogger(__name__)
class Command(BaseCommand):
"""
Invoke with:
python manage.py cms backfill_course_tabs
"""
help = (
'Backfill default course tabs for all courses. This command is essentially for when a new default '
'tab is added and we need to update all existing courses. Any new courses will pick up the new '
'tab automatically via course creation and the CourseTabList.initialize_default method.'
)
def handle(self, *args, **options):
"""
Gathers all course keys in the modulestore and updates the course tabs
if there are any new default course tabs. Else, makes no updates.
"""
store = modulestore()
course_keys = sorted(
(course.id for course in store.get_course_summaries()),
key=str # Different types of CourseKeys can't be compared without this.
)
logger.info(f'{len(course_keys)} courses read from modulestore.')
for course_key in course_keys:
course = store.get_course(course_key, depth=1)
existing_tabs = {tab.type for tab in course.tabs}
CourseTabList.initialize_default(course)
new_tabs = {tab.type for tab in course.tabs}
if existing_tabs != new_tabs:
# This will trigger the Course Published Signal which is necessary to update
# the corresponding Course Overview
logger.info(f'Updating tabs for {course_key}.')
store.update_item(course, ModuleStoreEnum.UserID.mgmt_command)

131
cms/djangoapps/contentstore/management/commands/tests/test_backfill_course_tabs.py Normal file
View File

@@ -0,0 +1,131 @@
"""
Tests for `backfill_course_outlines` Studio (cms) management command.
"""
from unittest import mock
from django.core.management import call_command
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.tabs import InvalidTabsException
class BackfillCourseTabsTest(ModuleStoreTestCase):
"""
Test `backfill_course_tabs`
"""
@mock.patch('cms.djangoapps.contentstore.management.commands.backfill_course_tabs.logger')
def test_no_tabs_to_add(self, mock_logger):
""" Calls command with a course already having all default tabs. """
course = CourseFactory()
tabs_before = course.tabs
call_command('backfill_course_tabs')
course = self.store.get_course(course.id)
tabs_after = course.tabs
assert tabs_before == tabs_after
# Ensure update_item was never called since there were no changes necessary
# Using logger as a proxy. First time is number of courses read.
assert mock_logger.info.call_count == 1
@mock.patch('cms.djangoapps.contentstore.management.commands.backfill_course_tabs.logger')
def test_add_one_tab(self, mock_logger):
"""
Calls command on a course with existing tabs, but not all default ones.
"""
course = CourseFactory()
course.tabs = [tab for tab in course.tabs if tab.type != 'dates']
self.update_course(course, ModuleStoreEnum.UserID.test)
assert len(course.tabs) == 6
assert 'dates' not in {tab.type for tab in course.tabs}
call_command('backfill_course_tabs')
course = self.store.get_course(course.id)
assert len(course.tabs) == 7
assert 'dates' in {tab.type for tab in course.tabs}
mock_logger.info.assert_called_with(f'Updating tabs for {course.id}.')
assert mock_logger.info.call_count == 2
call_command('backfill_course_tabs')
# Ensure rerunning the command does not require another update on the course.
# Goes up by one from the courses read log.
assert mock_logger.info.call_count == 3
@mock.patch('cms.djangoapps.contentstore.management.commands.backfill_course_tabs.logger')
def test_multiple_courses_one_update(self, mock_logger):
"""
Calls command on multiple courses, some that already have all their tabs, and some that need updates.
"""
CourseFactory()
CourseFactory()
CourseFactory()
course = CourseFactory()
course.tabs = [tab for tab in course.tabs if tab.type in ('course_info', 'courseware')]
self.update_course(course, ModuleStoreEnum.UserID.test)
assert len(course.tabs) == 2
assert 'dates' not in {tab.type for tab in course.tabs}
assert 'progress' not in {tab.type for tab in course.tabs}
call_command('backfill_course_tabs')
course = self.store.get_course(course.id)
assert len(course.tabs) == 7
assert 'dates' in {tab.type for tab in course.tabs}
assert 'progress' in {tab.type for tab in course.tabs}
mock_logger.info.assert_any_call('4 courses read from modulestore.')
mock_logger.info.assert_called_with(f'Updating tabs for {course.id}.')
assert mock_logger.info.call_count == 2
call_command('backfill_course_tabs')
# Ensure rerunning the command does not require another update on the course.
# Goes up by one from the courses read log.
assert mock_logger.info.call_count == 3
@mock.patch('cms.djangoapps.contentstore.management.commands.backfill_course_tabs.logger')
def test_multiple_courses_all_updated(self, mock_logger):
"""
Calls command on multiple courses where all of them need updates.
"""
course_1 = CourseFactory()
course_1.tabs = [tab for tab in course_1.tabs if tab.type != 'dates']
self.update_course(course_1, ModuleStoreEnum.UserID.test)
course_2 = CourseFactory()
course_2.tabs = [tab for tab in course_2.tabs if tab.type != 'progress']
self.update_course(course_2, ModuleStoreEnum.UserID.test)
assert len(course_1.tabs) == 6
assert len(course_2.tabs) == 6
assert 'dates' not in {tab.type for tab in course_1.tabs}
assert 'progress' not in {tab.type for tab in course_2.tabs}
call_command('backfill_course_tabs')
course_1 = self.store.get_course(course_1.id)
course_2 = self.store.get_course(course_2.id)
assert len(course_1.tabs) == 7
assert len(course_2.tabs) == 7
assert 'dates' in {tab.type for tab in course_1.tabs}
assert 'progress' in {tab.type for tab in course_2.tabs}
mock_logger.info.assert_any_call('2 courses read from modulestore.')
mock_logger.info.assert_any_call(f'Updating tabs for {course_1.id}.')
mock_logger.info.assert_any_call(f'Updating tabs for {course_2.id}.')
assert mock_logger.info.call_count == 3
call_command('backfill_course_tabs')
# Ensure rerunning the command does not require another update on any courses.
# Goes up by one from the courses read log.
assert mock_logger.info.call_count == 4
@mock.patch('cms.djangoapps.contentstore.management.commands.backfill_course_tabs.logger')
def test_command_fails_if_error_raised(self, mock_logger):
CourseFactory()
with mock.patch(
'cms.djangoapps.contentstore.management.commands.backfill_course_tabs.CourseTabList.initialize_default',
side_effect=InvalidTabsException
):
with self.assertRaises(InvalidTabsException):
call_command('backfill_course_tabs')
# Never calls the update, but does make it through grabbing the courses
mock_logger.info.assert_called_once_with('1 courses read from modulestore.')

9
cms/djangoapps/contentstore/tasks.py
View File

@@ -5,10 +5,10 @@ This file contains celery tasks for contentstore views
import base64
import json
import os
import shutil # lint-amnesty, pylint: disable=wrong-import-order
import tarfile # lint-amnesty, pylint: disable=wrong-import-order
from datetime import datetime # lint-amnesty, pylint: disable=wrong-import-order
from tempfile import NamedTemporaryFile, mkdtemp # lint-amnesty, pylint: disable=wrong-import-order
import shutil
import tarfile
from datetime import datetime
from tempfile import NamedTemporaryFile, mkdtemp
import olxcleaner
import pkg_resources
@@ -17,7 +17,6 @@ from celery import shared_task
from celery.utils.log import get_task_logger
from django.conf import settings
from django.contrib.auth import get_user_model
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
from django.core.exceptions import SuspiciousOperation
from django.core.files import File
from django.test import RequestFactory

7
cms/djangoapps/contentstore/views/tests/test_tabs.py
View File

@@ -193,11 +193,12 @@ class PrimitiveTabEdit(ModuleStoreTestCase):
tabs.primitive_delete(course, 1)
with self.assertRaises(IndexError):
tabs.primitive_delete(course, 7)
assert course.tabs[2] != {'type': 'discussion', 'name': 'Discussion'}
assert course.tabs[2] != {'type': 'dates', 'name': 'Dates'}
tabs.primitive_delete(course, 2)
assert {'type': 'progress'} not in course.tabs
# Check that discussion has shifted up
assert course.tabs[2] == {'type': 'discussion', 'name': 'Discussion'}
# Check that dates has shifted up
assert course.tabs[2] == {'type': 'dates', 'name': 'Dates'}
def test_insert(self):
"""Test primitive tab insertion."""

4
common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py
View File

@@ -30,7 +30,6 @@ from xmodule.modulestore.tests.utils import (
SPLIT_MODULESTORE_SETUP,
TEST_DATA_DIR,
MongoContentstoreBuilder,
mock_tab_from_json
)
from xmodule.modulestore.xml_exporter import export_course_to_xml
from xmodule.modulestore.xml_importer import import_course_from_xml
@@ -61,7 +60,6 @@ class CrossStoreXMLRoundtrip(CourseComparisonTest, PartitionTestCase):
self.addCleanup(rmtree, self.export_dir, ignore_errors=True)
@patch('xmodule.video_module.video_module.edxval_api', None)
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
@ddt.data(*itertools.product(
MODULESTORE_SETUPS,
MODULESTORE_SETUPS,
@@ -72,7 +70,7 @@ class CrossStoreXMLRoundtrip(CourseComparisonTest, PartitionTestCase):
@ddt.unpack
def test_round_trip(
self, source_builder, dest_builder, source_content_builder,
dest_content_builder, course_data_name, _mock_tab_from_json
dest_content_builder, course_data_name,
):
# Construct the contentstore for storing the first import
with source_content_builder.build() as source_content:

9
common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
View File

@@ -50,11 +50,7 @@ from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES
from xmodule.modulestore.tests.factories import check_exact_number_of_calls, check_mongo_calls, mongo_uses_error_check
from xmodule.modulestore.tests.mongo_connection import MONGO_HOST, MONGO_PORT_NUM
from xmodule.modulestore.tests.test_asides import AsideTestType
from xmodule.modulestore.tests.utils import (
MongoContentstoreBuilder,
create_modulestore_instance,
mock_tab_from_json
)
from xmodule.modulestore.tests.utils import MongoContentstoreBuilder, create_modulestore_instance
from xmodule.modulestore.xml_exporter import export_course_to_xml
from xmodule.modulestore.xml_importer import LocationMixin, import_course_from_xml
from xmodule.tests import DATA_DIR, CourseComparisonTest
@@ -2659,9 +2655,8 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
self.store.clone_course(course_key, dest_course_id, self.user_id)
signal_handler.send.assert_called_with('course_published', course_key=dest_course_id)
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_course_publish_signal_import_firing(self, default, _from_json):
def test_course_publish_signal_import_firing(self, default):
with MongoContentstoreBuilder().build() as contentstore:
signal_handler = Mock(name='signal_handler')
self.store = MixedModuleStore(

91
common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
View File

@@ -35,7 +35,6 @@ from xmodule.modulestore.inheritance import InheritanceMixin
from xmodule.modulestore.mongo import MongoKeyValueStore
from xmodule.modulestore.mongo.base import as_draft
from xmodule.modulestore.tests.mongo_connection import MONGO_HOST, MONGO_PORT_NUM
from xmodule.modulestore.tests.utils import mock_tab_from_json
from xmodule.modulestore.xml_exporter import export_course_to_xml
from xmodule.modulestore.xml_importer import LocationMixin, import_course_from_xml, perform_xlint
from xmodule.tests import DATA_DIR
@@ -131,37 +130,46 @@ class TestMongoModuleStoreBase(TestCase):
)
with patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json):
import_course_from_xml(
draft_store,
999,
DATA_DIR,
cls.courses,
static_content_store=content_store
)
import_course_from_xml(
draft_store,
999,
DATA_DIR,
cls.courses,
static_content_store=content_store
)
# also test a course with no importing of static content
import_course_from_xml(
draft_store,
999,
DATA_DIR,
['test_import_course'],
static_content_store=content_store,
do_import_static=False,
verbose=True
)
# also test a course with no importing of static content
import_course_from_xml(
draft_store,
999,
DATA_DIR,
['test_import_course'],
static_content_store=content_store,
do_import_static=False,
verbose=True
)
# also import a course under a different course_id (especially ORG)
import_course_from_xml(
draft_store,
999,
DATA_DIR,
['test_import_course'],
static_content_store=content_store,
do_import_static=False,
verbose=True,
target_id=CourseKey.from_string('guestx/foo/bar')
)
# also import a course under a different course_id (especially ORG)
import_course_from_xml(
draft_store,
999,
DATA_DIR,
['test_import_course'],
static_content_store=content_store,
do_import_static=False,
verbose=True,
target_id=CourseKey.from_string('guestx/foo/bar')
)
# Import a course for `test_reference_converters` since it manipulates the saved course
# which can cause any other test using the same course to have a flakey error
import_course_from_xml(
draft_store,
999,
DATA_DIR,
['test_import_course_2'],
static_content_store=content_store
)
return content_store, draft_store
@@ -206,12 +214,11 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
)
assert store.get_modulestore_type('') == ModuleStoreEnum.Type.mongo
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_get_courses(self, _from_json):
def test_get_courses(self):
'''Make sure the course objects loaded properly'''
courses = self.draft_store.get_courses()
assert len(courses) == 6
assert len(courses) == 7
course_ids = [course.id for course in courses]
for course_key in [
@@ -224,6 +231,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
['edX', 'test_unicode', '2012_Fall'],
['edX', 'toy', '2012_Fall'],
['guestx', 'foo', 'bar'],
['edX', 'test_import', '2014_Fall'],
]
]:
assert course_key in course_ids
@@ -236,8 +244,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
assert not self.draft_store.has_course(mix_cased)
assert self.draft_store.has_course(mix_cased, ignore_case=True)
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_get_org_courses(self, _from_json):
def test_get_org_courses(self):
"""
Make sure that we can query for a filtered list of courses for a given ORG
"""
@@ -255,7 +262,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
assert course_key in course_ids
courses = self.draft_store.get_courses(org='edX')
assert len(courses) == 5
assert len(courses) == 6
course_ids = [course.id for course in courses]
for course_key in [
@@ -266,6 +273,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
['edX', 'test_import_course', '2012_Fall'],
['edX', 'test_unicode', '2012_Fall'],
['edX', 'toy', '2012_Fall'],
['edX', 'test_import', '2014_Fall'],
]
]:
assert course_key in course_ids
@@ -431,8 +439,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
{'displayname': 'hello'}
)
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_get_courses_for_wiki(self, _from_json):
def test_get_courses_for_wiki(self):
"""
Test the get_courses_for_wiki method
"""
@@ -473,7 +480,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
"""
Test that references types get deserialized correctly
"""
course_key = CourseKey.from_string('edX/toy/2012_Fall')
course_key = CourseKey.from_string('edX/test_import/2014_Fall')
def setup_test():
course = self.draft_store.get_course(course_key)
@@ -548,8 +555,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
check_mongo_fields()
@patch('xmodule.video_module.video_module.edxval_api', None)
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_export_course_image(self, _from_json):
def test_export_course_image(self):
"""
Test to make sure that we have a course image in the contentstore,
then export it to ensure it gets copied to both file locations.
@@ -567,8 +573,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
assert path(root_dir / 'test_export/static/images_course_image.jpg').isfile()
@patch('xmodule.video_module.video_module.edxval_api', None)
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_export_course_image_nondefault(self, _from_json):
def test_export_course_image_nondefault(self):
"""
Make sure that if a non-default image path is specified that we
don't export it to the static default location

145
common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py
View File

@@ -39,7 +39,7 @@ from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore
from xmodule.modulestore.tests.factories import check_mongo_calls
from xmodule.modulestore.tests.mongo_connection import MONGO_HOST, MONGO_PORT_NUM
from xmodule.modulestore.tests.test_modulestore import check_has_course_method
from xmodule.modulestore.tests.utils import mock_tab_from_json
from xmodule.tabs import CourseTab
from xmodule.x_module import XModuleMixin
BRANCH_NAME_DRAFT = ModuleStoreEnum.BranchName.draft
@@ -96,21 +96,10 @@ class SplitModuleTest(unittest.TestCase):
"user_id": TEST_USER_ID,
"fields": {
"tabs": [
{
"type": "courseware"
},
{
"type": "course_info",
"name": "Course Info"
},
{
"type": "discussion",
"name": "Discussion"
},
{
"type": "wiki",
"name": "Wiki"
}
CourseTab.load('courseware'),
CourseTab.load('course_info'),
CourseTab.load('discussion'),
CourseTab.load('wiki'),
],
"start": _date_field.from_json("2013-02-14T05:00"),
"display_name": "The Ancient Greek Hero",
@@ -157,31 +146,16 @@ class SplitModuleTest(unittest.TestCase):
("course", "head12345"): {
"end": _date_field.from_json("2013-04-13T04:30"),
"tabs": [
{
"type": "courseware"
},
{
"type": "course_info",
"name": "Course Info"
},
{
"type": "discussion",
"name": "Discussion"
},
{
"type": "wiki",
"name": "Wiki"
},
{
"type": "static_tab",
"name": "Syllabus",
"url_slug": "01356a17b5924b17a04b7fc2426a3798"
},
{
"type": "static_tab",
"name": "Advice for Students",
"url_slug": "57e9991c0d794ff58f7defae3e042e39"
}
CourseTab.load('courseware'),
CourseTab.load('course_info'),
CourseTab.load('discussion'),
CourseTab.load('wiki'),
CourseTab.load(
'static_tab', name="Syllabus", url_slug="01356a17b5924b17a04b7fc2426a3798"
),
CourseTab.load(
'static_tab', name="Advice for Students", url_slug="57e9991c0d794ff58f7defae3e042e"
),
],
"graceperiod": _time_delta_field.from_json("2 hours 0 minutes 0 seconds"),
"grading_policy": {
@@ -345,21 +319,10 @@ class SplitModuleTest(unittest.TestCase):
"user_id": TEST_USER_ID,
"fields": {
"tabs": [
{
"type": "courseware"
},
{
"type": "course_info",
"name": "Course Info"
},
{
"type": "discussion",
"name": "Discussion"
},
{
"type": "wiki",
"name": "Wiki"
}
CourseTab.load('courseware'),
CourseTab.load('course_info'),
CourseTab.load('discussion'),
CourseTab.load('wiki'),
],
"start": _date_field.from_json("2013-02-14T05:00"),
"display_name": "A wonderful course",
@@ -453,21 +416,10 @@ class SplitModuleTest(unittest.TestCase):
"user_id": TEST_GUEST_USER_ID,
"fields": {
"tabs": [
{
"type": "courseware"
},
{
"type": "course_info",
"name": "Course Info"
},
{
"type": "discussion",
"name": "Discussion"
},
{
"type": "wiki",
"name": "Wiki"
}
CourseTab.load('courseware'),
CourseTab.load('course_info'),
CourseTab.load('discussion'),
CourseTab.load('wiki'),
],
"start": _date_field.from_json("2013-03-14T05:00"),
"display_name": "Yet another contender",
@@ -582,8 +534,7 @@ class SplitModuleTest(unittest.TestCase):
class TestHasChildrenAtDepth(SplitModuleTest):
"""Test the has_children_at_depth method of XModuleMixin. """
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_has_children_at_depth(self, _from_json):
def test_has_children_at_depth(self):
course_locator = CourseLocator(
org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT
)
@@ -622,8 +573,7 @@ class SplitModuleCourseTests(SplitModuleTest):
Course CRUD operation tests
'''
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_get_courses(self, _from_json):
def test_get_courses(self):
courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT)
# should have gotten 3 draft courses
assert len(courses) == 3, 'Wrong number of courses'
@@ -639,8 +589,7 @@ class SplitModuleCourseTests(SplitModuleTest):
assert course.edited_by == TEST_ASSISTANT_USER_ID
self.assertDictEqual(course.grade_cutoffs, {"Pass": 0.45})
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_get_courses_with_same_course_index(self, _from_json):
def test_get_courses_with_same_course_index(self):
"""
Test that if two courses point to same course index,
`get_courses` should return both courses.
@@ -659,8 +608,7 @@ class SplitModuleCourseTests(SplitModuleTest):
assert len(courses) == 4
assert new_draft_course.id.version_agnostic() in [c.id for c in courses]
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_get_org_courses(self, _from_json):
def test_get_org_courses(self):
courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT, org='guestx')
# should have gotten 1 draft courses
@@ -677,8 +625,7 @@ class SplitModuleCourseTests(SplitModuleTest):
courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT)
assert len(courses) == 3
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_branch_requests(self, _from_json):
def test_branch_requests(self):
# query w/ branch qualifier (both draft and published)
def _verify_published_course(courses_published):
""" Helper function for verifying published course. """
@@ -706,8 +653,7 @@ class SplitModuleCourseTests(SplitModuleTest):
locator_key_fields=['org', 'course', 'run']
)
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_get_course(self, _from_json):
def test_get_course(self):
'''
Test the various calling forms for get_course
'''
@@ -760,8 +706,7 @@ class SplitModuleCourseTests(SplitModuleTest):
with pytest.raises(ItemNotFoundError):
modulestore().get_course(CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_PUBLISHED)) # lint-amnesty, pylint: disable=line-too-long
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_cache(self, _from_json):
def test_cache(self):
"""
Test that the mechanics of caching work.
"""
@@ -773,8 +718,7 @@ class SplitModuleCourseTests(SplitModuleTest):
assert BlockKey('chapter', 'chapter1') in block_map
assert BlockKey('problem', 'problem3_2') in block_map
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_persist_dag(self, _from_json):
def test_persist_dag(self):
"""
try saving temporary xblocks
"""
@@ -919,8 +863,7 @@ class SplitModuleItemTests(SplitModuleTest):
Item read tests including inheritance
'''
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_has_item(self, _from_json):
def test_has_item(self):
'''
has_item(BlockUsageLocator)
'''
@@ -969,8 +912,7 @@ class SplitModuleItemTests(SplitModuleTest):
)
assert not modulestore().has_item(locator)
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_get_item(self, _from_json):
def test_get_item(self):
'''
get_item(blocklocator)
'''
@@ -1126,8 +1068,7 @@ class SplitModuleItemTests(SplitModuleTest):
parent = modulestore().get_parent_location(locator)
assert parent is None
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_get_children(self, _from_json):
def test_get_children(self):
"""
Test the existing get_children method on xdescriptors
"""
@@ -1482,8 +1423,7 @@ class TestItemCrud(SplitModuleTest):
other_updated = modulestore().update_item(other_block, self.user_id)
assert moved_child.version_agnostic() in version_agnostic(other_updated.children)
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_update_definition(self, _from_json):
def test_update_definition(self):
"""
test updating an item's definition: ensure it gets versioned as well as the course getting versioned
"""
@@ -1752,8 +1692,7 @@ class TestCourseCreation(SplitModuleTest):
fields['grading_policy']['GRADE_CUTOFFS']
)
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_update_course_index(self, _from_json):
def test_update_course_index(self):
"""
Test the versions pointers. NOTE: you can change the org, course, or other things, but
it's not clear how you'd find them again or associate them w/ existing student history since
@@ -1808,8 +1747,7 @@ class TestCourseCreation(SplitModuleTest):
dupe_course_key.org, dupe_course_key.course, dupe_course_key.run, user, BRANCH_NAME_DRAFT
)
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_bulk_ops_get_courses(self, _from_json):
def test_bulk_ops_get_courses(self):
"""
Test get_courses when some are created, updated, and deleted w/in a bulk operation
"""
@@ -1848,8 +1786,7 @@ class TestInheritance(SplitModuleTest):
"""
Test the metadata inheritance mechanism.
"""
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_inheritance(self, _from_json):
def test_inheritance(self):
"""
The actual test
"""
@@ -1923,8 +1860,7 @@ class TestPublish(SplitModuleTest):
"""
Test the publishing api
"""
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_publish_safe(self, _from_json):
def test_publish_safe(self):
"""
Test the standard patterns: publish to new branch, revise and publish
"""
@@ -1991,8 +1927,7 @@ class TestPublish(SplitModuleTest):
with pytest.raises(ItemNotFoundError):
modulestore().copy(self.user_id, source_course, destination_course, [problem1], [])
@patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
def test_move_delete(self, _from_json):
def test_move_delete(self):
"""
Test publishing moves and deletes.
"""

8
common/lib/xmodule/xmodule/modulestore/tests/utils.py
View File

@@ -64,14 +64,6 @@ def create_modulestore_instance(
)
def mock_tab_from_json(tab_dict):
"""
Mocks out the CourseTab.from_json to just return the tab_dict itself so that we don't have to deal
with plugin errors.
"""
return tab_dict
def add_temp_files_from_dict(file_dict, dir): # lint-amnesty, pylint: disable=redefined-builtin
"""
Takes in a dict formatted as: { file_name: content }, and adds files to directory

18
common/lib/xmodule/xmodule/tabs.py
View File

@@ -381,7 +381,6 @@ class CourseTabList(List):
__init__ method. This is because the default values are dependent on other information from
within the course.
"""
course_tabs = [
CourseTab.load('course_info'),
CourseTab.load('courseware')
@@ -406,11 +405,20 @@ class CourseTabList(List):
discussion_tab,
CourseTab.load('wiki'),
CourseTab.load('progress'),
CourseTab.load('dates'),
])
# While you should be able to do `tab.priority`, a lot of tests mock tabs to be a dict
# which causes them to throw an error on this line
course_tabs.sort(key=lambda tab: getattr(tab, 'priority', None) or float('inf'))
course.tabs.extend(course_tabs)
# Cross reference existing slugs with slugs this method would add to not add duplicates.
existing_tab_slugs = {tab.type for tab in course.tabs if course.tabs}
tabs_to_add = []
for tab in course_tabs:
if tab.type not in existing_tab_slugs:
tabs_to_add.append(tab)
if tabs_to_add:
tabs_to_add.extend(course.tabs)
tabs_to_add.sort(key=lambda tab: tab.priority or float('inf'))
course.tabs = tabs_to_add
@staticmethod
def get_discussion(course):

6
lms/djangoapps/courseware/tabs.py
View File

@@ -367,10 +367,8 @@ def get_course_tab_list(user, course):
if tab.type == 'static_tab' and tab.course_staff_only and \
not bool(user and has_access(user, 'staff', course, course.id)):
continue
# We had initially created a CourseTab.load() for dates that ended up
# persisting the dates tab tomodulestore on Course Run creation, but
# ignoring any static dates tab here we can fix forward without
# allowing the bug to continue to surface
# We are phasing this out in https://github.com/openedx/edx-platform/pull/30045/, but need this
# until the backfill course tabs command is completed
if tab.type == 'dates':
continue
course_tab_list.append(tab)

10
openedx/core/djangoapps/content/course_overviews/management/commands/simulate_publish.py
View File

@@ -85,7 +85,7 @@ class Command(BaseCommand):
dest='show_receivers',
action='store_true',
help=('Display the list of possible receiver functions and exit.')
), # lint-amnesty, pylint: disable=trailing-comma-tuple
)
parser.add_argument(
'--dry-run',
dest='dry_run',
@@ -95,7 +95,7 @@ class Command(BaseCommand):
"expensive modulestore query to find courses, but it will "
"not emit any signals."
)
), # lint-amnesty, pylint: disable=trailing-comma-tuple
)
parser.add_argument(
'--receivers',
dest='receivers',
@@ -141,7 +141,7 @@ class Command(BaseCommand):
"process. However, if you know what you're doing and need to "
"override that behavior, use this flag."
)
), # lint-amnesty, pylint: disable=trailing-comma-tuple
)
parser.add_argument(
'--skip-ccx',
dest='skip_ccx',
@@ -155,12 +155,12 @@ class Command(BaseCommand):
"if you know what you're doing, you can disable this behavior "
"with this flag, so that CCX receivers are omitted."
)
), # lint-amnesty, pylint: disable=trailing-comma-tuple
)
parser.add_argument(
'--args-from-database',
action='store_true',
help='Use arguments from the SimulateCoursePublishConfig model instead of the command line.',
), # lint-amnesty, pylint: disable=trailing-comma-tuple
)
def get_args_from_database(self):
""" Returns an options dictionary from the current SimulateCoursePublishConfig model. """

2
openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py
View File

@@ -62,7 +62,7 @@ class CourseOverviewTestCase(CatalogIntegrationMixin, ModuleStoreTestCase, Cache
None: None,
}
COURSE_OVERVIEW_TABS = {'courseware', 'info', 'textbooks', 'discussion', 'wiki', 'progress'}
COURSE_OVERVIEW_TABS = {'courseware', 'info', 'textbooks', 'discussion', 'wiki', 'progress', 'dates'}
ENABLED_SIGNALS = ['course_deleted', 'course_published']