Convert all tabs to the new plugin framework.
This commit is contained in:
@@ -225,17 +225,6 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
self._create_course_with_access_groups(course_location, self.user)
|
||||
store.delete_course(course_location, self.user.id)
|
||||
|
||||
course_location = self.store.make_course_key('testOrg', 'erroredCourse', 'RunBabyRun')
|
||||
course = self._create_course_with_access_groups(course_location, self.user)
|
||||
course_db_record = store._find_one(course.location)
|
||||
course_db_record.setdefault('metadata', {}).get('tabs', []).append({"type": "wiko", "name": "Wiki"})
|
||||
store.collection.update(
|
||||
{'_id': course.location.to_deprecated_son()},
|
||||
{'$set': {
|
||||
'metadata.tabs': course_db_record['metadata']['tabs'],
|
||||
}},
|
||||
)
|
||||
|
||||
courses_list, __ = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(len(courses_list), 1, courses_list)
|
||||
|
||||
|
||||
@@ -14,7 +14,7 @@ from django.conf import settings
|
||||
|
||||
from models.settings.course_details import (CourseDetails, CourseSettingsEncoder)
|
||||
from models.settings.course_grading import CourseGradingModel
|
||||
from contentstore.utils import EXTRA_TAB_PANELS, reverse_course_url, reverse_usage_url
|
||||
from contentstore.utils import reverse_course_url, reverse_usage_url
|
||||
from xmodule.modulestore.tests.factories import CourseFactory
|
||||
|
||||
from models.settings.course_metadata import CourseMetadata
|
||||
@@ -662,7 +662,7 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
If feature flag is off, then giturl must be filtered.
|
||||
"""
|
||||
# pylint: disable=unused-variable
|
||||
is_valid, errors, test_model = CourseMetadata.validate_from_json(
|
||||
is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json(
|
||||
self.course,
|
||||
{
|
||||
"giturl": {"value": "http://example.com"},
|
||||
@@ -677,7 +677,7 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
If feature flag is on, then giturl must not be filtered.
|
||||
"""
|
||||
# pylint: disable=unused-variable
|
||||
is_valid, errors, test_model = CourseMetadata.validate_from_json(
|
||||
is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json(
|
||||
self.course,
|
||||
{
|
||||
"giturl": {"value": "http://example.com"},
|
||||
@@ -736,7 +736,7 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
If feature flag is off, then edxnotes must be filtered.
|
||||
"""
|
||||
# pylint: disable=unused-variable
|
||||
is_valid, errors, test_model = CourseMetadata.validate_from_json(
|
||||
is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json(
|
||||
self.course,
|
||||
{
|
||||
"edxnotes": {"value": "true"},
|
||||
@@ -751,7 +751,7 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
If feature flag is on, then edxnotes must not be filtered.
|
||||
"""
|
||||
# pylint: disable=unused-variable
|
||||
is_valid, errors, test_model = CourseMetadata.validate_from_json(
|
||||
is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json(
|
||||
self.course,
|
||||
{
|
||||
"edxnotes": {"value": "true"},
|
||||
@@ -789,7 +789,7 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
self.assertNotIn('edxnotes', test_model)
|
||||
|
||||
def test_validate_from_json_correct_inputs(self):
|
||||
is_valid, errors, test_model = CourseMetadata.validate_from_json(
|
||||
is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json(
|
||||
self.course,
|
||||
{
|
||||
"advertised_start": {"value": "start A"},
|
||||
@@ -808,7 +808,7 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
|
||||
def test_validate_from_json_wrong_inputs(self):
|
||||
# input incorrectly formatted data
|
||||
is_valid, errors, test_model = CourseMetadata.validate_from_json(
|
||||
is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json(
|
||||
self.course,
|
||||
{
|
||||
"advertised_start": {"value": 1, "display_name": "Course Advertised Start Date", },
|
||||
@@ -819,7 +819,7 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
user=self.user
|
||||
)
|
||||
|
||||
# Check valid results from validate_from_json
|
||||
# Check valid results from validate_and_update_from_json
|
||||
self.assertFalse(is_valid)
|
||||
self.assertEqual(len(errors), 3)
|
||||
self.assertFalse(test_model)
|
||||
@@ -928,19 +928,50 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
"""
|
||||
Test that adding and removing specific advanced components adds and removes tabs.
|
||||
"""
|
||||
self.assertNotIn(EXTRA_TAB_PANELS.get("open_ended"), self.course.tabs)
|
||||
self.assertNotIn(EXTRA_TAB_PANELS.get("notes"), self.course.tabs)
|
||||
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)
|
||||
|
||||
# Now add the "combinedopenended" component and verify that the tab has been added
|
||||
self.client.ajax_post(self.course_setting_url, {
|
||||
ADVANCED_COMPONENT_POLICY_KEY: {"value": ["combinedopenended"]}
|
||||
})
|
||||
course = modulestore().get_course(self.course.id)
|
||||
self.assertIn(EXTRA_TAB_PANELS.get("open_ended"), course.tabs)
|
||||
self.assertNotIn(EXTRA_TAB_PANELS.get("notes"), course.tabs)
|
||||
self.assertIn(open_ended_tab, course.tabs)
|
||||
self.assertIn(peer_grading_tab, course.tabs)
|
||||
self.assertNotIn(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, {
|
||||
ADVANCED_COMPONENT_POLICY_KEY: {"value": []}
|
||||
ADVANCED_COMPONENT_POLICY_KEY: {"value": ["combinedopenended", "notes"]}
|
||||
})
|
||||
course = modulestore().get_course(self.course.id)
|
||||
self.assertNotIn(EXTRA_TAB_PANELS.get("open_ended"), course.tabs)
|
||||
self.assertIn(open_ended_tab, course.tabs)
|
||||
self.assertIn(peer_grading_tab, course.tabs)
|
||||
self.assertIn(notes_tab, course.tabs)
|
||||
|
||||
# Now remove the "combinedopenended" component and verify that the tab is gone
|
||||
self.client.ajax_post(self.course_setting_url, {
|
||||
ADVANCED_COMPONENT_POLICY_KEY: {"value": ["notes"]}
|
||||
})
|
||||
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)
|
||||
|
||||
# Finally disable student notes and verify that the "My Notes" tab is gone
|
||||
self.client.ajax_post(self.course_setting_url, {
|
||||
ADVANCED_COMPONENT_POLICY_KEY: {"value": [""]}
|
||||
})
|
||||
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)
|
||||
|
||||
|
||||
class CourseGraderUpdatesTest(CourseTestCase):
|
||||
|
||||
@@ -26,11 +26,6 @@ from student import auth
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
# In order to instantiate an open ended tab automatically, need to have this data
|
||||
OPEN_ENDED_PANEL = {"name": _("Open Ended Panel"), "type": "open_ended"}
|
||||
NOTES_PANEL = {"name": _("My Notes"), "type": "notes"}
|
||||
EXTRA_TAB_PANELS = {p['type']: p for p in [OPEN_ENDED_PANEL, NOTES_PANEL]}
|
||||
|
||||
|
||||
def add_instructor(course_key, requesting_user, new_instructor):
|
||||
"""
|
||||
|
||||
@@ -16,20 +16,19 @@ from django.core.urlresolvers import reverse
|
||||
from django.http import HttpResponseBadRequest, HttpResponseNotFound, HttpResponse, Http404
|
||||
from util.json_request import JsonResponse, JsonResponseBadRequest
|
||||
from util.date_utils import get_default_time_display
|
||||
from util.db import generate_int_id, MYSQL_MAX_INT
|
||||
from edxmako.shortcuts import render_to_response
|
||||
|
||||
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 PDFTextbookTabs, CourseTab, CourseTabManager
|
||||
from xmodule.tabs import CourseTab
|
||||
from openedx.core.djangoapps.course_views.course_views import CourseViewTypeManager
|
||||
from xmodule.modulestore import EdxJSONEncoder
|
||||
from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError
|
||||
from opaque_keys import InvalidKeyError
|
||||
from opaque_keys.edx.locations import Location
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from openedx.core.lib.plugins.api import CourseViewType
|
||||
|
||||
from django_future.csrf import ensure_csrf_cookie
|
||||
from contentstore.course_info_model import get_course_updates, update_course_updates, delete_course_update
|
||||
@@ -46,10 +45,8 @@ from contentstore.utils import (
|
||||
get_lms_link_for_item,
|
||||
reverse_course_url,
|
||||
reverse_library_url,
|
||||
reverse_usage_url,
|
||||
reverse_url,
|
||||
remove_all_instructors,
|
||||
EXTRA_TAB_PANELS,
|
||||
)
|
||||
from models.settings.course_details import CourseDetails, CourseSettingsEncoder
|
||||
from models.settings.course_grading import CourseGradingModel
|
||||
@@ -58,9 +55,6 @@ from util.json_request import expect_json
|
||||
from util.string_utils import _has_non_ascii_characters
|
||||
from student.auth import has_studio_write_access, has_studio_read_access
|
||||
from .component import (
|
||||
OPEN_ENDED_COMPONENT_TYPES,
|
||||
NOTE_COMPONENT_TYPES,
|
||||
ADVANCED_COMPONENT_POLICY_KEY,
|
||||
SPLIT_TEST_COMPONENT_TYPE,
|
||||
ADVANCED_COMPONENT_TYPES,
|
||||
)
|
||||
@@ -84,7 +78,6 @@ from course_action_state.models import CourseRerunState, CourseRerunUIStateManag
|
||||
from course_action_state.managers import CourseActionStateItemNotFoundError
|
||||
from microsite_configuration import microsite
|
||||
from xmodule.course_module import CourseFields
|
||||
from xmodule.split_test_module import get_split_user_partitions
|
||||
from student.auth import has_course_author_access
|
||||
|
||||
from util.milestones_helpers import (
|
||||
@@ -994,88 +987,38 @@ def grading_handler(request, course_key_string, grader_index=None):
|
||||
return JsonResponse()
|
||||
|
||||
|
||||
def is_advanced_component_present(request, advanced_components):
|
||||
"""
|
||||
Return True when one of `advanced_components` is present in the request.
|
||||
|
||||
raises TypeError
|
||||
when request.ADVANCED_COMPONENT_POLICY_KEY is malformed (not iterable)
|
||||
"""
|
||||
if ADVANCED_COMPONENT_POLICY_KEY not in request.json:
|
||||
return False
|
||||
|
||||
new_advanced_component_list = request.json[ADVANCED_COMPONENT_POLICY_KEY]['value']
|
||||
for ac_type in advanced_components:
|
||||
if ac_type in new_advanced_component_list and ac_type in ADVANCED_COMPONENT_TYPES:
|
||||
return True
|
||||
|
||||
|
||||
def is_field_value_true(request, field_list):
|
||||
"""
|
||||
Return True when one of field values is set to True by request
|
||||
"""
|
||||
return any([request.json.get(field, {}).get('value') for field in field_list])
|
||||
|
||||
|
||||
def _refresh_course_tabs(request, course_module):
|
||||
"""
|
||||
Automatically adds/removes tabs if changes to the course require them.
|
||||
"""
|
||||
tab_component_map = {
|
||||
# 'tab_type': (check_function, list_of_checked_components_or_values),
|
||||
|
||||
# open ended tab by combinedopendended or peergrading module
|
||||
'open_ended': (is_advanced_component_present, OPEN_ENDED_COMPONENT_TYPES),
|
||||
# notes tab
|
||||
'notes': (is_advanced_component_present, NOTE_COMPONENT_TYPES),
|
||||
}
|
||||
|
||||
def update_tab(tabs, tab_type, tab_enabled):
|
||||
"""
|
||||
Adds or removes a course tab based upon whether it is enabled.
|
||||
"""
|
||||
tab_panel = _get_tab_panel_for_type(tab_type)
|
||||
if tab_enabled:
|
||||
tab_panel = {
|
||||
"type": tab_type.name,
|
||||
"name": tab_type.title,
|
||||
}
|
||||
has_tab = tab_panel in tabs
|
||||
if tab_enabled and not has_tab:
|
||||
tabs.append(CourseTab.from_json(tab_panel))
|
||||
elif tab_panel in tabs:
|
||||
elif not tab_enabled and has_tab:
|
||||
tabs.remove(tab_panel)
|
||||
|
||||
course_tabs = copy.copy(course_module.tabs)
|
||||
|
||||
for tab_type in tab_component_map.keys():
|
||||
check, component_types = tab_component_map[tab_type]
|
||||
try:
|
||||
tab_enabled = check(request, component_types)
|
||||
except TypeError:
|
||||
# user has failed to put iterable value into advanced component list.
|
||||
# return immediately and let validation handle.
|
||||
return
|
||||
update_tab(course_tabs, tab_type, tab_enabled)
|
||||
|
||||
# Additionally update any persistent tabs provided by course views
|
||||
for tab_type in CourseTabManager.get_tab_types().values():
|
||||
if issubclass(tab_type, CourseViewType) and tab_type.is_persistent:
|
||||
tab_enabled = tab_type.is_enabled(course_module, settings, user=request.user)
|
||||
# Additionally update any tabs that are provided by non-dynamic course views
|
||||
for tab_type in CourseViewTypeManager.get_course_view_types():
|
||||
if not tab_type.is_dynamic and tab_type.is_default:
|
||||
tab_enabled = tab_type.is_enabled(course_module, user=request.user)
|
||||
update_tab(course_tabs, tab_type, tab_enabled)
|
||||
|
||||
# Save the tabs into the course if they have been changed
|
||||
if not course_tabs == course_module.tabs:
|
||||
if course_tabs != course_module.tabs:
|
||||
course_module.tabs = course_tabs
|
||||
|
||||
|
||||
def _get_tab_panel_for_type(tab_type):
|
||||
"""
|
||||
Returns a tab panel representation for the specified tab type.
|
||||
"""
|
||||
tab_panel = EXTRA_TAB_PANELS.get(tab_type)
|
||||
if tab_panel:
|
||||
return tab_panel
|
||||
return {
|
||||
"name": tab_type.title,
|
||||
"type": tab_type.name
|
||||
}
|
||||
|
||||
|
||||
@login_required
|
||||
@ensure_csrf_cookie
|
||||
@require_http_methods(("GET", "POST", "PUT"))
|
||||
@@ -1107,7 +1050,7 @@ def advanced_settings_handler(request, course_key_string):
|
||||
try:
|
||||
# validate data formats and update the course module.
|
||||
# Note: don't update mongo yet, but wait until after any tabs are changed
|
||||
is_valid, errors, updated_data = CourseMetadata.validate_from_json(
|
||||
is_valid, errors, updated_data = CourseMetadata.validate_and_update_from_json(
|
||||
course_module,
|
||||
request.json,
|
||||
user=request.user,
|
||||
@@ -1238,8 +1181,8 @@ def textbooks_list_handler(request, course_key_string):
|
||||
textbook["id"] = tid
|
||||
tids.add(tid)
|
||||
|
||||
if not any(tab['type'] == PDFTextbookTabs.type for tab in course.tabs):
|
||||
course.tabs.append(PDFTextbookTabs())
|
||||
if not any(tab['type'] == 'pdf_textbooks' for tab in course.tabs):
|
||||
course.tabs.append(CourseTab.load('pdf_textbooks'))
|
||||
course.pdf_textbooks = textbooks
|
||||
store.update_item(course, request.user.id)
|
||||
return JsonResponse(course.pdf_textbooks)
|
||||
@@ -1255,8 +1198,8 @@ def textbooks_list_handler(request, course_key_string):
|
||||
existing = course.pdf_textbooks
|
||||
existing.append(textbook)
|
||||
course.pdf_textbooks = existing
|
||||
if not any(tab['type'] == PDFTextbookTabs.type for tab in course.tabs):
|
||||
course.tabs.append(PDFTextbookTabs())
|
||||
if not any(tab['type'] == 'pdf_textbooks' for tab in course.tabs):
|
||||
course.tabs.append(CourseTab.load('pdf_textbooks'))
|
||||
store.update_item(course, request.user.id)
|
||||
resp = JsonResponse(textbook, status=201)
|
||||
resp["Location"] = reverse_course_url(
|
||||
|
||||
@@ -12,12 +12,12 @@ from django.http import HttpResponse
|
||||
from django.shortcuts import redirect
|
||||
from django.utils.translation import ugettext as _
|
||||
|
||||
from openedx.core.djangoapps.course_views.course_views import StaticTab
|
||||
from edxmako.shortcuts import render_to_string, render_to_response
|
||||
from opaque_keys.edx.keys import UsageKey
|
||||
from xblock.core import XBlock
|
||||
import dogstats_wrapper as dog_stats_api
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from xmodule.tabs import StaticTab
|
||||
from xmodule.x_module import DEPRECATION_VSCOMPAT_EVENT
|
||||
|
||||
from contentstore.utils import reverse_course_url, reverse_library_url, reverse_usage_url
|
||||
|
||||
@@ -10,11 +10,13 @@ from django.contrib.auth.decorators import login_required
|
||||
from django.core.exceptions import PermissionDenied
|
||||
from django_future.csrf import ensure_csrf_cookie
|
||||
from django.views.decorators.http import require_http_methods
|
||||
|
||||
from edxmako.shortcuts import render_to_response
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from xmodule.modulestore import ModuleStoreEnum
|
||||
from xmodule.tabs import CourseTabList, StaticTab, CourseTab, InvalidTabsException
|
||||
from xmodule.tabs import CourseTabList, CourseTab, InvalidTabsException
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
from openedx.core.djangoapps.course_views.course_views import StaticTab
|
||||
|
||||
from ..utils import get_lms_link_for_item
|
||||
|
||||
@@ -61,7 +63,7 @@ def tabs_handler(request, course_key_string):
|
||||
# present in the same order they are displayed in LMS
|
||||
|
||||
tabs_to_render = []
|
||||
for tab in CourseTabList.iterate_displayable(course_item, settings, inline_collections=False):
|
||||
for tab in CourseTabList.iterate_displayable(course_item, inline_collections=False):
|
||||
if isinstance(tab, StaticTab):
|
||||
# static tab needs its locator information to render itself as an xmodule
|
||||
static_tab_loc = course_key.make_usage_key('static_tab', tab.url_slug)
|
||||
|
||||
@@ -8,7 +8,7 @@ from contentstore.utils import reverse_course_url
|
||||
from xmodule.x_module import STUDENT_VIEW
|
||||
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
from xmodule.tabs import CourseTabList, WikiTab
|
||||
from xmodule.tabs import CourseTabList
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
|
||||
@@ -52,7 +52,7 @@ class TabsPageTests(CourseTestCase):
|
||||
self.client.ajax_post(
|
||||
self.url,
|
||||
data=json.dumps({
|
||||
'tab_id_locator': {'tab_id': WikiTab.type},
|
||||
'tab_id_locator': {'tab_id': 'courseware'},
|
||||
'unsupported_request': None,
|
||||
}),
|
||||
)
|
||||
@@ -158,10 +158,9 @@ class TabsPageTests(CourseTestCase):
|
||||
self.assertEqual(new_tab.is_hidden, new_is_hidden_setting)
|
||||
|
||||
def test_toggle_tab_visibility(self):
|
||||
"""Test toggling of tab visiblity"""
|
||||
|
||||
self.check_toggle_tab_visiblity(WikiTab.type, True)
|
||||
self.check_toggle_tab_visiblity(WikiTab.type, False)
|
||||
"""Test toggling of tab visibility"""
|
||||
self.check_toggle_tab_visiblity('wiki', True)
|
||||
self.check_toggle_tab_visiblity('wiki', False)
|
||||
|
||||
def test_toggle_invalid_tab_visibility(self):
|
||||
"""Test toggling visibility of an invalid tab"""
|
||||
|
||||
@@ -150,11 +150,12 @@ class CourseMetadata(object):
|
||||
return cls.update_from_dict(key_values, descriptor, user)
|
||||
|
||||
@classmethod
|
||||
def validate_from_json(cls, descriptor, jsondict, user, filter_tabs=True):
|
||||
def validate_and_update_from_json(cls, descriptor, jsondict, user, filter_tabs=True):
|
||||
"""
|
||||
Validate the values in the json dict (validated by xblock fields from_json method)
|
||||
|
||||
If all fields validate, go ahead and update those values on the object and return it.
|
||||
If all fields validate, go ahead and update those values on the object and return it without
|
||||
persisting it to the DB.
|
||||
If not, return the error objects list.
|
||||
|
||||
Returns:
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
<%!
|
||||
from django.utils.translation import ugettext as _
|
||||
from django.core.urlresolvers import reverse
|
||||
from xmodule.tabs import StaticTab
|
||||
from openedx.core.djangoapps.course_views.course_views import StaticTab
|
||||
from django.template.defaultfilters import escapejs
|
||||
%>
|
||||
<%block name="title">${_("Pages")}</%block>
|
||||
|
||||
Reference in New Issue
Block a user