Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions cms/djangoapps/contentstore/tests/test_course_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,80 @@ def test_update_from_json_filtered_off(self):
)
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 on, 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,
Expand Down Expand Up @@ -711,6 +785,22 @@ def test_advanced_components_munge_tabs(self):
course = modulestore().get_course(self.course.id)
self.assertNotIn(EXTRA_TAB_PANELS.get("open_ended"), course.tabs)

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):
"""
Expand Down
3 changes: 2 additions & 1 deletion cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
# 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):
Expand Down
89 changes: 73 additions & 16 deletions cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,37 @@ 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


# pylint: disable=invalid-name
def _config_course_advanced_components(request, course_module):
"""
Expand All @@ -845,6 +876,7 @@ def _config_course_advanced_components(request, course_module):
"""
# 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,
Expand All @@ -855,22 +887,13 @@ def _config_course_advanced_components(request, course_module):
component_types = tab_component_map.get(tab_type)
found_ac_type = False
for ac_type in component_types:

# 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.
if _add_tab(request, tab_type, course_module):
# Set this flag to avoid the tab removal code below.
filter_tabs = False
found_ac_type = True # break
else:
# If not iterable, return immediately and let validation handle.
Expand All @@ -879,17 +902,50 @@ def _config_course_advanced_components(request, course_module):
# 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}})
if _remove_tab(request, tab_type, course_module):
# Indicate that tabs should *not* be filtered out of
# the metadata
filter_tabs = False

return filter_tabs


# pylint: disable=invalid-name
def _config_course_settings(request, course_module, filter_tabs=True):
"""
Check to see if the user enabled some advanced settings (boolean).
This is a hack that does the following :

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add more formal Summary line at first.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add more formal Summary line at first.

Done.

1) adds/removes the edx notes panel tab to a course automatically if
the user has indicated that they want the notes module enabled in
their course
"""
tab_component_map = {
'edxnotes': ['edxnotes']
}
# Check to see if the user instantiated any notes or open ended components
for tab_type in tab_component_map.keys():
if tab_type in request.json:
component_types = tab_component_map.get(tab_type)
found_ac_type = False
for ac_type in component_types:
field_value = request.json[ac_type]['value']
if field_value is True:
if _add_tab(request, ac_type, course_module):
# Set this flag to avoid the tab removal code below.
filter_tabs = False
found_ac_type = True # break

# 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
if _remove_tab(request, ac_type, course_module):
# Indicate that tabs should *not* be filtered out of
# the metadata
filter_tabs = False

return filter_tabs


@login_required
@ensure_csrf_cookie
@require_http_methods(("GET", "POST", "PUT"))
Expand Down Expand Up @@ -921,6 +977,7 @@ def advanced_settings_handler(request, course_key_string):
try:
# Whether or not to filter the tabs key out of the settings metadata
filter_tabs = _config_course_advanced_components(request, course_module)
filter_tabs = _config_course_settings(request, course_module, filter_tabs)

# validate data formats and update
is_valid, errors, updated_data = CourseMetadata.validate_and_update_from_json(
Expand Down
1 change: 1 addition & 0 deletions cms/djangoapps/contentstore/views/tabs.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +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,
Expand Down
6 changes: 5 additions & 1 deletion cms/djangoapps/models/settings/course_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class CourseMetadata(object):
'user_partitions',
'name', # from xblock
'tags', # from xblock
'visible_to_staff_only'
'visible_to_staff_only',
]

@classmethod
Expand All @@ -45,6 +45,10 @@ def filtered_list(cls):
if not settings.FEATURES.get('ENABLE_EXPORT_GIT'):
filtered_list.append('giturl')

# Do not show edxnotes if feature is not enabled.
if not settings.FEATURES.get('ENABLE_EDXNOTES'):
filtered_list.append('edxnotes')

return filtered_list

@classmethod
Expand Down
2 changes: 2 additions & 0 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@

# Modulestore to use for new courses
'DEFAULT_STORE_FOR_NEW_COURSE': None,

'ENABLE_EDXNOTES': True,
}
ENABLE_JASMINE = False

Expand Down
2 changes: 2 additions & 0 deletions cms/envs/devstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,5 @@
#####################################################################
# Lastly, run any migrations, if needed.
MODULESTORE = convert_module_store_setting_if_needed(MODULESTORE)

FEATURES['ENABLE_EDXNOTES'] = True
1 change: 1 addition & 0 deletions cms/envs/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@
# the one in lms/envs/test.py
FEATURES['ENABLE_DISCUSSION_SERVICE'] = False

FEATURES['ENABLE_EDXNOTES'] = True
EDXNOTES_INTERFACE = {
'url': 'http://localhost:8042/',
}
51 changes: 0 additions & 51 deletions common/djangoapps/edxnotes/helpers.py

This file was deleted.

Loading