From ea89d2eea4cc1e0b4ea9a60a36c2942442a95e8f Mon Sep 17 00:00:00 2001 From: muhammad-ammar Date: Wed, 3 Feb 2016 14:17:13 +0500 Subject: [PATCH] hide notes tab if feature flag is disabled or harvard notes enabled TNL-2816 --- lms/djangoapps/edxnotes/helpers.py | 25 ++-------- lms/djangoapps/edxnotes/plugins.py | 19 +++++++- lms/djangoapps/edxnotes/tests.py | 75 ++++++++++++++++++++++++------ 3 files changed, 80 insertions(+), 39 deletions(-) diff --git a/lms/djangoapps/edxnotes/helpers.py b/lms/djangoapps/edxnotes/helpers.py index 4fbec324d1..d982f4dc2d 100644 --- a/lms/djangoapps/edxnotes/helpers.py +++ b/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) diff --git a/lms/djangoapps/edxnotes/plugins.py b/lms/djangoapps/edxnotes/plugins.py index e2cdf8a29a..68519fe549 100644 --- a/lms/djangoapps/edxnotes/plugins.py +++ b/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)) diff --git a/lms/djangoapps/edxnotes/tests.py b/lms/djangoapps/edxnotes/tests.py index 6bb53f9731..b1af6997c7 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/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)