From 3a7cff01e88588a01b6f74d897863d0586da806a Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 2 Oct 2018 12:02:35 -0400 Subject: [PATCH 1/5] Allow AccessResponse objects to carry an HTML fragment targetted at the user --- lms/djangoapps/courseware/access_response.py | 30 ++++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/courseware/access_response.py b/lms/djangoapps/courseware/access_response.py index 270cb3b044..4f254887e8 100644 --- a/lms/djangoapps/courseware/access_response.py +++ b/lms/djangoapps/courseware/access_response.py @@ -9,7 +9,7 @@ from xmodule.course_metadata_utils import DEFAULT_START_DATE class AccessResponse(object): """Class that represents a response from a has_access permission check.""" - def __init__(self, has_access, error_code=None, developer_message=None, user_message=None): + def __init__(self, has_access, error_code=None, developer_message=None, user_message=None, user_fragment=None): """ Creates an AccessResponse object. @@ -21,11 +21,14 @@ class AccessResponse(object): to show the developer user_message (String): optional - default is None. Message to show the user + user_fragment (:py:class:`~web_fragments.fragment.Fragment`): optional - + An html fragment to display to the user if their access is denied. """ self.has_access = has_access self.error_code = error_code self.developer_message = developer_message self.user_message = user_message + self.user_fragment = user_fragment if has_access: assert error_code is None @@ -54,15 +57,29 @@ class AccessResponse(object): "has_access": self.has_access, "error_code": self.error_code, "developer_message": self.developer_message, - "user_message": self.user_message + "user_message": self.user_message, + "user_fragment": self.user_fragment, } def __repr__(self): - return "AccessResponse({!r}, {!r}, {!r}, {!r})".format( + return "AccessResponse({!r}, {!r}, {!r}, {!r}, {!r})".format( self.has_access, self.error_code, self.developer_message, - self.user_message + self.user_message, + self.user_fragment, + ) + + def __eq__(self, other): + if not isinstance(other, AccessResponse): + return False + + return ( + self.has_access == other.has_access and + self.error_code == other.error_code and + self.developer_message == other.developer_message and + self.user_message == other.user_message and + self.user_fragment == other.user_fragment ) @@ -72,7 +89,7 @@ class AccessError(AccessResponse): denial in has_access. Contains the error code, user and developer messages. Subclasses represent specific errors. """ - def __init__(self, error_code, developer_message, user_message): + def __init__(self, error_code, developer_message, user_message, user_fragment=None): """ Creates an AccessError object. @@ -83,9 +100,10 @@ class AccessError(AccessResponse): error_code (String): unique identifier for the specific type of error developer_message (String): message to show the developer user_message (String): message to show the user + user_fragment (:py:class:`~web_fragments.fragment.Fragment`): HTML to show the user """ - super(AccessError, self).__init__(False, error_code, developer_message, user_message) + super(AccessError, self).__init__(False, error_code, developer_message, user_message, user_fragment) class StartDateError(AccessError): From 2ce5417e0c89684b9f676ecade5acccabd6ad443 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 2 Oct 2018 13:46:59 -0400 Subject: [PATCH 2/5] Allow UserPartitions to specify a text or html message when a user is denied access because of their partition membership --- .../xmodule/xmodule/partitions/partitions.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/common/lib/xmodule/xmodule/partitions/partitions.py b/common/lib/xmodule/xmodule/partitions/partitions.py index f2630744e0..c0271ad5d0 100644 --- a/common/lib/xmodule/xmodule/partitions/partitions.py +++ b/common/lib/xmodule/xmodule/partitions/partitions.py @@ -239,3 +239,33 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche group_id=group_id, partition_id=self.id ) ) + + def access_denied_message(self, block, user, user_group, allowed_groups): + """ + Return a message that should be displayed to the user when they are not allowed to access + content managed by this partition, or None if there is no applicable message. + + Arguments: + block (:class:`.XBlock`): The content being managed + user (:class:`.User`): The user who was denied access + user_group (:class:`.Group`): The current Group the user is in + allowed_groups (list of :class:`.Group`): The groups who are allowed to see the content + + Returns: str + """ + return None + + def access_denied_fragment(self, block, user, course_key, user_group, allowed_groups): + """ + Return an html fragment that should be displayed to the user when they are not allowed to access + content managed by this partition, or None if there is no applicable message. + + Arguments: + block (:class:`.XBlock`): The content being managed + user (:class:`.User`): The user who was denied access + user_group (:class:`.Group`): The current Group the user is in + allowed_groups (list of :class:`.Group`): The groups who are allowed to see the content + + Returns: :class:`.Fragment` + """ + return None From ae41ac446e64981dab547636d4476bdda26b33a0 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 2 Oct 2018 13:47:30 -0400 Subject: [PATCH 3/5] Allow AccessResponse messages to appear on the student dashboard --- common/djangoapps/student/tests/test_views.py | 8 +++++--- common/djangoapps/student/views/dashboard.py | 8 ++++---- lms/templates/dashboard.html | 2 +- .../dashboard/_dashboard_course_listing.html | 12 ++++++++++-- themes/edx.org/lms/templates/dashboard.html | 2 +- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index ffa5c6b089..20b00120ee 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -24,6 +24,7 @@ from edx_oauth2_provider.tests.factories import (ClientFactory, TrustedClientFactory) from entitlements.tests.factories import CourseEntitlementFactory from milestones.tests.utils import MilestonesTestCaseMixin +from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory @@ -377,10 +378,11 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, program = ProgramFactory() CourseEntitlementFactory.create(user=self.user, course_uuid=program['courses'][0]['uuid']) mock_get_programs.return_value = [program] - mock_course_overview.return_value = CourseOverviewFactory.create(start=self.TOMORROW) + course_key = CourseKey.from_string('course-v1:FAKE+FA1-MA1.X+3T2017') + mock_course_overview.return_value = CourseOverviewFactory.create(start=self.TOMORROW, id=course_key) mock_course_runs.return_value = [ { - 'key': 'course-v1:FAKE+FA1-MA1.X+3T2017', + 'key': unicode(course_key), 'enrollment_end': str(self.TOMORROW), 'pacing_type': 'instructor_paced', 'type': 'verified', @@ -388,7 +390,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, } ] mock_pseudo_session.return_value = { - 'key': 'course-v1:FAKE+FA1-MA1.X+3T2017', + 'key': unicode(course_key), 'type': 'verified' } response = self.client.get(self.path) diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 4414178040..ab0d3aca4b 100644 --- a/common/djangoapps/student/views/dashboard.py +++ b/common/djangoapps/student/views/dashboard.py @@ -647,10 +647,10 @@ def student_dashboard(request): staff_access = True errored_courses = modulestore().get_errored_courses() - show_courseware_links_for = frozenset( - enrollment.course_id for enrollment in course_enrollments - if has_access(request.user, 'load', enrollment.course_overview) - ) + show_courseware_links_for = { + enrollment.course_id: has_access(request.user, 'load', enrollment.course_overview) + for enrollment in course_enrollments + } # Find programs associated with course runs being displayed. This information # is passed in the template context to allow rendering of program-related diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index 1fc45e6c1e..22f5e529d8 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -175,7 +175,7 @@ from student.models import CourseEnrollment show_email_settings = (enrollment.course_id in show_email_settings_for) session_id = enrollment.course_id - show_courseware_link = (session_id in show_courseware_links_for) + show_courseware_link = show_courseware_links_for.get(session_id, False) cert_status = cert_statuses.get(session_id) can_refund_entitlement = entitlement and entitlement.is_entitlement_refundable() can_unenroll = (not cert_status) or cert_status.get('can_unenroll') if not unfulfilled_entitlement else False diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index 680bd640c9..1b16d0ba33 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -184,8 +184,8 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ % elif not is_course_blocked: - ${_('View Course')} @@ -202,6 +202,14 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ % endif % endif + % elif hasattr(show_courseware_link, 'user_message'): + + ${show_courseware_link.user_message} + +  ${_('for {course_display_name}').format(course_display_name=course_overview.display_name_with_default)} + + % endif % if show_courseware_link or course_overview.has_social_sharing_url() or course_overview.has_marketing_url(): diff --git a/themes/edx.org/lms/templates/dashboard.html b/themes/edx.org/lms/templates/dashboard.html index e05515c7f5..63a54c5a80 100644 --- a/themes/edx.org/lms/templates/dashboard.html +++ b/themes/edx.org/lms/templates/dashboard.html @@ -190,7 +190,7 @@ from student.models import CourseEnrollment show_email_settings = (enrollment.course_id in show_email_settings_for) session_id = enrollment.course_id - show_courseware_link = (session_id in show_courseware_links_for) + show_courseware_link = show_courseware_links_for.get(session_id, False) cert_status = cert_statuses.get(session_id) can_refund_entitlement = entitlement and entitlement.is_entitlement_refundable() can_unenroll = (not cert_status) or cert_status.get('can_unenroll') if not unfulfilled_entitlement else False From 13c0dd580f7cd930953dc3fbb6cfcab2e09d1633 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 2 Oct 2018 14:06:47 -0400 Subject: [PATCH 4/5] Make Student Dashboard course access error messages float to the right of the unenroll gear --- lms/static/sass/multicourse/_dashboard.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/static/sass/multicourse/_dashboard.scss b/lms/static/sass/multicourse/_dashboard.scss index 9c17dab753..3eace7c921 100644 --- a/lms/static/sass/multicourse/_dashboard.scss +++ b/lms/static/sass/multicourse/_dashboard.scss @@ -1342,7 +1342,7 @@ p.course-block { .enter-course-blocked { @include box-sizing(border-box); - @include float(left); + @include float(right); display: block; font: normal 15px/1.6rem $font-family-sans-serif; From 5af7fffcede9701f4f7fb0298d5ba4e7eafeaa13 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 10 Oct 2018 15:22:59 -0400 Subject: [PATCH 5/5] Remove unnecessary mocking of CourseKey.from_string in tests --- common/djangoapps/student/tests/test_views.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index 20b00120ee..a6fb6e92a6 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -451,8 +451,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, @patch('entitlements.api.v1.views.get_course_runs_for_course') @patch.object(CourseOverview, 'get_from_id') - @patch('opaque_keys.edx.keys.CourseKey.from_string') - def test_sessions_for_entitlement_course_runs(self, mock_course_key, mock_course_overview, mock_course_runs): + def test_sessions_for_entitlement_course_runs(self, mock_course_overview, mock_course_runs): """ When a learner has a fulfilled entitlement for a course run in the past, there should be no availableSession data passed to the JS view. When a learner has a fulfilled entitlement for a course run enrollment ending in the @@ -468,7 +467,6 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, start=self.TOMORROW, end=self.THREE_YEARS_FROM_NOW, self_paced=True, enrollment_end=self.THREE_YEARS_AGO ) mock_course_overview.return_value = mocked_course_overview - mock_course_key.return_value = mocked_course_overview.id course_enrollment = CourseEnrollmentFactory(user=self.user, course_id=unicode(mocked_course_overview.id)) mock_course_runs.return_value = [ { @@ -488,7 +486,6 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, mocked_course_overview.save() mock_course_overview.return_value = mocked_course_overview - mock_course_key.return_value = mocked_course_overview.id mock_course_runs.return_value = [ { 'key': str(mocked_course_overview.id), @@ -506,7 +503,6 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, mocked_course_overview.save() mock_course_overview.return_value = mocked_course_overview - mock_course_key.return_value = mocked_course_overview.id mock_course_runs.return_value = [ { 'key': str(mocked_course_overview.id), @@ -522,8 +518,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, @patch('openedx.core.djangoapps.programs.utils.get_programs') @patch('student.views.dashboard.get_visible_sessions_for_entitlement') @patch.object(CourseOverview, 'get_from_id') - @patch('opaque_keys.edx.keys.CourseKey.from_string') - def test_fulfilled_entitlement(self, mock_course_key, mock_course_overview, mock_course_runs, mock_get_programs): + def test_fulfilled_entitlement(self, mock_course_overview, mock_course_runs, mock_get_programs): """ When a learner has a fulfilled entitlement, their course dashboard should have: - exactly one course item, meaning it: @@ -536,7 +531,6 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, start=self.TOMORROW, self_paced=True, enrollment_end=self.TOMORROW ) mock_course_overview.return_value = mocked_course_overview - mock_course_key.return_value = mocked_course_overview.id course_enrollment = CourseEnrollmentFactory(user=self.user, course_id=unicode(mocked_course_overview.id)) mock_course_runs.return_value = [ { @@ -560,8 +554,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, @patch('openedx.core.djangoapps.programs.utils.get_programs') @patch('student.views.dashboard.get_visible_sessions_for_entitlement') @patch.object(CourseOverview, 'get_from_id') - @patch('opaque_keys.edx.keys.CourseKey.from_string') - def test_fulfilled_expired_entitlement(self, mock_course_key, mock_course_overview, mock_course_runs, mock_get_programs): + def test_fulfilled_expired_entitlement(self, mock_course_overview, mock_course_runs, mock_get_programs): """ When a learner has a fulfilled entitlement that is expired, their course dashboard should have: - exactly one course item, meaning it: @@ -573,7 +566,6 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, start=self.TOMORROW, self_paced=True, enrollment_end=self.TOMORROW ) mock_course_overview.return_value = mocked_course_overview - mock_course_key.return_value = mocked_course_overview.id course_enrollment = CourseEnrollmentFactory(user=self.user, course_id=unicode(mocked_course_overview.id), created=self.THREE_YEARS_AGO) mock_course_runs.return_value = [ {