From f7d0753210b13b5a692e3d9df663f13176cd69ff Mon Sep 17 00:00:00 2001 From: Michael Roytman Date: Tue, 9 Apr 2019 17:46:52 -0400 Subject: [PATCH] remove enroll CTA on course homepage for logged out users visiting a Master's only course --- common/djangoapps/course_modes/models.py | 26 ++++++++++ .../course_modes/tests/test_models.py | 27 ++++++++++ lms/djangoapps/courseware/tests/test_views.py | 52 ++++++++++++++++++- lms/djangoapps/courseware/views/views.py | 8 ++- .../course_experience/tests/views/helpers.py | 32 ++++++++++-- .../tests/views/test_course_home.py | 30 ++++++++++- .../views/course_home_messages.py | 20 +++++-- 7 files changed, 184 insertions(+), 11 deletions(-) diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index 4046877c15..11a97b24ea 100644 --- a/common/djangoapps/course_modes/models.py +++ b/common/djangoapps/course_modes/models.py @@ -550,6 +550,32 @@ class CourseMode(models.Model): """ return slug in [cls.PROFESSIONAL, cls.NO_ID_PROFESSIONAL_MODE] + @classmethod + def contains_masters_mode(cls, modes_dict): + """ + Check whether the modes_dict contains a Master's mode. + + Args: + modes_dict (dict): a dict of course modes + + Returns: + bool: whether modes_dict contains a Master's mode + """ + return cls.MASTERS in modes_dict + + @classmethod + def is_masters_only(cls, course_id): + """ + Check whether the course contains only a Master's mode. + + Args: + course_id (CourseKey): course key of course to check + + Returns: bool: whether the course contains only a Master's mode + """ + modes = cls.modes_for_course_dict(course_id) + return cls.contains_masters_mode(modes) and len(modes) == 1 + @classmethod def is_mode_upgradeable(cls, mode_slug): """ diff --git a/common/djangoapps/course_modes/tests/test_models.py b/common/djangoapps/course_modes/tests/test_models.py index 596c2fbd42..6055be65e8 100644 --- a/common/djangoapps/course_modes/tests/test_models.py +++ b/common/djangoapps/course_modes/tests/test_models.py @@ -486,6 +486,33 @@ class CourseModeModelTest(TestCase): else: self.assertFalse(is_error_expected, "Expected a ValidationError to be thrown.") + @ddt.data( + ([], False), + ([CourseMode.VERIFIED, CourseMode.AUDIT], False), + ([CourseMode.MASTERS], True), + ([CourseMode.VERIFIED, CourseMode.AUDIT, CourseMode.MASTERS], True) + ) + @ddt.unpack + def test_contains_masters_mode(self, available_modes, expected_contains_masters_mode): + for mode in available_modes: + self.create_mode(mode, mode, 10) + + modes = CourseMode.modes_for_course_dict(self.course_key) + self.assertEqual(CourseMode.contains_masters_mode(modes), expected_contains_masters_mode) + + @ddt.data( + ([], False), + ([CourseMode.VERIFIED, CourseMode.AUDIT], False), + ([CourseMode.MASTERS], True), + ([CourseMode.VERIFIED, CourseMode.AUDIT, CourseMode.MASTERS], False) + ) + @ddt.unpack + def test_is_masters_only(self, available_modes, expected_is_masters_only): + for mode in available_modes: + self.create_mode(mode, mode, 10) + + self.assertEqual(CourseMode.is_masters_only(self.course_key), expected_is_masters_only) + class TestDisplayPrices(ModuleStoreTestCase): @override_settings(PAID_COURSE_REGISTRATION_CURRENCY=["USD", "$"]) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index a020a5af79..9671f7c3df 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -18,7 +18,7 @@ from django.conf import settings from django.contrib.auth.models import AnonymousUser from django.urls import reverse, reverse_lazy from django.http import Http404, HttpResponseBadRequest -from django.test import TestCase +from django.test import TestCase, RequestFactory from django.test.client import Client from django.test.utils import override_settings from freezegun import freeze_time @@ -2517,6 +2517,56 @@ class TestIndexView(ModuleStoreTestCase): self.assertIn('xblock-student_view-html', response.content) self.assertIn('xblock-student_view-video', response.content) + @patch('openedx.core.djangoapps.util.user_messages.PageLevelMessages.register_warning_message') + def test_courseware_messages_masters_only(self, patch_register_warning_message): + with patch('courseware.views.views.CourseTabView.should_show_enroll_button') as patch_should_show_enroll_button: + course = CourseFactory() + + user = self.create_user_for_course(course, CourseUserType.UNENROLLED) + request = RequestFactory().get('/') + request.user = user + + button_html = '' + + patch_should_show_enroll_button.return_value = False + views.CourseTabView.register_user_access_warning_messages(request, course) + # pull message out of the calls to the mock so that + # we can make finer grained assertions than mock provides + message = patch_register_warning_message.mock_calls[0][1][1] + assert button_html not in message + + patch_register_warning_message.reset_mock() + + patch_should_show_enroll_button.return_value = True + views.CourseTabView.register_user_access_warning_messages(request, course) + # pull message out of the calls to the mock so that + # we can make finer grained assertions than mock provides + message = patch_register_warning_message.mock_calls[0][1][1] + assert button_html in message + + @ddt.data( + [True, True, True, False, ], + [False, True, True, False, ], + [True, False, True, False, ], + [True, True, False, False, ], + [False, False, True, False, ], + [True, False, False, True, ], + [False, True, False, False, ], + [False, False, False, False, ], + ) + @ddt.unpack + def test_should_show_enroll_button(self, course_open_for_self_enrollment, + invitation_only, is_masters_only, expected_should_show_enroll_button): + with patch('courseware.views.views.course_open_for_self_enrollment') as patch_course_open_for_self_enrollment, \ + patch('course_modes.models.CourseMode.is_masters_only') as patch_is_masters_only: + course = CourseFactory() + + patch_course_open_for_self_enrollment.return_value = course_open_for_self_enrollment + patch_is_masters_only.return_value = is_masters_only + course.invitation_only = invitation_only + + self.assertEqual(views.CourseTabView.should_show_enroll_button(course), expected_should_show_enroll_button) + @ddt.ddt class TestIndexViewCompleteOnView(ModuleStoreTestCase, CompletionWaffleTestMixin): diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index c8ec728af5..dd8b73a9b1 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -551,7 +551,7 @@ class CourseTabView(EdxFragmentView): else: if not CourseEnrollment.is_enrolled(request.user, course.id) and not allow_anonymous: # Only show enroll button if course is open for enrollment. - if course_open_for_self_enrollment(course.id) and not course.invitation_only: + if CourseTabView.should_show_enroll_button(course): enroll_message = _(u'You must be enrolled in the course to see course content. \ {enroll_link_start}Enroll now{enroll_link_end}.') PageLevelMessages.register_warning_message( @@ -567,6 +567,12 @@ class CourseTabView(EdxFragmentView): Text(_('You must be enrolled in the course to see course content.')) ) + @staticmethod + def should_show_enroll_button(course): + return (course_open_for_self_enrollment(course.id) + and not course.invitation_only + and not CourseMode.is_masters_only(course.id)) + @staticmethod def handle_exceptions(request, course_key, course, exception): u""" diff --git a/openedx/features/course_experience/tests/views/helpers.py b/openedx/features/course_experience/tests/views/helpers.py index 24922add83..435d562862 100644 --- a/openedx/features/course_experience/tests/views/helpers.py +++ b/openedx/features/course_experience/tests/views/helpers.py @@ -3,6 +3,7 @@ Test helpers for the course experience. """ from datetime import timedelta +from django.core.exceptions import ObjectDoesNotExist from django.utils.timezone import now from course_modes.models import CourseMode @@ -10,9 +11,16 @@ from course_modes.models import CourseMode TEST_COURSE_PRICE = 50 -def add_course_mode(course, upgrade_deadline_expired=False): +def add_course_mode(course, mode_slug=CourseMode.VERIFIED, + mode_display_name='Verified Certificate', upgrade_deadline_expired=False): """ - Adds a course mode to the test course. + Add a course mode to the test course. + + Args: + course + mode_slug (str): the slug of the mode to add + mode_display_name (str): the display name of the mode to add + upgrade_deadline_expired (bool): whether the upgrade deadline has passed """ upgrade_exp_date = now() if upgrade_deadline_expired: @@ -22,8 +30,24 @@ def add_course_mode(course, upgrade_deadline_expired=False): CourseMode( course_id=course.id, - mode_slug=CourseMode.VERIFIED, - mode_display_name="Verified Certificate", + mode_slug=mode_slug, + mode_display_name=mode_display_name, min_price=TEST_COURSE_PRICE, _expiration_datetime=upgrade_exp_date, ).save() + + +def remove_course_mode(course, mode_slug): + """ + Remove a course mode from the test course if it exists in the course. + + Args: + course + mode_slug (str): slug of the mode to remove + """ + try: + mode = CourseMode.objects.get(course_id=course.id, mode_slug=mode_slug) + except ObjectDoesNotExist: + pass + + mode.delete() diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index 41ea43f3f5..ba9730a022 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -58,7 +58,7 @@ from xmodule.modulestore.tests.django_utils import CourseUserType, ModuleStoreTe from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls from ... import COURSE_PRE_START_ACCESS_FLAG, ENABLE_COURSE_GOALS -from .helpers import add_course_mode +from .helpers import add_course_mode, remove_course_mode from .test_course_updates import create_course_update, remove_course_updates TEST_PASSWORD = 'test' @@ -656,6 +656,34 @@ class TestCourseHomePageAccess(CourseHomePageTestCase): response = self.client.get(url) self.assertEqual(response.status_code, 404) + @override_waffle_flag(COURSE_PRE_START_ACCESS_FLAG, active=True) + def test_masters_course_message(self): + enroll_button_html = "" + + # Verify that unenrolled users visiting a course with a Master's track + # that is not the only track are shown an enroll call to action message + add_course_mode(self.course, CourseMode.MASTERS, 'Master\'s Mode', upgrade_deadline_expired=False) + + self.create_user_for_course(self.course, CourseUserType.UNENROLLED) + url = course_home_url(self.course) + response = self.client.get(url) + + self.assertContains(response, TEST_COURSE_HOME_MESSAGE) + self.assertContains(response, TEST_COURSE_HOME_MESSAGE_UNENROLLED) + self.assertContains(response, enroll_button_html) + + # Verify that unenrolled users visiting a course that contains only a Master's track + # are not shown an enroll call to action message + remove_course_mode(self.course, CourseMode.VERIFIED) + + response = self.client.get(url) + + expected_message = ('You must be enrolled in the course to see course content. ' + 'Please contact your degree administrator or edX Support if you have questions.') + self.assertContains(response, TEST_COURSE_HOME_MESSAGE) + self.assertContains(response, expected_message) + self.assertNotContains(response, enroll_button_html) + @override_waffle_flag(COURSE_PRE_START_ACCESS_FLAG, active=True) def test_course_messaging(self): """ diff --git a/openedx/features/course_experience/views/course_home_messages.py b/openedx/features/course_experience/views/course_home_messages.py index 084f2f296e..56521f2711 100644 --- a/openedx/features/course_experience/views/course_home_messages.py +++ b/openedx/features/course_experience/views/course_home_messages.py @@ -5,6 +5,7 @@ View logic for handling course messages. from datetime import datetime from babel.dates import format_date, format_timedelta +from course_modes.models import CourseMode from courseware.courses import get_course_date_blocks, get_course_with_access from django.contrib import auth from django.template.loader import render_to_string @@ -129,7 +130,20 @@ def _register_course_home_messages(request, course, user_access, course_start_da ) if not user_access['is_anonymous'] and not user_access['is_staff'] and \ not user_access['is_enrolled']: - if not course.invitation_only: + + title = Text(_(u'Welcome to {course_display_name}')).format( + course_display_name=course.display_name + ) + + if CourseMode.is_masters_only(course.id): + # if a course is a Master's only course, we will not offer user ability to self-enroll + CourseHomeMessages.register_info_message( + request, + Text(_('You must be enrolled in the course to see course content. ' + 'Please contact your degree administrator or edX Support if you have questions.')), + title=title + ) + elif not course.invitation_only: CourseHomeMessages.register_info_message( request, Text(_( @@ -138,9 +152,7 @@ def _register_course_home_messages(request, course, user_access, course_start_da open_enroll_link=HTML('') ), - title=Text(_(u'Welcome to {course_display_name}')).format( - course_display_name=course.display_name - ) + title=title ) else: CourseHomeMessages.register_info_message(