Merge pull request #24443 from edx/ddumesnil/hide-dates-tab-unenrolled-aa-219
AA-219: Dates Tab behavior improvements
This commit is contained in:
@@ -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):
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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">
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user