TNL-213: Student Notes

CHANGELOG.rst

@@ -5,6 +5,28 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+LMS: Student Notes: Eventing for Student Notes. TNL-931
+
+LMS: Student Notes: Add course structure view. TNL-762
+
+LMS: Student Notes: Scroll and opening of notes. TNL-784
+
+LMS: Student Notes: Add styling to Notes page. TNL-932
+
+LMS: Student Notes: Add more graceful error message.
+
+LMS: Student Notes: Toggle all notes TNL-661
+
+LMS: Student Notes: Use JWT ID-Token for authentication annotation requests. TNL-782
+
+LMS: Student Notes: Add possibility to search notes. TNL-731
+
+LMS: Student Notes: Toggle single note visibility. TNL-660
+
+LMS: Student Notes: Add Notes page. TNL-797
+
+LMS: Student Notes: Add possibility to add/edit/remove notes. TNL-655
+
 Platform: Add group_access field to all xblocks.  TNL-670
 
 LMS: Add support for user partitioning based on cohort.  TNL-710

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -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 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 @@ def test_advanced_components_munge_tabs(self):
         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):
     """

cms/djangoapps/contentstore/utils.py

@@ -30,7 +30,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):

cms/djangoapps/contentstore/views/course.py

@@ -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):
-    """
-    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:
-
-                        # 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
+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.
+
+    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
-                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_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(

cms/djangoapps/models/settings/course_metadata.py

@@ -47,6 +47,10 @@ def filtered_list(cls):
         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

cms/envs/common.py

@@ -116,6 +116,9 @@
     # for consistency in user-experience, keep the value of this feature flag
     # in sync with the one in lms/envs/common.py
     'IS_EDX_DOMAIN': False,
+
+    # let students save and manage their annotations
+    'ENABLE_EDXNOTES': True,
 }
 ENABLE_JASMINE = False
 

cms/envs/test.py

@@ -229,3 +229,5 @@
 
 # Enable content libraries code for the tests
 FEATURES['ENABLE_CONTENT_LIBRARIES'] = True
+
+FEATURES['ENABLE_EDXNOTES'] = True

cms/static/js/factories/base.js

@@ -1,2 +1,2 @@
-define(['js/base', 'coffee/src/main', 'coffee/src/logger', 'datepair', 'accessibility',
+define(['js/base', 'coffee/src/main', 'js/src/logger', 'datepair', 'accessibility',
 'ieshim', 'tooltip_manager']);

cms/static/require-config.js

@@ -220,7 +220,7 @@ require.config({
         "coffee/src/main": {
             deps: ["coffee/src/ajax_prefix"]
         },
-        "coffee/src/logger": {
+        "js/src/logger": {
             exports: "Logger",
             deps: ["coffee/src/ajax_prefix"]
         },