TNL-213: Let Students Add Personal Notes to Course Content.
Co-Authored-By: Jean-Michel Claus <jmc@edx.org> Co-Authored-By: Brian Talbot <btalbot@edx.org> Co-Authored-By: Tim Babych <tim@edx.org> Co-Authored-By: Oleg Marshev <oleg@edx.org> Co-Authored-By: Chris Rodriguez <crodriguez@edx.org>
This commit is contained in:
@@ -552,6 +552,80 @@ class CourseMetadataEditingTest(CourseTestCase):
|
||||
)
|
||||
self.assertNotIn('giturl', test_model)
|
||||
|
||||
@patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': True})
|
||||
def test_edxnotes_present(self):
|
||||
"""
|
||||
If feature flag ENABLE_EDXNOTES is on, show the setting as a non-deprecated Advanced Setting.
|
||||
"""
|
||||
test_model = CourseMetadata.fetch(self.fullcourse)
|
||||
self.assertIn('edxnotes', test_model)
|
||||
|
||||
@patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': False})
|
||||
def test_edxnotes_not_present(self):
|
||||
"""
|
||||
If feature flag ENABLE_EDXNOTES is off, don't show the setting at all on the Advanced Settings page.
|
||||
"""
|
||||
test_model = CourseMetadata.fetch(self.fullcourse)
|
||||
self.assertNotIn('edxnotes', test_model)
|
||||
|
||||
@patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': False})
|
||||
def test_validate_update_filtered_edxnotes_off(self):
|
||||
"""
|
||||
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(
|
||||
self.course,
|
||||
{
|
||||
"edxnotes": {"value": "true"},
|
||||
},
|
||||
user=self.user
|
||||
)
|
||||
self.assertNotIn('edxnotes', test_model)
|
||||
|
||||
@patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': True})
|
||||
def test_validate_update_filtered_edxnotes_on(self):
|
||||
"""
|
||||
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(
|
||||
self.course,
|
||||
{
|
||||
"edxnotes": {"value": "true"},
|
||||
},
|
||||
user=self.user
|
||||
)
|
||||
self.assertIn('edxnotes', test_model)
|
||||
|
||||
@patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': True})
|
||||
def test_update_from_json_filtered_edxnotes_on(self):
|
||||
"""
|
||||
If feature flag is on, then edxnotes must be updated.
|
||||
"""
|
||||
test_model = CourseMetadata.update_from_json(
|
||||
self.course,
|
||||
{
|
||||
"edxnotes": {"value": "true"},
|
||||
},
|
||||
user=self.user
|
||||
)
|
||||
self.assertIn('edxnotes', test_model)
|
||||
|
||||
@patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': False})
|
||||
def test_update_from_json_filtered_edxnotes_off(self):
|
||||
"""
|
||||
If feature flag is off, then edxnotes must not be updated.
|
||||
"""
|
||||
test_model = CourseMetadata.update_from_json(
|
||||
self.course,
|
||||
{
|
||||
"edxnotes": {"value": "true"},
|
||||
},
|
||||
user=self.user
|
||||
)
|
||||
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(
|
||||
self.course,
|
||||
@@ -711,6 +785,23 @@ 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):
|
||||
"""
|
||||
|
||||
@@ -30,7 +30,8 @@ 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 = dict([(p['type'], p) for p in [OPEN_ENDED_PANEL, NOTES_PANEL]])
|
||||
EDXNOTES_PANEL = {"name": _("Notes"), "type": "edxnotes"}
|
||||
EXTRA_TAB_PANELS = dict([(p['type'], p) for p in [OPEN_ENDED_PANEL, NOTES_PANEL, EDXNOTES_PANEL]])
|
||||
|
||||
|
||||
def add_instructor(course_key, requesting_user, new_instructor):
|
||||
|
||||
@@ -867,62 +867,100 @@ def grading_handler(request, course_key_string, grader_index=None):
|
||||
|
||||
|
||||
# pylint: disable=invalid-name
|
||||
def _config_course_advanced_components(request, course_module):
|
||||
def _add_tab(request, tab_type, course_module):
|
||||
"""
|
||||
Check to see if the user instantiated any advanced components. This
|
||||
is a hack that does the following :
|
||||
1) adds/removes the open ended panel tab to a course automatically
|
||||
if the user has indicated that they want to edit the
|
||||
combinedopendended or peergrading module
|
||||
2) adds/removes the notes panel tab to a course automatically if
|
||||
the user has indicated that they want the notes module enabled in
|
||||
their course
|
||||
Adds tab to the course.
|
||||
"""
|
||||
# TODO refactor the above into distinct advanced policy settings
|
||||
filter_tabs = True # Exceptional conditions will pull this to False
|
||||
if ADVANCED_COMPONENT_POLICY_KEY in request.json: # Maps tab types to components
|
||||
tab_component_map = {
|
||||
'open_ended': OPEN_ENDED_COMPONENT_TYPES,
|
||||
'notes': NOTE_COMPONENT_TYPES,
|
||||
}
|
||||
# Check to see if the user instantiated any notes or open ended components
|
||||
for tab_type in tab_component_map.keys():
|
||||
component_types = tab_component_map.get(tab_type)
|
||||
found_ac_type = False
|
||||
for ac_type in component_types:
|
||||
# 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
|
||||
|
||||
# Check if the user has incorrectly failed to put the value in an iterable.
|
||||
new_advanced_component_list = request.json[ADVANCED_COMPONENT_POLICY_KEY]['value']
|
||||
if hasattr(new_advanced_component_list, '__iter__'):
|
||||
if ac_type in new_advanced_component_list and ac_type in ADVANCED_COMPONENT_TYPES:
|
||||
|
||||
# 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
|
||||
filter_tabs = False # Set this flag to avoid the tab removal code below.
|
||||
found_ac_type = True # break
|
||||
else:
|
||||
# If not iterable, return immediately and let validation handle.
|
||||
return
|
||||
# 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
|
||||
|
||||
# If we did not find a module type in the advanced settings,
|
||||
# we may need to remove the tab from the course.
|
||||
if not found_ac_type: # Remove tab from the course if needed
|
||||
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}})
|
||||
# Indicate that tabs should *not* be filtered out of
|
||||
# the metadata
|
||||
filter_tabs = False
|
||||
|
||||
return filter_tabs
|
||||
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])
|
||||
|
||||
|
||||
# pylint: disable=invalid-name
|
||||
def _modify_tabs_to_components(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.
|
||||
"""
|
||||
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),
|
||||
# student notes tab
|
||||
'edxnotes': (is_field_value_true, ['edxnotes'])
|
||||
}
|
||||
|
||||
tabs_changed = False
|
||||
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
|
||||
|
||||
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
|
||||
|
||||
return tabs_changed
|
||||
|
||||
|
||||
@login_required
|
||||
@@ -954,8 +992,8 @@ def advanced_settings_handler(request, course_key_string):
|
||||
return JsonResponse(CourseMetadata.fetch(course_module))
|
||||
else:
|
||||
try:
|
||||
# Whether or not to filter the tabs key out of the settings metadata
|
||||
filter_tabs = _config_course_advanced_components(request, course_module)
|
||||
# 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(
|
||||
|
||||
@@ -47,6 +47,10 @@ class CourseMetadata(object):
|
||||
if not settings.FEATURES.get('ENABLE_EXPORT_GIT'):
|
||||
filtered_list.append('giturl')
|
||||
|
||||
# Do not show edxnotes if the feature is disabled.
|
||||
if not settings.FEATURES.get('ENABLE_EDXNOTES'):
|
||||
filtered_list.append('edxnotes')
|
||||
|
||||
return filtered_list
|
||||
|
||||
@classmethod
|
||||
|
||||
Reference in New Issue
Block a user