From 0a08852600d078eb3d18c0ef7c80410081056d0c Mon Sep 17 00:00:00 2001 From: Dillon Dumesnil Date: Thu, 22 Apr 2021 14:55:26 -0400 Subject: [PATCH] feat: AA-741: Enables the dates tab for all enrolled learners Removes the relative dates flag check as it is no longer necessary to show the dates tab --- .../course_metadata/v1/tests/test_views.py | 4 +-- lms/djangoapps/courseware/tabs.py | 9 +----- lms/djangoapps/courseware/tests/test_tabs.py | 32 +++++++------------ .../courseware_api/tests/test_views.py | 2 +- 4 files changed, 16 insertions(+), 31 deletions(-) diff --git a/lms/djangoapps/course_home_api/course_metadata/v1/tests/test_views.py b/lms/djangoapps/course_home_api/course_metadata/v1/tests/test_views.py index 31699bb24c..359393b054 100644 --- a/lms/djangoapps/course_home_api/course_metadata/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/course_metadata/v1/tests/test_views.py @@ -35,7 +35,7 @@ class CourseHomeMetadataTests(BaseCourseHomeTests): assert response.status_code == 200 assert not response.data.get('is_staff') # 'Course', 'Wiki', 'Progress' tabs - assert len(response.data.get('tabs', [])) == 3 + assert len(response.data.get('tabs', [])) == 4 def test_get_authenticated_staff_user(self): self.client.logout() @@ -51,7 +51,7 @@ class CourseHomeMetadataTests(BaseCourseHomeTests): assert response.data['is_staff'] # This differs for a staff user because they also receive the Instructor tab # 'Course', 'Wiki', 'Progress', and 'Instructor' tabs - assert len(response.data.get('tabs', [])) == 4 + assert len(response.data.get('tabs', [])) == 5 def test_get_unknown_course(self): url = reverse('course-home-course-metadata', args=['course-v1:unknown+course+2T2020']) diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 1c66b4bb56..0a8cc2da5c 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -12,7 +12,7 @@ from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.entrance_exams import user_can_skip_entrance_exam from lms.djangoapps.course_home_api.toggles import course_home_mfe_dates_tab_is_active, course_home_mfe_outline_tab_is_active, course_home_mfe_progress_tab_is_active # lint-amnesty, pylint: disable=line-too-long from openedx.core.lib.course_tabs import CourseTabPluginManager -from openedx.features.course_experience import RELATIVE_DATES_FLAG, DISABLE_UNIFIED_COURSE_TAB_FLAG, default_course_url_name # lint-amnesty, pylint: disable=line-too-long +from openedx.features.course_experience import DISABLE_UNIFIED_COURSE_TAB_FLAG, default_course_url_name from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url from common.djangoapps.student.models import CourseEnrollment from xmodule.tabs import CourseTab, CourseTabList, course_reverse_func_from_name_func, key_checker @@ -342,13 +342,6 @@ class DatesTab(EnrolledTab): tab_dict['link_func'] = link_func super().__init__(tab_dict) - @classmethod - def is_enabled(cls, course, user=None): - """Returns true if this tab is enabled.""" - if not super().is_enabled(course, user=user): - return False - return RELATIVE_DATES_FLAG.is_enabled(course.id) - def get_course_tab_list(user, course): """ diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index 7e76b6bfa5..9857a36e34 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -400,7 +400,7 @@ class EntranceExamsTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, Mi milestone ) course_tab_list = get_course_tab_list(self.user, self.course) - assert len(course_tab_list) == 1 + assert len(course_tab_list) == 2 assert course_tab_list[0]['tab_id'] == 'courseware' assert course_tab_list[0]['name'] == 'Entrance Exam' @@ -425,7 +425,7 @@ class EntranceExamsTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, Mi self.client.logout() self.login(self.email, self.password) course_tab_list = get_course_tab_list(self.user, self.course) - assert len(course_tab_list) == 4 + assert len(course_tab_list) == 5 def test_course_tabs_list_for_staff_members(self): """ @@ -437,7 +437,7 @@ class EntranceExamsTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, Mi staff_user = StaffFactory(course_key=self.course.id) self.client.login(username=staff_user.username, password='test') course_tab_list = get_course_tab_list(staff_user, self.course) - assert len(course_tab_list) == 4 + assert len(course_tab_list) == 5 class TextBookCourseViewsTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase): @@ -652,16 +652,13 @@ class CourseTabListTestCase(TabListTestCase): # enumerate the tabs with a staff user user = UserFactory(is_staff=True) CourseEnrollment.enroll(user, self.course.id) - # Need to mock this flag as we care that orders match, and a tab not enabled will result in a failure - with patch('lms.djangoapps.courseware.tabs.RELATIVE_DATES_FLAG') as mock_flag: - mock_flag.is_enabled().return_value = True - for i, tab in enumerate(xmodule_tabs.CourseTabList.iterate_displayable(self.course, user=user)): - if getattr(tab, 'is_collection_item', False): - # a collection item was found as a result of a collection tab - assert getattr(self.course.tabs[i], 'is_collection', False) - else: - # all other tabs must match the expected type - assert tab.type == self.course.tabs[i].type + for i, tab in enumerate(xmodule_tabs.CourseTabList.iterate_displayable(self.course, user=user)): + if getattr(tab, 'is_collection_item', False): + # a collection item was found as a result of a collection tab + assert getattr(self.course.tabs[i], 'is_collection', False) + else: + # all other tabs must match the expected type + assert tab.type == self.course.tabs[i].type # test including non-empty collections assert {'type': 'html_textbooks'} in\ @@ -894,11 +891,8 @@ class DiscussionLinkTestCase(TabTestCase): class DatesTabTestCase(TabListTestCase): """Test cases for dates tab""" - - @patch('lms.djangoapps.courseware.tabs.RELATIVE_DATES_FLAG') @patch('common.djangoapps.student.models.CourseEnrollment.is_enrolled') - def test_dates_tab_disabled_if_unenrolled(self, is_enrolled, mock_flag): - mock_flag.is_enabled().return_value = True + def test_dates_tab_disabled_if_unenrolled(self, is_enrolled): tab = DatesTab({'type': DatesTab.type, 'name': 'dates'}) is_enrolled.return_value = False @@ -912,10 +906,8 @@ class DatesTabTestCase(TabListTestCase): enrolled_user = self.create_mock_user(is_staff=False, is_enrolled=True) assert self.is_tab_enabled(tab, self.course, enrolled_user) - @patch('lms.djangoapps.courseware.tabs.RELATIVE_DATES_FLAG') - def test_singular_dates_tab(self, mock_flag): + def test_singular_dates_tab(self): """Test cases for making sure no persisted dates tab is surfaced""" - mock_flag.is_enabled().return_value = True user = self.create_mock_user() self.course.tabs = self.all_valid_tab_list self.course.save() diff --git a/openedx/core/djangoapps/courseware_api/tests/test_views.py b/openedx/core/djangoapps/courseware_api/tests/test_views.py index 9976a7d6d8..32b6501082 100644 --- a/openedx/core/djangoapps/courseware_api/tests/test_views.py +++ b/openedx/core/djangoapps/courseware_api/tests/test_views.py @@ -139,7 +139,7 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): enrollment = response.data['enrollment'] assert enrollment_mode == enrollment['mode'] assert enrollment['is_active'] - assert len(response.data['tabs']) == 5 + assert len(response.data['tabs']) == 6 found = False for tab in response.data['tabs']: if tab['type'] == 'external_link':