From d4cd3bd611f0a22c504f5ead6ee8b2c2f1743902 Mon Sep 17 00:00:00 2001 From: Kshitij Sobti Date: Tue, 26 Oct 2021 09:57:19 +0000 Subject: [PATCH] fix: enabling the notes tool using the course authoring will now add the notes tab (#29093) If an existing course doesn't already have the notes tab, enabling notes will not make it show up. This change fixes this by adding the tab in case it isn't already in the course. --- lms/djangoapps/edxnotes/plugins.py | 7 +++ .../course_apps/tests/test_notes_app.py | 26 +++++++++ .../course_apps/tests/test_wiki_app.py | 49 +++------------- .../djangoapps/course_apps/tests/utils.py | 58 +++++++++++++++++++ 4 files changed, 99 insertions(+), 41 deletions(-) create mode 100644 openedx/core/djangoapps/course_apps/tests/test_notes_app.py diff --git a/lms/djangoapps/edxnotes/plugins.py b/lms/djangoapps/edxnotes/plugins.py index 0efec84952..908ff91d52 100644 --- a/lms/djangoapps/edxnotes/plugins.py +++ b/lms/djangoapps/edxnotes/plugins.py @@ -13,6 +13,7 @@ from lms.djangoapps.courseware.tabs import EnrolledTab from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.course_apps.plugins import CourseApp from openedx.core.lib.courses import get_course_by_id +from xmodule.tabs import CourseTab, CourseTabList User = get_user_model() @@ -79,6 +80,12 @@ class EdxNotesCourseApp(CourseApp): """ course = get_course_by_id(course_key) course.edxnotes = enabled + if enabled: + notes_tab = CourseTabList.get_tab_by_id(course.tabs, 'edxnotes') + if notes_tab is None: + # If the course doesn't already have the notes tab, add it. + notes_tab = CourseTab.load("edxnotes") + course.tabs.append(notes_tab) modulestore().update_item(course, user.id) return enabled diff --git a/openedx/core/djangoapps/course_apps/tests/test_notes_app.py b/openedx/core/djangoapps/course_apps/tests/test_notes_app.py new file mode 100644 index 0000000000..79503ebf84 --- /dev/null +++ b/openedx/core/djangoapps/course_apps/tests/test_notes_app.py @@ -0,0 +1,26 @@ +""" +Tests for wiki course app. +""" +from unittest.mock import patch + +from django.conf import settings + +from lms.djangoapps.edxnotes.plugins import EdxNotesCourseApp +from openedx.core.djangoapps.course_apps.tests.utils import TabBasedCourseAppTestMixin +from openedx.core.djangolib.testing.utils import skip_unless_cms +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +@skip_unless_cms +@patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': True}) +class NotesCourseAppTestCase(TabBasedCourseAppTestMixin, ModuleStoreTestCase): + """Test cases for Notes CourseApp.""" + + tab_type = 'edxnotes' + course_app_class = EdxNotesCourseApp + + def _assert_app_enabled(self, app_tab): + assert app_tab.is_enabled(self.course, self.user) + + def _assert_app_disabled(self, app_tab): + assert not app_tab.is_enabled(self.course, self.user) diff --git a/openedx/core/djangoapps/course_apps/tests/test_wiki_app.py b/openedx/core/djangoapps/course_apps/tests/test_wiki_app.py index 6c7228a348..5e3d56a2bf 100644 --- a/openedx/core/djangoapps/course_apps/tests/test_wiki_app.py +++ b/openedx/core/djangoapps/course_apps/tests/test_wiki_app.py @@ -2,54 +2,21 @@ Tests for wiki course app. """ -from common.djangoapps.student.tests.factories import AdminFactory, UserFactory from lms.djangoapps.course_wiki.plugins.course_app import WikiCourseApp +from openedx.core.djangoapps.course_apps.tests.utils import TabBasedCourseAppTestMixin from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory @skip_unless_cms -class WikiCourseAppTestCase(ModuleStoreTestCase): +class WikiCourseAppTestCase(TabBasedCourseAppTestMixin, ModuleStoreTestCase): """Test cases for Wiki CourseApp.""" - def setUp(self): - super().setUp() - self.course = CourseFactory.create() - self.instructor = AdminFactory.create() - self.user = UserFactory() + tab_type = 'wiki' + course_app_class = WikiCourseApp - def get_wiki_tab(self, course_key): - """ - Reload the course and fetch the wiki tab if present. - """ - course = self.store.get_course(course_key) - return next((tab for tab in course.tabs if tab.type == 'wiki'), None) + def _assert_app_enabled(self, app_tab): + assert not app_tab.is_hidden - def test_app_disabled_by_default(self): - """ - Test that the wiki tab is disabled by default. - """ - assert not WikiCourseApp.is_enabled(self.course.id) - - def test_app_enabling(self): - """ - Test that enabling and disable the app enabled/disables the tab. - """ - WikiCourseApp.set_enabled(self.course.id, True, self.instructor) - wiki_tab = self.get_wiki_tab(self.course.id) - assert not wiki_tab.is_hidden - WikiCourseApp.set_enabled(self.course.id, False, self.instructor) - wiki_tab = self.get_wiki_tab(self.course.id) - assert wiki_tab.is_hidden - - def test_app_adds_wiki(self): - """ - Test that enabling the app for a course that doesn't have the wiki tab - adds the wiki tab. - """ - self.course.tabs = [tab for tab in self.course.tabs if tab.type != 'wiki'] - self.store.update_item(self.course, self.instructor.id) - assert self.get_wiki_tab(self.course.id) is None - WikiCourseApp.set_enabled(self.course.id, True, self.instructor) - assert self.get_wiki_tab(self.course.id) is not None + def _assert_app_disabled(self, app_tab): + assert app_tab.is_hidden diff --git a/openedx/core/djangoapps/course_apps/tests/utils.py b/openedx/core/djangoapps/course_apps/tests/utils.py index 49bae28c21..60d55e705d 100644 --- a/openedx/core/djangoapps/course_apps/tests/utils.py +++ b/openedx/core/djangoapps/course_apps/tests/utils.py @@ -5,7 +5,11 @@ from typing import Type from opaque_keys.edx.keys import CourseKey +from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory from openedx.core.djangoapps.course_apps.plugins import CourseApp +from openedx.core.djangolib.testing.utils import skip_unless_cms +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.factories import CourseFactory def make_test_course_app( @@ -55,3 +59,57 @@ def make_test_course_app( TestCourseApp.name = name TestCourseApp.description = description return TestCourseApp + + +@skip_unless_cms +class TabBasedCourseAppTestMixin: + """Test cases a course app adding/removing tabs CourseApp.""" + + tab_type = None + course_app_class = None + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + self.instructor = AdminFactory.create() + self.user = UserFactory() + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id) + + def reload_course(self): + self.course = modulestore().get_course(self.course.id) + + def get_app_tab(self, course_key): + """ + Reload the course and fetch the app tab if present. + """ + course = self.store.get_course(course_key) + return next((tab for tab in course.tabs if tab.type == self.tab_type), None) + + def test_app_disabled_by_default(self): + """ + Test that the app tab is disabled by default. + """ + assert not self.course_app_class.is_enabled(self.course.id) + + def test_app_enabling(self): + """ + Test that enabling and disable the app enabled/disables the tab. + """ + self.course_app_class.set_enabled(self.course.id, True, self.instructor) + self.reload_course() + app_tab = self.get_app_tab(self.course.id) + self._assert_app_enabled(app_tab) + self.course_app_class.set_enabled(self.course.id, False, self.instructor) + self.reload_course() + app_tab = self.get_app_tab(self.course.id) + self._assert_app_disabled(app_tab) + + def test_app_adds_tab(self): + """ + Test that enabling the app for a course that doesn't have the app tab adds the tab. + """ + self.course.tabs = [tab for tab in self.course.tabs if tab.type != self.tab_type] + self.store.update_item(self.course, self.instructor.id) + assert self.get_app_tab(self.course.id) is None + self.course_app_class.set_enabled(self.course.id, True, self.instructor) + assert self.get_app_tab(self.course.id) is not None