AA-219: Dates Tab behavior improvements

This switches the Dates Tab to be an enrolled tab allowing only enrolled learners to view. Additionally, it will now redirect logged out learners to the login page if they hit the Dates Tab directly.
2020-07-09 13:59:57 -07:00
parent 0e2b612c1f
commit be346499da
7 changed files with 98 additions and 37 deletions
--- 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):
--- 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</a>',
+        ]
+        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</a>',
-            ]
            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
--- 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)
--- 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):
--- 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.
--- 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
-    <div class="dates-tab-link">
-        <a href="${dates_tab_link}">View all course dates</a>
-    </div>
+    % if user_enrolled:
+        <div class="dates-tab-link">
+            <a href="${dates_tab_link}">View all course dates</a>
+        </div>
+    % endif
 % endif

 <%static:require_module_async module_name="js/dateutil_factory" class_name="DateUtilFactory">
--- 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)