From 8c580251a1ce8be7ff04277a3952e353b1f68f83 Mon Sep 17 00:00:00 2001 From: polesye Date: Mon, 22 Dec 2014 11:35:30 +0200 Subject: [PATCH] TNL-930: Turn off Student Notes when HAT is enabled. --- .../xmodule/modulestore/inheritance.py | 9 +++--- lms/djangoapps/edxnotes/decorators.py | 6 ++-- lms/djangoapps/edxnotes/helpers.py | 20 +++++++++++-- lms/djangoapps/edxnotes/tests.py | 30 +++++++++++++++++-- 4 files changed, 53 insertions(+), 12 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/inheritance.py b/common/lib/xmodule/xmodule/modulestore/inheritance.py index 23654a16fc02..486e2ee61047 100644 --- a/common/lib/xmodule/xmodule/modulestore/inheritance.py +++ b/common/lib/xmodule/xmodule/modulestore/inheritance.py @@ -173,14 +173,15 @@ class InheritanceMixin(XBlockMixin): default=default_reset_button ) edxnotes = Boolean( - display_name=_("Enable Notes"), - help=_("Enter true or false. If true, you can use the Notes for HTML components."), + display_name=_("Enable Student Notes"), + help=_("Enter true or false. If true, students can use the Student Notes feature."), default=False, scope=Scope.settings ) edxnotes_visibility = Boolean( - display_name=_("Enable visibility of Notes"), - help=_("Enter true or false. If true, Notes for HTML components will be visible."), + display_name="Student Notes Visibility", + help=_("Indicates whether Student Notes are visible in the course. " + "Students can also show or hide their notes in the courseware."), default=True, scope=Scope.user_info ) diff --git a/lms/djangoapps/edxnotes/decorators.py b/lms/djangoapps/edxnotes/decorators.py index a9eb466ef841..aadeff8a1258 100644 --- a/lms/djangoapps/edxnotes/decorators.py +++ b/lms/djangoapps/edxnotes/decorators.py @@ -26,8 +26,10 @@ def get_html(self, *args, **kwargs): is_studio = getattr(self.system, "is_author_mode", False) course = self.descriptor.runtime.modulestore.get_course(self.runtime.course_id) - # Must be disabled in Studio or depends on the feature flag/advanced - # settings of the course. + # Must be disabled: + # - in Studio; + # - when Harvard Annotation Tool is enabled for the course; + # - when the feature flag or `edxnotes` setting of the course is set to False. if is_studio or not is_feature_enabled(course): return original_get_html(self, *args, **kwargs) else: diff --git a/lms/djangoapps/edxnotes/helpers.py b/lms/djangoapps/edxnotes/helpers.py index 21015249800f..b69517cfea2a 100644 --- a/lms/djangoapps/edxnotes/helpers.py +++ b/lms/djangoapps/edxnotes/helpers.py @@ -323,13 +323,27 @@ def generate_uid(): def is_feature_enabled(course): """ - Returns True if the edxnotes app is enabled for the course, False otherwise. + Returns True if Student Notes feature is enabled for the course, + False otherwise. - In order for the app to be enabled it must be: + 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. """ tab_found = next((True for t in course.tabs if t["type"] == "edxnotes"), False) feature_enabled = settings.FEATURES.get("ENABLE_EDXNOTES") - return feature_enabled and tab_found + return (feature_enabled and 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)) diff --git a/lms/djangoapps/edxnotes/tests.py b/lms/djangoapps/edxnotes/tests.py index 5f9918152bbd..9e5f7b485d90 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/lms/djangoapps/edxnotes/tests.py @@ -104,7 +104,7 @@ def test_edxnotes_enabled(self, mock_generate_uid, mock_get_id_token, mock_get_t @patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": True}) def test_edxnotes_disabled_if_edxnotes_flag_is_false(self): """ - Tests if get_html is wrapped when feature flag is on, but edxnotes are + Tests that get_html is wrapped when feature flag is on, but edxnotes are disabled for the course. """ self.assertEqual("original_get_html", self.problem.get_html()) @@ -112,17 +112,25 @@ def test_edxnotes_disabled_if_edxnotes_flag_is_false(self): @patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": False}) def test_edxnotes_disabled(self): """ - Tests if get_html is not wrapped when feature flag is off. + Tests that get_html is not wrapped when feature flag is off. """ self.assertEqual("original_get_html", self.problem.get_html()) def test_edxnotes_studio(self): """ - Tests if get_html is not wrapped when problem is rendered in Studio. + Tests that get_html is not wrapped when problem is rendered in Studio. """ self.problem.system.is_author_mode = True self.assertEqual("original_get_html", self.problem.get_html()) + def test_edxnotes_harvard_notes_enabled(self): + """ + Tests that get_html is not wrapped when Harvard Annotation Tool is enabled. + """ + self.course.advanced_modules = ["videoannotation", "imageannotation", "textannotation"] + enable_edxnotes_for_the_course(self.course, self.user.id) + self.assertEqual("original_get_html", self.problem.get_html()) + @skipUnless(settings.FEATURES["ENABLE_EDXNOTES"], "EdxNotes feature needs to be enabled.") class EdxNotesHelpersTest(ModuleStoreTestCase): @@ -179,6 +187,22 @@ def test_edxnotes_not_enabled(self): 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. + """ + self.course.advanced_modules = ["foo", "imageannotation", "boo"] + self.assertFalse(helpers.is_feature_enabled(self.course)) + + self.course.advanced_modules = ["foo", "boo", "videoannotation"] + self.assertFalse(helpers.is_feature_enabled(self.course)) + + self.course.advanced_modules = ["textannotation", "foo", "boo"] + self.assertFalse(helpers.is_feature_enabled(self.course)) + + self.course.advanced_modules = ["textannotation", "videoannotation", "imageannotation"] + self.assertFalse(helpers.is_feature_enabled(self.course)) + def test_edxnotes_enabled(self): """ Tests that edxnotes are enabled when the course tab configuration contains