hide notes tab if feature flag is disabled or harvard notes enabled

TNL-2816

lms/djangoapps/edxnotes/helpers.py

@@ -20,6 +20,7 @@ from django.core.urlresolvers import reverse
 from django.utils.translation import ugettext as _
 
 from edxnotes.exceptions import EdxNotesParseError, EdxNotesServiceUnavailable
+from edxnotes.plugins import EdxNotesTab
 from courseware.views import get_current_child
 from courseware.access import has_access
 from openedx.core.lib.token_utils import get_id_token
@@ -426,26 +427,6 @@ def generate_uid():
 
 def is_feature_enabled(course):
     """
-    Returns True if Student Notes feature is enabled for the course,
-    False otherwise.
-
-    In order for the application to be enabled it must be:
-        1) enabled globally via FEATURES.
-        2) present in the course tab configuration.
-        3) Harvard Annotation Tool must be disabled for the course.
+    Returns True if Student Notes feature is enabled for the course, False otherwise.
     """
-    return (settings.FEATURES.get("ENABLE_EDXNOTES")
-            and [t for t in course.tabs if t["type"] == "edxnotes"]  # tab found
-            and not is_harvard_notes_enabled(course))
-
-
-def is_harvard_notes_enabled(course):
-    """
-    Returns True if Harvard Annotation Tool is enabled for the course,
-    False otherwise.
-
-    Checks for 'textannotation', 'imageannotation', 'videoannotation' in the list
-    of advanced modules of the course.
-    """
-    modules = set(['textannotation', 'imageannotation', 'videoannotation'])
-    return bool(modules.intersection(course.advanced_modules))
+    return EdxNotesTab.is_enabled(course)

lms/djangoapps/edxnotes/plugins.py

@@ -1,9 +1,8 @@
 """
 Registers the "edX Notes" feature for the edX platform.
 """
-
+from django.conf import settings
 from django.utils.translation import ugettext_noop
-
 from courseware.tabs import EnrolledTab
 
 
@@ -27,4 +26,20 @@ class EdxNotesTab(EnrolledTab):
         """
         if not super(EdxNotesTab, cls).is_enabled(course, user=user):
             return False
+
+        if not settings.FEATURES.get("ENABLE_EDXNOTES") or is_harvard_notes_enabled(course):
+            return False
+
         return course.edxnotes
+
+
+def is_harvard_notes_enabled(course):
+    """
+    Returns True if Harvard Annotation Tool is enabled for the course,
+    False otherwise.
+
+    Checks for 'textannotation', 'imageannotation', 'videoannotation' in the list
+    of advanced modules of the course.
+    """
+    modules = set(['textannotation', 'imageannotation', 'videoannotation'])
+    return bool(modules.intersection(course.advanced_modules))

lms/djangoapps/edxnotes/tests.py

@@ -14,6 +14,7 @@ from edxmako.shortcuts import render_to_string
 from edxnotes import helpers
 from edxnotes.decorators import edxnotes
 from edxnotes.exceptions import EdxNotesParseError, EdxNotesServiceUnavailable
+from edxnotes.plugins import EdxNotesTab
 from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.core.exceptions import ImproperlyConfigured
@@ -32,6 +33,8 @@ from courseware.tabs import get_course_tab_list
 from student.tests.factories import UserFactory, CourseEnrollmentFactory
 
 
+FEATURES = settings.FEATURES.copy()
+
 NOTES_API_EMPTY_RESPONSE = {
     "total": 0,
     "rows": [],
@@ -140,6 +143,7 @@ class EdxNotesDecoratorTest(ModuleStoreTestCase):
         Tests that get_html is wrapped when feature flag is on, but edxnotes are
         disabled for the course.
         """
+        self.course.edxnotes = False
         self.assertEqual("original_get_html", self.problem.get_html())
 
     @patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": False})
@@ -227,14 +231,6 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
             'position': position,
         })
 
-    def test_edxnotes_not_enabled(self):
-        """
-        Tests that edxnotes are disabled when the course tab configuration does NOT
-        contain a tab with type "edxnotes."
-        """
-        self.course.tabs = []
-        self.assertFalse(helpers.is_feature_enabled(self.course))
-
     def test_edxnotes_harvard_notes_enabled(self):
         """
         Tests that edxnotes are disabled when Harvard Annotation Tool is enabled.
@@ -251,15 +247,17 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
         self.course.advanced_modules = ["textannotation", "videoannotation", "imageannotation"]
         self.assertFalse(helpers.is_feature_enabled(self.course))
 
-    def test_edxnotes_enabled(self):
+    @ddt.unpack
+    @ddt.data(
+        {'_edxnotes': True},
+        {'_edxnotes': False}
+    )
+    def test_is_feature_enabled(self, _edxnotes):
         """
-        Tests that edxnotes are enabled when the course tab configuration contains
-        a tab with type "edxnotes."
+        Tests that is_feature_enabled shows correct behavior.
         """
-        self.course.tabs = [{"type": "foo"},
-                            {"name": "Notes", "type": "edxnotes"},
-                            {"type": "bar"}]
-        self.assertTrue(helpers.is_feature_enabled(self.course))
+        self.course.edxnotes = _edxnotes
+        self.assertEqual(helpers.is_feature_enabled(self.course), _edxnotes)
 
     @ddt.data(
         helpers.get_public_endpoint,
@@ -1135,3 +1133,50 @@ class EdxNotesViewsTest(ModuleStoreTestCase):
             content_type="application/json",
         )
         self.assertEqual(response.status_code, 400)
+
+
+@skipUnless(settings.FEATURES["ENABLE_EDXNOTES"], "EdxNotes feature needs to be enabled.")
+@ddt.ddt
+class EdxNotesPluginTest(ModuleStoreTestCase):
+    """
+    EdxNotesTab tests.
+    """
+
+    def setUp(self):
+        super(EdxNotesPluginTest, self).setUp()
+        self.course = CourseFactory.create(edxnotes=True)
+        self.user = UserFactory.create(username="ma", email="ma@ma.info", password="edx")
+        CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id)
+
+    def test_edxnotes_tab_with_unauthorized_user(self):
+        """
+        Verify EdxNotesTab visibility when user is unauthroized.
+        """
+        user = UserFactory.create(username="ma1", email="ma1@ma1.info", password="edx")
+        self.assertFalse(EdxNotesTab.is_enabled(self.course, user=user))
+
+    @ddt.unpack
+    @ddt.data(
+        {'enable_edxnotes': False},
+        {'enable_edxnotes': True}
+    )
+    def test_edxnotes_tab_with_feature_flag(self, enable_edxnotes):
+        """
+        Verify EdxNotesTab visibility when ENABLE_EDXNOTES feature flag is enabled/disabled.
+        """
+        FEATURES['ENABLE_EDXNOTES'] = enable_edxnotes
+        with override_settings(FEATURES=FEATURES):
+            self.assertEqual(EdxNotesTab.is_enabled(self.course), enable_edxnotes)
+
+    @ddt.unpack
+    @ddt.data(
+        {'harvard_notes_enabled': False},
+        {'harvard_notes_enabled': True}
+    )
+    def test_edxnotes_tab_with_harvard_notes(self, harvard_notes_enabled):
+        """
+        Verify EdxNotesTab visibility when harvard notes feature is enabled/disabled.
+        """
+        with patch("edxnotes.plugins.is_harvard_notes_enabled") as mock_harvard_notes_enabled:
+            mock_harvard_notes_enabled.return_value = harvard_notes_enabled
+            self.assertEqual(EdxNotesTab.is_enabled(self.course), not harvard_notes_enabled)