From c384ee50b77e52fdf5cf78b7e3caa5f04fe5eef2 Mon Sep 17 00:00:00 2001 From: Tim Babych Date: Mon, 12 Jan 2015 14:30:31 +0200 Subject: [PATCH] refactor tab modification by components --- cms/djangoapps/contentstore/views/course.py | 130 +++++++++----------- 1 file changed, 56 insertions(+), 74 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index d8ccc4594ddd..10fbfc495424 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -868,87 +868,70 @@ def _remove_tab(request, tab_type, course_module): return False -# pylint: disable=invalid-name -def _config_course_advanced_components(request, 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 - """ - # 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: - # 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: - 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. - return +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 - # 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, tab_type, course_module): - # Indicate that tabs should *not* be filtered out of - # the metadata - filter_tabs = False - return filter_tabs +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 _config_course_settings(request, course_module, filter_tabs=True): +def _modify_tabs_to_components(request, course_module): """ - Check to see if the user enabled some advanced settings (boolean). - This is a hack that does the following : - 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 + 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 = { - 'edxnotes': ['edxnotes'] + # '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']) } - # Check to see if the user instantiated any notes or open ended components + + tabs_changed = False 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 + 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 @@ -980,9 +963,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) - filter_tabs = _config_course_settings(request, course_module, filter_tabs) + # 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(