diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 660d99be69..57962266b5 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -13,7 +13,6 @@ 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 from lms.djangoapps.course_home_api.utils import get_microfrontend_url -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.course_tabs import CourseTabPluginManager from openedx.features.course_experience import RELATIVE_DATES_FLAG, UNIFIED_COURSE_TAB_FLAG, default_course_url_name from student.models import CourseEnrollment @@ -205,7 +204,7 @@ class LinkTab(CourseTab): """ link_value = '' - def __init__(self, tab_dict=None, name=None, link=None): + def __init__(self, tab_dict=None, link=None): self.link_value = tab_dict['link'] if tab_dict else link def link_value_func(_course, _reverse_func): diff --git a/lms/djangoapps/courseware/tests/test_date_summary.py b/lms/djangoapps/courseware/tests/test_date_summary.py index 7ea8e94914..36e821a739 100644 --- a/lms/djangoapps/courseware/tests/test_date_summary.py +++ b/lms/djangoapps/courseware/tests/test_date_summary.py @@ -663,18 +663,33 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): ) @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=True) def test_dates_tab_link_render(self, url_name): - with freeze_time('2015-01-02'): - course = create_course_run() - user = create_user() + """ The dates tab link should only show for enrolled or staff users """ + course = create_course_run() + html_elements = [ + 'class="dates-tab-link"', + 'View all course dates', + ] + url = reverse(url_name, args=(course.id,)) + + def assert_html_elements(assert_function, user): self.client.login(username=user.username, password=TEST_PASSWORD) - url = reverse(url_name, args=(course.id,)) response = self.client.get(url, follow=True) - html_elements = [ - 'class="dates-tab-link"', - 'View all course dates', - ] for html in html_elements: - self.assertContains(response, html) + assert_function(response, html) + self.client.logout() + + with freeze_time('2015-01-02'): + unenrolled_user = create_user() + assert_html_elements(self.assertNotContains, unenrolled_user) + + staff_user = create_user() + staff_user.is_staff = True + staff_user.save() + assert_html_elements(self.assertContains, staff_user) + + enrolled_user = create_user() + CourseEnrollmentFactory(course_id=course.id, user=enrolled_user, mode=CourseMode.VERIFIED) + assert_html_elements(self.assertContains, enrolled_user) @ddt.ddt diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index b5d621beff..fa4c88826b 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -164,7 +164,6 @@ class TabTestCase(SharedModuleStoreTestCase): if for_authenticated_users_only: user = self.create_mock_user(is_staff=False, is_enrolled=False) self.assertEqual(expected_value, self.is_tab_enabled(tab, self.course, user)) - assert False if not for_staff_only and not for_authenticated_users_only and not for_enrolled_users_only: user = AnonymousUser() self.assertEqual(expected_value, self.is_tab_enabled(tab, self.course, user)) @@ -903,9 +902,29 @@ class DiscussionLinkTestCase(TabTestCase): class DatesTabTestCase(TabListTestCase): - """Test cases for making sure no persisted dates tab is surfaced""" + """Test cases for dates tab""" - def test_singular_dates_tab(self): + @patch('lms.djangoapps.courseware.tabs.RELATIVE_DATES_FLAG') + @patch('student.models.CourseEnrollment.is_enrolled') + def test_dates_tab_disabled_if_unenrolled(self, is_enrolled, mock_flag): + mock_flag.is_enabled().return_value = True + tab = DatesTab({'type': DatesTab.type, 'name': 'dates'}) + + is_enrolled.return_value = False + unenrolled_user = self.create_mock_user(is_staff=False, is_enrolled=False) + self.assertFalse(self.is_tab_enabled(tab, self.course, unenrolled_user)) + + staff_user = self.create_mock_user(is_staff=True, is_enrolled=False) + self.assertTrue(self.is_tab_enabled(tab, self.course, staff_user)) + + is_enrolled.return_value = True + enrolled_user = self.create_mock_user(is_staff=False, is_enrolled=True) + self.assertTrue(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): + """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() @@ -918,11 +937,9 @@ class DatesTabTestCase(TabListTestCase): self.assertTrue(has_dates_tab) # Verify that there is only 1 'dates' tab in the returned result from get_course_tab_list() - with patch('lms.djangoapps.courseware.tabs.RELATIVE_DATES_FLAG') as mock_flag: - mock_flag.is_enabled().return_value = True - tabs = get_course_tab_list(user, self.course) - num_dates_tabs = 0 - for tab in tabs: - if tab.type == 'dates': - num_dates_tabs += 1 - self.assertEqual(num_dates_tabs, 1) + tabs = get_course_tab_list(user, self.course) + num_dates_tabs = 0 + for tab in tabs: + if tab.type == 'dates': + num_dates_tabs += 1 + self.assertEqual(num_dates_tabs, 1) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index fa29d0b48d..f70d8f6f6e 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -3098,9 +3098,37 @@ class DatesTabTestCase(ModuleStoreTestCase): ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2017, 1, 1)) def _get_response(self, course): - """ Returns the HTML for the progress page """ + """ Returns the HTML for the dates page """ return self.client.get(reverse('dates', args=[six.text_type(course.id)])) + def test_tab_redirects_if_not_logged_in(self): + self.client.logout() + response = self._get_response(self.course) + self.assertEqual(response.status_code, 302) + self.assertIn('/login?next=/courses/', response.url) + + def test_tab_redirects_if_not_enrolled_and_not_staff(self): + response = self._get_response(self.course) + self.assertEqual(response.status_code, 302) + # Beginning of redirect URL + self.assertIn('/courses/', response.url) + # End of redirect URL + self.assertIn('/course/', response.url) + + # Now check staff users can see + self.user.is_staff = True + self.user.save() + response = self._get_response(self.course) + self.assertEqual(response.status_code, 200) + + # Enrolled users can also see + self.client.logout() + enrolled_user = UserFactory() + CourseEnrollmentFactory(course_id=self.course.id, user=enrolled_user, mode=CourseMode.VERIFIED) + self.client.login(username=enrolled_user.username, password=TEST_PASSWORD) + response = self._get_response(self.course) + self.assertEqual(response.status_code, 200) + @RELATIVE_DATES_FLAG.override(active=True) @patch('edx_django_utils.monitoring.set_custom_metric') def test_defaults(self, mock_set_custom_metric): diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 4858c19dc5..b795c0e6c8 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -21,7 +21,6 @@ from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpRespo from django.shortcuts import redirect from django.template.context_processors import csrf from django.urls import reverse -from django.utils import timezone from django.utils.decorators import method_decorator from django.utils.http import urlquote_plus from django.utils.text import slugify @@ -111,12 +110,7 @@ from openedx.core.djangolib.markup import HTML, Text from openedx.core.lib.mobile_utils import is_request_from_mobile_app from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.course_duration_limits.access import generate_course_expired_fragment -from openedx.features.course_experience import ( - COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, - RELATIVE_DATES_FLAG, - UNIFIED_COURSE_TAB_FLAG, - course_home_url_name -) +from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG, course_home_url_name from openedx.features.course_experience.course_tools import CourseToolsPluginManager from openedx.features.course_experience.utils import dates_banner_should_display from openedx.features.course_experience.views.course_dates import CourseDatesFragmentView @@ -735,9 +729,6 @@ class CourseTabView(EdxFragmentView): """ Creates the context for the fragment's template. """ - from lms.urls import RESET_COURSE_DEADLINES_NAME - from openedx.features.course_experience.urls import COURSE_HOME_VIEW_NAME - can_masquerade = request.user.has_perm(MASQUERADE_AS_STUDENT, course) supports_preview_menu = tab.get('supports_preview_menu', False) uses_bootstrap = self.uses_bootstrap(request, course, tab=tab) @@ -1031,6 +1022,7 @@ def program_marketing(request, program_uuid): return render_to_response('courseware/program_marketing.html', context) +@login_required @ensure_csrf_cookie @ensure_valid_course_key def dates(request, course_id): @@ -1061,7 +1053,7 @@ def dates(request, course_id): request.user = masquerade_user user_is_enrolled = CourseEnrollment.is_enrolled(request.user, course_key) - user_is_staff = has_access(request.user, 'staff', course_key) + user_is_staff = bool(has_access(request.user, 'staff', course_key)) # Render the full content to enrolled users, as well as to course and global staff. # Unenrolled users who are not course or global staff are redirected to the Outline Tab. diff --git a/openedx/features/course_experience/templates/course_experience/course-dates-fragment.html b/openedx/features/course_experience/templates/course_experience/course-dates-fragment.html index 2ca2de0154..5787fde1e0 100644 --- a/openedx/features/course_experience/templates/course_experience/course-dates-fragment.html +++ b/openedx/features/course_experience/templates/course_experience/course-dates-fragment.html @@ -13,9 +13,11 @@ from django.utils.translation import ugettext as _ % for course_date_block in course_date_blocks: <%include file="dates-summary.html" args="course_date=course_date_block" /> % endfor - + % if user_enrolled: + + % endif % endif <%static:require_module_async module_name="js/dateutil_factory" class_name="DateUtilFactory"> diff --git a/openedx/features/course_experience/views/course_dates.py b/openedx/features/course_experience/views/course_dates.py index f241ba8a83..3fc0ac270c 100644 --- a/openedx/features/course_experience/views/course_dates.py +++ b/openedx/features/course_experience/views/course_dates.py @@ -10,8 +10,10 @@ from django.utils.translation import get_language_bidi from opaque_keys.edx.keys import CourseKey from web_fragments.fragment import Fragment +from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.courses import get_course_date_blocks, get_course_with_access from openedx.core.djangoapps.plugin_api.views import EdxFragmentView +from student.models import CourseEnrollment class CourseDatesFragmentView(EdxFragmentView): @@ -27,10 +29,16 @@ class CourseDatesFragmentView(EdxFragmentView): course_key = CourseKey.from_string(course_id) course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=False) course_date_blocks = get_course_date_blocks(course, request.user, request, num_assignments=1) + # We will use this boolean to gate if we show a link to the dates tab. This same logic + # dictates if we show the tab at all. + user_enrolled = (request.user and request.user.is_authenticated and + (bool(CourseEnrollment.is_enrolled(request.user, course.id) or + has_access(request.user, 'staff', course, course.id)))) context = { 'course_date_blocks': [block for block in course_date_blocks if block.title != 'current_datetime'], 'dates_tab_link': reverse('dates', args=[course.id]), + 'user_enrolled': user_enrolled, } html = render_to_string(self.template_name, context) dates_fragment = Fragment(html)