Add extensible course view types for edX platform
This commit is contained in:
committed by
Diana Huang
parent
9008548c25
commit
94e1c42314
@@ -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_and_update_from_json(
|
||||
is_valid, errors, test_model = CourseMetadata.validate_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_and_update_from_json(
|
||||
is_valid, errors, test_model = CourseMetadata.validate_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_and_update_from_json(
|
||||
is_valid, errors, test_model = CourseMetadata.validate_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_and_update_from_json(
|
||||
is_valid, errors, test_model = CourseMetadata.validate_from_json(
|
||||
self.course,
|
||||
{
|
||||
"edxnotes": {"value": "true"},
|
||||
@@ -788,8 +788,8 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
)
|
||||
self.assertNotIn('edxnotes', test_model)
|
||||
|
||||
def test_validate_and_update_from_json_correct_inputs(self):
|
||||
is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json(
|
||||
def test_validate_from_json_correct_inputs(self):
|
||||
is_valid, errors, test_model = CourseMetadata.validate_from_json(
|
||||
self.course,
|
||||
{
|
||||
"advertised_start": {"value": "start A"},
|
||||
@@ -802,18 +802,13 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
self.assertTrue(len(errors) == 0)
|
||||
self.update_check(test_model)
|
||||
|
||||
# fresh fetch to ensure persistence
|
||||
fresh = modulestore().get_course(self.course.id)
|
||||
test_model = CourseMetadata.fetch(fresh)
|
||||
self.update_check(test_model)
|
||||
|
||||
# Tab gets tested in test_advanced_settings_munge_tabs
|
||||
self.assertIn('advanced_modules', test_model, 'Missing advanced_modules')
|
||||
self.assertEqual(test_model['advanced_modules']['value'], ['combinedopenended'], 'advanced_module is not updated')
|
||||
|
||||
def test_validate_and_update_from_json_wrong_inputs(self):
|
||||
def test_validate_from_json_wrong_inputs(self):
|
||||
# input incorrectly formatted data
|
||||
is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json(
|
||||
is_valid, errors, test_model = CourseMetadata.validate_from_json(
|
||||
self.course,
|
||||
{
|
||||
"advertised_start": {"value": 1, "display_name": "Course Advertised Start Date", },
|
||||
@@ -824,7 +819,7 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
user=self.user
|
||||
)
|
||||
|
||||
# Check valid results from validate_and_update_from_json
|
||||
# Check valid results from validate_from_json
|
||||
self.assertFalse(is_valid)
|
||||
self.assertEqual(len(errors), 3)
|
||||
self.assertFalse(test_model)
|
||||
@@ -947,23 +942,6 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
course = modulestore().get_course(self.course.id)
|
||||
self.assertNotIn(EXTRA_TAB_PANELS.get("open_ended"), course.tabs)
|
||||
|
||||
@patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': True})
|
||||
def test_course_settings_munge_tabs(self):
|
||||
"""
|
||||
Test that adding and removing specific course settings adds and removes tabs.
|
||||
"""
|
||||
self.assertNotIn(EXTRA_TAB_PANELS.get("edxnotes"), self.course.tabs)
|
||||
self.client.ajax_post(self.course_setting_url, {
|
||||
"edxnotes": {"value": True}
|
||||
})
|
||||
course = modulestore().get_course(self.course.id)
|
||||
self.assertIn(EXTRA_TAB_PANELS.get("edxnotes"), course.tabs)
|
||||
self.client.ajax_post(self.course_setting_url, {
|
||||
"edxnotes": {"value": False}
|
||||
})
|
||||
course = modulestore().get_course(self.course.id)
|
||||
self.assertNotIn(EXTRA_TAB_PANELS.get("edxnotes"), course.tabs)
|
||||
|
||||
|
||||
class CourseGraderUpdatesTest(CourseTestCase):
|
||||
"""
|
||||
|
||||
@@ -108,61 +108,6 @@ class ExtraPanelTabTestCase(TestCase):
|
||||
course.tabs = tabs
|
||||
return course
|
||||
|
||||
def test_add_extra_panel_tab(self):
|
||||
""" Tests if a tab can be added to a course tab list. """
|
||||
for tab_type in utils.EXTRA_TAB_PANELS.keys():
|
||||
tab = utils.EXTRA_TAB_PANELS.get(tab_type)
|
||||
|
||||
# test adding with changed = True
|
||||
for tab_setup in ['', 'x', 'x,y,z']:
|
||||
course = self.get_course_with_tabs(tab_setup)
|
||||
expected_tabs = copy.copy(course.tabs)
|
||||
expected_tabs.append(tab)
|
||||
changed, actual_tabs = utils.add_extra_panel_tab(tab_type, course)
|
||||
self.assertTrue(changed)
|
||||
self.assertEqual(actual_tabs, expected_tabs)
|
||||
|
||||
# test adding with changed = False
|
||||
tab_test_setup = [
|
||||
[tab],
|
||||
[tab, self.get_tab_type_dicts('x,y,z')],
|
||||
[self.get_tab_type_dicts('x,y'), tab, self.get_tab_type_dicts('z')],
|
||||
[self.get_tab_type_dicts('x,y,z'), tab]]
|
||||
|
||||
for tab_setup in tab_test_setup:
|
||||
course = self.get_course_with_tabs(tab_setup)
|
||||
expected_tabs = copy.copy(course.tabs)
|
||||
changed, actual_tabs = utils.add_extra_panel_tab(tab_type, course)
|
||||
self.assertFalse(changed)
|
||||
self.assertEqual(actual_tabs, expected_tabs)
|
||||
|
||||
def test_remove_extra_panel_tab(self):
|
||||
""" Tests if a tab can be removed from a course tab list. """
|
||||
for tab_type in utils.EXTRA_TAB_PANELS.keys():
|
||||
tab = utils.EXTRA_TAB_PANELS.get(tab_type)
|
||||
|
||||
# test removing with changed = True
|
||||
tab_test_setup = [
|
||||
[tab],
|
||||
[tab, self.get_tab_type_dicts('x,y,z')],
|
||||
[self.get_tab_type_dicts('x,y'), tab, self.get_tab_type_dicts('z')],
|
||||
[self.get_tab_type_dicts('x,y,z'), tab]]
|
||||
|
||||
for tab_setup in tab_test_setup:
|
||||
course = self.get_course_with_tabs(tab_setup)
|
||||
expected_tabs = [t for t in course.tabs if t != utils.EXTRA_TAB_PANELS.get(tab_type)]
|
||||
changed, actual_tabs = utils.remove_extra_panel_tab(tab_type, course)
|
||||
self.assertTrue(changed)
|
||||
self.assertEqual(actual_tabs, expected_tabs)
|
||||
|
||||
# test removing with changed = False
|
||||
for tab_setup in ['', 'x', 'x,y,z']:
|
||||
course = self.get_course_with_tabs(tab_setup)
|
||||
expected_tabs = copy.copy(course.tabs)
|
||||
changed, actual_tabs = utils.remove_extra_panel_tab(tab_type, course)
|
||||
self.assertFalse(changed)
|
||||
self.assertEqual(actual_tabs, expected_tabs)
|
||||
|
||||
|
||||
class CourseImageTestCase(ModuleStoreTestCase):
|
||||
"""Tests for course image URLs."""
|
||||
|
||||
@@ -95,7 +95,7 @@ class CourseTestCase(ModuleStoreTestCase):
|
||||
client = AjaxEnabledTestClient()
|
||||
if authenticate:
|
||||
client.login(username=nonstaff.username, password=password)
|
||||
nonstaff.is_authenticated = True
|
||||
nonstaff.is_authenticated = lambda: authenticate
|
||||
return client, nonstaff
|
||||
|
||||
def populate_course(self, branching=2):
|
||||
|
||||
@@ -3,7 +3,6 @@ Common utility functions useful throughout the contentstore
|
||||
"""
|
||||
# pylint: disable=no-member
|
||||
|
||||
import copy
|
||||
import logging
|
||||
import re
|
||||
from datetime import datetime
|
||||
@@ -30,8 +29,7 @@ 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"}
|
||||
EDXNOTES_PANEL = {"name": _("Notes"), "type": "edxnotes"}
|
||||
EXTRA_TAB_PANELS = dict([(p['type'], p) for p in [OPEN_ENDED_PANEL, NOTES_PANEL, EDXNOTES_PANEL]])
|
||||
EXTRA_TAB_PANELS = {p['type']: p for p in [OPEN_ENDED_PANEL, NOTES_PANEL]}
|
||||
|
||||
|
||||
def add_instructor(course_key, requesting_user, new_instructor):
|
||||
@@ -287,46 +285,6 @@ def ancestor_has_staff_lock(xblock, parent_xblock=None):
|
||||
return parent_xblock.visible_to_staff_only
|
||||
|
||||
|
||||
def add_extra_panel_tab(tab_type, course):
|
||||
"""
|
||||
Used to add the panel tab to a course if it does not exist.
|
||||
@param tab_type: A string representing the tab type.
|
||||
@param course: A course object from the modulestore.
|
||||
@return: Boolean indicating whether or not a tab was added and a list of tabs for the course.
|
||||
"""
|
||||
# Copy course tabs
|
||||
course_tabs = copy.copy(course.tabs)
|
||||
changed = False
|
||||
# Check to see if open ended panel is defined in the course
|
||||
|
||||
tab_panel = EXTRA_TAB_PANELS.get(tab_type)
|
||||
if tab_panel not in course_tabs:
|
||||
# Add panel to the tabs if it is not defined
|
||||
course_tabs.append(tab_panel)
|
||||
changed = True
|
||||
return changed, course_tabs
|
||||
|
||||
|
||||
def remove_extra_panel_tab(tab_type, course):
|
||||
"""
|
||||
Used to remove the panel tab from a course if it exists.
|
||||
@param tab_type: A string representing the tab type.
|
||||
@param course: A course object from the modulestore.
|
||||
@return: Boolean indicating whether or not a tab was added and a list of tabs for the course.
|
||||
"""
|
||||
# Copy course tabs
|
||||
course_tabs = copy.copy(course.tabs)
|
||||
changed = False
|
||||
# Check to see if open ended panel is defined in the course
|
||||
|
||||
tab_panel = EXTRA_TAB_PANELS.get(tab_type)
|
||||
if tab_panel in course_tabs:
|
||||
# Add panel to the tabs if it is not defined
|
||||
course_tabs = [ct for ct in course_tabs if ct != tab_panel]
|
||||
changed = True
|
||||
return changed, course_tabs
|
||||
|
||||
|
||||
def reverse_url(handler_name, key_name=None, key_value=None, kwargs=None):
|
||||
"""
|
||||
Creates the URL for the given handler.
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
"""
|
||||
Views related to operations on course objects
|
||||
"""
|
||||
import copy
|
||||
from django.shortcuts import redirect
|
||||
import json
|
||||
import random
|
||||
@@ -22,12 +23,13 @@ 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
|
||||
from xmodule.tabs import PDFTextbookTabs, CourseTab, CourseTabManager
|
||||
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
|
||||
@@ -42,13 +44,12 @@ from contentstore.utils import (
|
||||
add_instructor,
|
||||
initialize_permissions,
|
||||
get_lms_link_for_item,
|
||||
add_extra_panel_tab,
|
||||
remove_extra_panel_tab,
|
||||
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
|
||||
@@ -993,37 +994,6 @@ def grading_handler(request, course_key_string, grader_index=None):
|
||||
return JsonResponse()
|
||||
|
||||
|
||||
# pylint: disable=invalid-name
|
||||
def _add_tab(request, tab_type, course_module):
|
||||
"""
|
||||
Adds tab to the course.
|
||||
"""
|
||||
# Add tab to the course if needed
|
||||
changed, new_tabs = add_extra_panel_tab(tab_type, course_module)
|
||||
# If a tab has been added to the course, then send the
|
||||
# metadata along to CourseMetadata.update_from_json
|
||||
if changed:
|
||||
course_module.tabs = new_tabs
|
||||
request.json.update({'tabs': {'value': new_tabs}})
|
||||
# Indicate that tabs should not be filtered out of
|
||||
# the metadata
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
# pylint: disable=invalid-name
|
||||
def _remove_tab(request, tab_type, course_module):
|
||||
"""
|
||||
Removes the tab from the course.
|
||||
"""
|
||||
changed, new_tabs = remove_extra_panel_tab(tab_type, course_module)
|
||||
if changed:
|
||||
course_module.tabs = new_tabs
|
||||
request.json.update({'tabs': {'value': new_tabs}})
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def is_advanced_component_present(request, advanced_components):
|
||||
"""
|
||||
Return True when one of `advanced_components` is present in the request.
|
||||
@@ -1047,13 +1017,9 @@ def is_field_value_true(request, field_list):
|
||||
return any([request.json.get(field, {}).get('value') for field in field_list])
|
||||
|
||||
|
||||
# pylint: disable=invalid-name
|
||||
def _modify_tabs_to_components(request, course_module):
|
||||
def _refresh_course_tabs(request, course_module):
|
||||
"""
|
||||
Automatically adds/removes tabs if user indicated that they want
|
||||
respective modules enabled in the course
|
||||
|
||||
Return True when tab configuration has been modified.
|
||||
Automatically adds/removes tabs if changes to the course require them.
|
||||
"""
|
||||
tab_component_map = {
|
||||
# 'tab_type': (check_function, list_of_checked_components_or_values),
|
||||
@@ -1062,11 +1028,20 @@ def _modify_tabs_to_components(request, course_module):
|
||||
'open_ended': (is_advanced_component_present, OPEN_ENDED_COMPONENT_TYPES),
|
||||
# notes tab
|
||||
'notes': (is_advanced_component_present, NOTE_COMPONENT_TYPES),
|
||||
# student notes tab
|
||||
'edxnotes': (is_field_value_true, ['edxnotes'])
|
||||
}
|
||||
|
||||
tabs_changed = False
|
||||
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:
|
||||
tabs.append(CourseTab.from_json(tab_panel))
|
||||
elif tab_panel in tabs:
|
||||
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:
|
||||
@@ -1075,19 +1050,30 @@ def _modify_tabs_to_components(request, course_module):
|
||||
# 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)
|
||||
|
||||
if tab_enabled:
|
||||
# check passed, some of this component_types are present, adding tab
|
||||
if _add_tab(request, tab_type, course_module):
|
||||
# tab indeed was added, the change needs to propagate
|
||||
tabs_changed = True
|
||||
else:
|
||||
# the tab should not be present (anymore)
|
||||
if _remove_tab(request, tab_type, course_module):
|
||||
# tab indeed was removed, the change needs to propagate
|
||||
tabs_changed = True
|
||||
# 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)
|
||||
update_tab(course_tabs, tab_type, tab_enabled)
|
||||
|
||||
return tabs_changed
|
||||
# Save the tabs into the course if they have been changed
|
||||
if not 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
|
||||
@@ -1119,18 +1105,21 @@ def advanced_settings_handler(request, course_key_string):
|
||||
return JsonResponse(CourseMetadata.fetch(course_module))
|
||||
else:
|
||||
try:
|
||||
# do not process tabs unless they were modified according to course metadata
|
||||
filter_tabs = not _modify_tabs_to_components(request, course_module)
|
||||
|
||||
# validate data formats and update
|
||||
is_valid, errors, updated_data = CourseMetadata.validate_and_update_from_json(
|
||||
# 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(
|
||||
course_module,
|
||||
request.json,
|
||||
filter_tabs=filter_tabs,
|
||||
user=request.user,
|
||||
)
|
||||
|
||||
if is_valid:
|
||||
# update the course tabs if required by any setting changes
|
||||
_refresh_course_tabs(request, course_module)
|
||||
|
||||
# now update mongo
|
||||
modulestore().update_item(course_module, request.user.id)
|
||||
|
||||
return JsonResponse(updated_data)
|
||||
else:
|
||||
return JsonResponseBadRequest(errors)
|
||||
|
||||
@@ -61,10 +61,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_cms(
|
||||
course_item,
|
||||
settings,
|
||||
):
|
||||
for tab in CourseTabList.iterate_displayable(course_item, settings, 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)
|
||||
|
||||
@@ -116,7 +116,7 @@ class ImportTestCase(CourseTestCase):
|
||||
Check that course is imported successfully in existing course and users have their access roles
|
||||
"""
|
||||
# Create a non_staff user and add it to course staff only
|
||||
__, nonstaff_user = self.create_non_staff_authed_user_client(authenticate=False)
|
||||
__, nonstaff_user = self.create_non_staff_authed_user_client()
|
||||
auth.add_users(self.user, CourseStaffRole(self.course.id), nonstaff_user)
|
||||
|
||||
course = self.store.get_course(self.course.id)
|
||||
|
||||
@@ -150,11 +150,11 @@ class CourseMetadata(object):
|
||||
return cls.update_from_dict(key_values, descriptor, user)
|
||||
|
||||
@classmethod
|
||||
def validate_and_update_from_json(cls, descriptor, jsondict, user, filter_tabs=True):
|
||||
def validate_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 in the database.
|
||||
If all fields validate, go ahead and update those values on the object and return it.
|
||||
If not, return the error objects list.
|
||||
|
||||
Returns:
|
||||
@@ -183,19 +183,19 @@ class CourseMetadata(object):
|
||||
|
||||
# If did validate, go ahead and update the metadata
|
||||
if did_validate:
|
||||
updated_data = cls.update_from_dict(key_values, descriptor, user)
|
||||
updated_data = cls.update_from_dict(key_values, descriptor, user, save=False)
|
||||
|
||||
return did_validate, errors, updated_data
|
||||
|
||||
@classmethod
|
||||
def update_from_dict(cls, key_values, descriptor, user):
|
||||
def update_from_dict(cls, key_values, descriptor, user, save=True):
|
||||
"""
|
||||
Update metadata descriptor in modulestore from key_values.
|
||||
Update metadata descriptor from key_values. Saves to modulestore if save is true.
|
||||
"""
|
||||
for key, value in key_values.iteritems():
|
||||
setattr(descriptor, key, value)
|
||||
|
||||
if len(key_values):
|
||||
if save and len(key_values):
|
||||
modulestore().update_item(descriptor, user.id)
|
||||
|
||||
return cls.fetch(descriptor)
|
||||
|
||||
Reference in New Issue
Block a user