Make course tab refreshing safer.
This commit is contained in:
@@ -19,6 +19,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
|
||||
|
||||
from models.settings.course_metadata import CourseMetadata
|
||||
from xmodule.fields import Date
|
||||
from xmodule.tabs import InvalidTabsException
|
||||
|
||||
from .utils import CourseTestCase
|
||||
from xmodule.modulestore.django import modulestore
|
||||
@@ -617,6 +618,7 @@ class CourseGradingTest(CourseTestCase):
|
||||
self.assertEqual(json.loads(response.content).get('graderType'), u'notgraded')
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class CourseMetadataEditingTest(CourseTestCase):
|
||||
"""
|
||||
Tests for CourseMetadata.
|
||||
@@ -626,6 +628,7 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
self.fullcourse = CourseFactory.create()
|
||||
self.course_setting_url = get_url(self.course.id, 'advanced_settings_handler')
|
||||
self.fullcourse_setting_url = get_url(self.fullcourse.id, 'advanced_settings_handler')
|
||||
self.notes_tab = {"type": "notes", "name": "My Notes"}
|
||||
|
||||
def test_fetch_initial_fields(self):
|
||||
test_model = CourseMetadata.fetch(self.course)
|
||||
@@ -930,12 +933,11 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
"""
|
||||
open_ended_tab = {"type": "open_ended", "name": "Open Ended Panel"}
|
||||
peer_grading_tab = {"type": "peer_grading", "name": "Peer grading"}
|
||||
notes_tab = {"type": "notes", "name": "My Notes"}
|
||||
|
||||
# First ensure that none of the tabs are visible
|
||||
self.assertNotIn(open_ended_tab, self.course.tabs)
|
||||
self.assertNotIn(peer_grading_tab, self.course.tabs)
|
||||
self.assertNotIn(notes_tab, self.course.tabs)
|
||||
self.assertNotIn(self.notes_tab, self.course.tabs)
|
||||
|
||||
# Now add the "combinedopenended" component and verify that the tab has been added
|
||||
self.client.ajax_post(self.course_setting_url, {
|
||||
@@ -944,7 +946,7 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
course = modulestore().get_course(self.course.id)
|
||||
self.assertIn(open_ended_tab, course.tabs)
|
||||
self.assertIn(peer_grading_tab, course.tabs)
|
||||
self.assertNotIn(notes_tab, course.tabs)
|
||||
self.assertNotIn(self.notes_tab, course.tabs)
|
||||
|
||||
# Now enable student notes and verify that the "My Notes" tab has also been added
|
||||
self.client.ajax_post(self.course_setting_url, {
|
||||
@@ -953,7 +955,7 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
course = modulestore().get_course(self.course.id)
|
||||
self.assertIn(open_ended_tab, course.tabs)
|
||||
self.assertIn(peer_grading_tab, course.tabs)
|
||||
self.assertIn(notes_tab, course.tabs)
|
||||
self.assertIn(self.notes_tab, course.tabs)
|
||||
|
||||
# Now remove the "combinedopenended" component and verify that the tab is gone
|
||||
self.client.ajax_post(self.course_setting_url, {
|
||||
@@ -962,7 +964,7 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
course = modulestore().get_course(self.course.id)
|
||||
self.assertNotIn(open_ended_tab, course.tabs)
|
||||
self.assertNotIn(peer_grading_tab, course.tabs)
|
||||
self.assertIn(notes_tab, course.tabs)
|
||||
self.assertIn(self.notes_tab, course.tabs)
|
||||
|
||||
# Finally disable student notes and verify that the "My Notes" tab is gone
|
||||
self.client.ajax_post(self.course_setting_url, {
|
||||
@@ -971,25 +973,40 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
course = modulestore().get_course(self.course.id)
|
||||
self.assertNotIn(open_ended_tab, course.tabs)
|
||||
self.assertNotIn(peer_grading_tab, course.tabs)
|
||||
self.assertNotIn(notes_tab, course.tabs)
|
||||
self.assertNotIn(self.notes_tab, course.tabs)
|
||||
|
||||
def mark_wiki_as_hidden(self, tabs):
|
||||
""" Mark the wiki tab as hidden. """
|
||||
for tab in tabs:
|
||||
if tab.type == 'wiki':
|
||||
tab['is_hidden'] = True
|
||||
return tabs
|
||||
def test_advanced_components_munge_tabs_validation_failure(self):
|
||||
with patch('contentstore.views.course._refresh_course_tabs', side_effect=InvalidTabsException):
|
||||
resp = self.client.ajax_post(self.course_setting_url, {
|
||||
ADVANCED_COMPONENT_POLICY_KEY: {"value": ["notes"]}
|
||||
})
|
||||
self.assertEqual(resp.status_code, 400)
|
||||
|
||||
def test_advanced_components_munge_tabs_hidden_tabs(self):
|
||||
updated_tabs = self.mark_wiki_as_hidden(self.course.tabs)
|
||||
self.course.tabs = updated_tabs
|
||||
error_msg = [
|
||||
{
|
||||
'message': 'An error occurred while trying to save your tabs',
|
||||
'model': {'display_name': 'Tabs Exception'}
|
||||
}
|
||||
]
|
||||
self.assertEqual(json.loads(resp.content), error_msg)
|
||||
|
||||
# verify that the course wasn't saved into the modulestore
|
||||
course = modulestore().get_course(self.course.id)
|
||||
self.assertNotIn("notes", course.advanced_modules)
|
||||
|
||||
@ddt.data(
|
||||
[{'type': 'courseware'}, {'type': 'course_info'}, {'type': 'wiki', 'is_hidden': True}],
|
||||
[{'type': 'courseware', 'name': 'Courses'}, {'type': 'course_info', 'name': 'Info'}],
|
||||
)
|
||||
def test_course_tab_configurations(self, tab_list):
|
||||
self.course.tabs = tab_list
|
||||
modulestore().update_item(self.course, self.user.id)
|
||||
self.client.ajax_post(self.course_setting_url, {
|
||||
ADVANCED_COMPONENT_POLICY_KEY: {"value": ["notes"]}
|
||||
})
|
||||
course = modulestore().get_course(self.course.id)
|
||||
notes_tab = {"type": "notes", "name": "My Notes"}
|
||||
self.assertIn(notes_tab, course.tabs)
|
||||
tab_list.append(self.notes_tab)
|
||||
self.assertEqual(tab_list, course.tabs)
|
||||
|
||||
|
||||
class CourseGraderUpdatesTest(CourseTestCase):
|
||||
|
||||
@@ -5,6 +5,7 @@ import copy
|
||||
from django.shortcuts import redirect
|
||||
import json
|
||||
import random
|
||||
import logging
|
||||
import string # pylint: disable=deprecated-module
|
||||
from django.utils.translation import ugettext as _
|
||||
import django.utils
|
||||
@@ -22,7 +23,7 @@ from xmodule.course_module import DEFAULT_START_DATE
|
||||
from xmodule.error_module import ErrorDescriptor
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from xmodule.contentstore.content import StaticContent
|
||||
from xmodule.tabs import CourseTab
|
||||
from xmodule.tabs import CourseTab, CourseTabList, InvalidTabsException
|
||||
from openedx.core.lib.course_tabs import CourseTabPluginManager
|
||||
from openedx.core.djangoapps.credit.api import is_credit_course, get_credit_requirements
|
||||
from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements
|
||||
@@ -87,6 +88,8 @@ from util.milestones_helpers import (
|
||||
is_valid_course_key
|
||||
)
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
__all__ = ['course_info_handler', 'course_handler', 'course_listing',
|
||||
'course_info_update_handler', 'course_search_index_handler',
|
||||
'course_rerun_handler',
|
||||
@@ -1024,6 +1027,9 @@ def grading_handler(request, course_key_string, grader_index=None):
|
||||
def _refresh_course_tabs(request, course_module):
|
||||
"""
|
||||
Automatically adds/removes tabs if changes to the course require them.
|
||||
|
||||
Raises:
|
||||
InvalidTabsException: raised if there's a problem with the new version of the tabs.
|
||||
"""
|
||||
|
||||
def update_tab(tabs, tab_type, tab_enabled):
|
||||
@@ -1047,6 +1053,8 @@ def _refresh_course_tabs(request, course_module):
|
||||
tab_enabled = tab_type.is_enabled(course_module, user=request.user)
|
||||
update_tab(course_tabs, tab_type, tab_enabled)
|
||||
|
||||
CourseTabList.validate_tabs(course_tabs)
|
||||
|
||||
# Save the tabs into the course if they have been changed
|
||||
if course_tabs != course_module.tabs:
|
||||
course_module.tabs = course_tabs
|
||||
@@ -1090,8 +1098,18 @@ def advanced_settings_handler(request, course_key_string):
|
||||
)
|
||||
|
||||
if is_valid:
|
||||
# update the course tabs if required by any setting changes
|
||||
_refresh_course_tabs(request, course_module)
|
||||
try:
|
||||
# update the course tabs if required by any setting changes
|
||||
_refresh_course_tabs(request, course_module)
|
||||
except InvalidTabsException as err:
|
||||
log.exception(err.message)
|
||||
response_message = [
|
||||
{
|
||||
'message': _('An error occurred while trying to save your tabs'),
|
||||
'model': {'display_name': _('Tabs Exception')}
|
||||
}
|
||||
]
|
||||
return JsonResponseBadRequest(response_message)
|
||||
|
||||
# now update mongo
|
||||
modulestore().update_item(course_module, request.user.id)
|
||||
@@ -1101,7 +1119,7 @@ def advanced_settings_handler(request, course_key_string):
|
||||
return JsonResponseBadRequest(errors)
|
||||
|
||||
# Handle all errors that validation doesn't catch
|
||||
except (TypeError, ValueError) as err:
|
||||
except (TypeError, ValueError, InvalidTabsException) as err:
|
||||
return HttpResponseBadRequest(
|
||||
django.utils.html.escape(err.message),
|
||||
content_type="text/plain"
|
||||
|
||||
@@ -18,6 +18,7 @@ class WikiTab(EnrolledTab):
|
||||
title = _('Wiki')
|
||||
view_name = "course_wiki"
|
||||
is_hideable = True
|
||||
is_default = False
|
||||
|
||||
@classmethod
|
||||
def is_enabled(cls, course, user=None):
|
||||
|
||||
@@ -32,6 +32,7 @@ class CoursewareTab(EnrolledTab):
|
||||
priority = 10
|
||||
view_name = 'courseware'
|
||||
is_movable = False
|
||||
is_default = False
|
||||
|
||||
|
||||
class CourseInfoTab(CourseTab):
|
||||
@@ -44,6 +45,7 @@ class CourseInfoTab(CourseTab):
|
||||
view_name = 'info'
|
||||
tab_id = 'info'
|
||||
is_movable = False
|
||||
is_default = False
|
||||
|
||||
@classmethod
|
||||
def is_enabled(cls, course, user=None):
|
||||
@@ -59,6 +61,7 @@ class SyllabusTab(EnrolledTab):
|
||||
priority = 30
|
||||
view_name = 'syllabus'
|
||||
allow_multiple = True
|
||||
is_default = False
|
||||
|
||||
@classmethod
|
||||
def is_enabled(cls, course, user=None): # pylint: disable=unused-argument
|
||||
@@ -76,6 +79,7 @@ class ProgressTab(EnrolledTab):
|
||||
priority = 40
|
||||
view_name = 'progress'
|
||||
is_hideable = True
|
||||
is_default = False
|
||||
|
||||
@classmethod
|
||||
def is_enabled(cls, course, user=None): # pylint: disable=unused-argument
|
||||
@@ -91,6 +95,7 @@ class TextbookTabsBase(CourseTab):
|
||||
# Translators: 'Textbooks' refers to the tab in the course that leads to the course' textbooks
|
||||
title = _("Textbooks")
|
||||
is_collection = True
|
||||
is_default = False
|
||||
|
||||
@classmethod
|
||||
def is_enabled(cls, course, user=None): # pylint: disable=unused-argument
|
||||
@@ -222,6 +227,7 @@ class ExternalDiscussionCourseTab(LinkTab):
|
||||
# Translators: 'Discussion' refers to the tab in the courseware that leads to the discussion forums
|
||||
title = _('Discussion')
|
||||
priority = None
|
||||
is_default = False
|
||||
|
||||
@classmethod
|
||||
def validate(cls, tab_dict, raise_error=True):
|
||||
|
||||
@@ -480,6 +480,7 @@ class TabListTestCase(TabTestCase):
|
||||
[{'type': CoursewareTab.type}, {'type': 'discussion', 'name': 'fake_name'}],
|
||||
# incorrect order
|
||||
[{'type': CourseInfoTab.type, 'name': 'fake_name'}, {'type': CoursewareTab.type}],
|
||||
[{'type': 'unknown_type'}]
|
||||
]
|
||||
|
||||
# tab types that should appear only once
|
||||
|
||||
@@ -59,6 +59,7 @@ class DiscussionTab(EnrolledTab):
|
||||
priority = None
|
||||
view_name = 'django_comment_client.forum.views.forum_form_discussion'
|
||||
is_hideable = settings.FEATURES.get('ALLOW_HIDING_DISCUSSION_TAB', False)
|
||||
is_default = False
|
||||
|
||||
@classmethod
|
||||
def is_enabled(cls, course, user=None):
|
||||
|
||||
Reference in New Issue
Block a user