From be472a8d52f3dd8464a61115bd597ec973ca05c6 Mon Sep 17 00:00:00 2001 From: Matt Tuchfarber Date: Fri, 2 Nov 2018 14:39:09 -0400 Subject: [PATCH] Extract upsell into template, extract ecomm call, use mobile utils --- lms/djangoapps/courseware/module_render.py | 53 ++++++--- lms/static/sass/_build-lms-v1.scss | 1 + lms/static/sass/_build-lms-v2.scss | 2 + .../sass/features/_content-type-gating.scss | 32 ++++++ .../content_type_gating/partitions.py | 102 ++++-------------- .../access_denied_message.html | 21 ++++ .../content_type_gating/tests/test_access.py | 5 +- openedx/tests/settings.py | 2 + 8 files changed, 120 insertions(+), 98 deletions(-) create mode 100644 lms/static/sass/features/_content-type-gating.scss create mode 100644 openedx/features/content_type_gating/templates/content_type_gating/access_denied_message.html diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index b8c51a1cf7..7b95e99c6b 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -5,6 +5,7 @@ Module rendering import hashlib import json import logging +import textwrap from collections import OrderedDict from functools import partial @@ -17,8 +18,10 @@ from django.template.context_processors import csrf from django.core.exceptions import PermissionDenied from django.urls import reverse from django.http import Http404, HttpResponse, HttpResponseForbidden +from django.utils.text import slugify from django.views.decorators.clickjacking import xframe_options_exempt from django.views.decorators.csrf import csrf_exempt +from edx_django_utils.cache import RequestCache from edx_django_utils.monitoring import set_custom_metrics_for_course_key, set_monitoring_transaction_name from edx_proctoring.services import ProctoringService from opaque_keys import InvalidKeyError @@ -34,6 +37,7 @@ from xblock.runtime import KvsFieldData import static_replace from capa.xqueue_interface import XQueueInterface from courseware.access import get_user_role, has_access +from courseware.access_response import IncorrectPartitionGroupError from courseware.entrance_exams import user_can_skip_entrance_exam, user_has_passed_entrance_exam from courseware.masquerade import ( MasqueradingKeyValueStore, @@ -43,7 +47,6 @@ from courseware.masquerade import ( ) from courseware.model_data import DjangoKeyValueStore, FieldDataCache from edxmako.shortcuts import render_to_string -from edx_django_utils.cache import RequestCache from eventtracking import tracker from lms.djangoapps.courseware.field_overrides import OverrideFieldData from lms.djangoapps.grades.signals.signals import SCORE_PUBLISHED @@ -55,6 +58,7 @@ from openedx.core.djangoapps.bookmarks.services import BookmarksService from openedx.core.djangoapps.crawlers.models import CrawlersConfig from openedx.core.djangoapps.credit.services import CreditService from openedx.core.djangoapps.util.user_utils import SystemUser +from openedx.core.djangolib.markup import HTML from openedx.core.lib.gating.services import GatingService from openedx.core.lib.license import wrap_with_license from openedx.core.lib.url_utils import quote_slashes, unquote_slashes @@ -71,7 +75,6 @@ from student.roles import CourseBetaTesterRole from track import contexts from util import milestones_helpers from util.json_request import JsonResponse -from django.utils.text import slugify from web_fragments.fragment import Fragment from xmodule.util.sandboxing import can_execute_unsafe_code, get_python_lib_zip from xblock_django.user_service import DjangoXBlockUserService @@ -176,6 +179,7 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_ for chapter in chapters: # Only show required content, if there is required content # chapter.hide_from_toc is read-only (bool) + # xss-lint: disable=python-deprecated-display-name display_id = slugify(chapter.display_name_with_default_escaped) local_hide_from_toc = False if required_content: @@ -197,6 +201,7 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_ found_active_section = True section_context = { + # xss-lint: disable=python-deprecated-display-name 'display_name': section.display_name_with_default_escaped, 'url_name': section.url_name, 'format': section.format if section.format is not None else '', @@ -220,6 +225,7 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_ last_processed_chapter = chapter toc_chapters.append({ + # xss-lint: disable=python-deprecated-display-name 'display_name': chapter.display_name_with_default_escaped, 'display_id': display_id, 'url_name': chapter.url_name, @@ -331,13 +337,17 @@ def get_module(user, request, usage_key, field_data_cache, log.debug("Error in get_module: ItemNotFoundError") return None - except: + except: # pylint: disable=W0702 # Something has gone terribly wrong, but still not letting it turn into a 500. log.exception("Error in get_module") return None -def display_access_messages(user, block, view, frag, context): +def display_access_messages(user, block, view, frag, context): # pylint: disable=W0613 + """ + An XBlock wrapper that replaces the content fragment with a fragment or message determined by + the has_access check. + """ blocked_prior_sibling = RequestCache('display_access_messages_prior_sibling') load_access = has_access(user, 'load', block, block.scope_ids.usage_id.course_key) @@ -347,7 +357,7 @@ def display_access_messages(user, block, view, frag, context): prior_sibling = blocked_prior_sibling.get_cached_response(block.parent) - if prior_sibling.is_found and prior_sibling.value == load_access: + if prior_sibling.is_found and prior_sibling.value.error_code == load_access.error_code: return Fragment(u"") else: blocked_prior_sibling.set(block.parent, load_access) @@ -355,16 +365,16 @@ def display_access_messages(user, block, view, frag, context): if load_access.user_fragment: msg_fragment = load_access.user_fragment elif load_access.user_message: - msg_fragment = Fragment(textwrap.dedent(u"""\ + msg_fragment = Fragment(textwrap.dedent(HTML(u"""\
{}
- """.format(load_access.user_message))) + """).format(load_access.user_message))) else: msg_fragment = Fragment(u"") if load_access.developer_message and has_access(user, 'staff', block, block.scope_ids.usage_id.course_key): - msg_fragment.content += textwrap.dedent(u"""\ + msg_fragment.content += textwrap.dedent(HTML(u"""\
{}
- """.format(load_access.developer_message)) + """).format(load_access.developer_message)) return msg_fragment @@ -384,6 +394,7 @@ def get_xqueue_callback_url_prefix(request): return settings.XQUEUE_INTERFACE.get('callback_url', prefix) +# pylint: disable=too-many-statements def get_module_for_descriptor(user, request, descriptor, field_data_cache, course_key, position=None, wrap_xmodule_display=True, grade_bucket_type=None, static_asset_path='', disable_staff_debug_info=False, @@ -508,7 +519,8 @@ def get_module_system_for_user( static_asset_path=static_asset_path, user_location=user_location, request_token=request_token, - course=course + course=course, + will_recheck_access=True, ) def get_event_handler(event_type): @@ -838,7 +850,7 @@ def get_module_for_descriptor_internal(user, descriptor, student_data, course_id track_function, xqueue_callback_url_prefix, request_token, position=None, wrap_xmodule_display=True, grade_bucket_type=None, static_asset_path='', user_location=None, disable_staff_debug_info=False, - course=None): + course=None, will_recheck_access=False): """ Actually implement get_module, without requiring a request. @@ -883,12 +895,18 @@ def get_module_for_descriptor_internal(user, descriptor, student_data, course_id user_needs_access_check = getattr(user, 'known', True) and not isinstance(user, SystemUser) if user_needs_access_check: access = has_access(user, 'load', descriptor, course_id) - if not access: - if access.user_message or access.user_fragment: - # This content will be access restricted by modifying the outgoing html - return descriptor - else: - return None + # A descriptor should only be returned if either the user has access, or the user doesn't have access, but + # the failed access has a message for the user and the caller of this function specifies it will check access + # again. This allows blocks to show specific error message or upsells when access is denied. + caller_will_handle_access_error = ( + not access + and will_recheck_access + and (access.user_message or access.user_fragment) + and isinstance(access, IncorrectPartitionGroupError) + ) + if access or caller_will_handle_access_error: + return descriptor + return None return descriptor @@ -1034,6 +1052,7 @@ def get_module_by_usage_id(request, course_id, usage_id, disable_staff_debug_inf tracking_context = { 'module': { + # xss-lint: disable=python-deprecated-display-name 'display_name': descriptor.display_name_with_default_escaped, 'usage_key': unicode(descriptor.location), } diff --git a/lms/static/sass/_build-lms-v1.scss b/lms/static/sass/_build-lms-v1.scss index 47b84dbc23..17e2758dcc 100644 --- a/lms/static/sass/_build-lms-v1.scss +++ b/lms/static/sass/_build-lms-v1.scss @@ -71,6 +71,7 @@ @import 'features/learner-profile'; @import 'features/journals'; @import 'features/_unsupported-browser-alert'; +@import 'features/content-type-gating'; // search @import 'search/search'; diff --git a/lms/static/sass/_build-lms-v2.scss b/lms/static/sass/_build-lms-v2.scss index e18e0184e9..7c0f2a2e05 100644 --- a/lms/static/sass/_build-lms-v2.scss +++ b/lms/static/sass/_build-lms-v2.scss @@ -34,6 +34,8 @@ @import 'features/course-upgrade-message'; @import 'features/learner-analytics-dashboard'; @import 'features/journals'; +@import 'features/content-type-gating'; + // Responsive Design @import 'header'; diff --git a/lms/static/sass/features/_content-type-gating.scss b/lms/static/sass/features/_content-type-gating.scss new file mode 100644 index 0000000000..9b8d8add8c --- /dev/null +++ b/lms/static/sass/features/_content-type-gating.scss @@ -0,0 +1,32 @@ +.content-paywall { + margin-top: 10px; + border-radius: 5px 5px 5px 5px; + display: flex; + justify-content: space-between; + border: lightgrey 1px solid; + padding: 15px 20px; +} +.content-paywall h3 { + font-weight: 600; + margin-bottom: 10px; +} +.content-paywall .fa-lock { + color: black; + margin-right: 10px; + font-size: 24px; + margin-left: 5px; +} +.content-paywall .certDIV_1 { + color: rgb(25, 125, 29); + height: 20px; + width: 300px; + font: normal normal 600 normal 14px / 20px 'Helvetica Neue', Helvetica, Arial, sans-serif; +} +.content-paywall .certA_2 { + text-decoration: underline !important; + color: rgb(0, 117, 180); + font: normal normal 400 normal 16px / 25.6px 'Open Sans'; +} +.content-paywall img { + height: 60px; +} diff --git a/openedx/features/content_type_gating/partitions.py b/openedx/features/content_type_gating/partitions.py index 9f6c4d9044..eb934aaae3 100644 --- a/openedx/features/content_type_gating/partitions.py +++ b/openedx/features/content_type_gating/partitions.py @@ -6,13 +6,12 @@ of audit learners. """ import logging -import textwrap from course_modes.models import CourseMode import crum from django.apps import apps -from django.conf import settings +from django.template.loader import render_to_string from django.utils.translation import ugettext_lazy as _ from web_fragments.fragment import Fragment @@ -23,6 +22,7 @@ from lms.djangoapps.courseware.masquerade import ( get_masquerading_user_group, ) from xmodule.partitions.partitions import Group, UserPartition, UserPartitionError +from openedx.core.lib.mobile_utils import is_request_from_mobile_app from openedx.features.course_duration_limits.config import ( CONTENT_TYPE_GATING_FLAG, CONTENT_TYPE_GATING_STUDIO_UI_FLAG, @@ -79,92 +79,34 @@ def create_content_gating_partition(course): class ContentTypeGatingPartition(UserPartition): - + """ + A custom UserPartition which allows us to override the access denied messaging in regards + to gated content. + """ def access_denied_fragment(self, block, user, user_group, allowed_groups): - ecomm_service = EcommerceService() - ecommerce_checkout = ecomm_service.is_enabled(user) - ecommerce_checkout_link = '' - ecommerce_bulk_checkout_link = '' - verified_mode = None - CourseMode = apps.get_model('course_modes.CourseMode') modes = CourseMode.modes_for_course_dict(block.scope_ids.usage_id.course_key) - verified_mode = modes.get(CourseMode.VERIFIED, '') - - if ecommerce_checkout and verified_mode.sku: - ecommerce_checkout_link = ecomm_service.get_checkout_page_url(verified_mode.sku) - + verified_mode = modes.get(CourseMode.VERIFIED) if verified_mode is None: return None + ecommerce_checkout_link = self._get_checkout_link(user, verified_mode.sku) request = crum.get_current_request() - if 'org.edx.mobile' in request.META.get('HTTP_USER_AGENT', ''): - upsell = '' - else: - upsell = textwrap.dedent("""\ - - - Upgrade to unlock (${min_price}) - - - """.format( - ecommerce_checkout_link=ecommerce_checkout_link, - # TODO: Does this need i18n? - min_price=verified_mode.min_price, - )) - - frag = Fragment(textwrap.dedent(u"""\ -
-
-

- - Verified Track Access -

- - Graded assessments are available to Verified Track learners. - - {upsell} -
- -
- """.format( - upsell=upsell, - ))) - frag.add_css(textwrap.dedent("""\ - .content-paywall { - margin-top: 10px; - border-radius: 5px 5px 5px 5px; - display: flex; - justify-content: space-between; - border: lightgrey 1px solid; - padding: 15px 20px; - } - .content-paywall h3 { - font-weight: 600; - margin-bottom: 10px; - } - .content-paywall .fa-lock { - color: black; - margin-right: 10px; - font-size: 24px; - margin-left: 5px; - } - .content-paywall .certDIV_1 { - color: rgb(25, 125, 29); - height: 20px; - width: 300px; - font: normal normal 600 normal 14px / 20px 'Helvetica Neue', Helvetica, Arial, sans-serif; - } - .content-paywall .certA_2 { - text-decoration: underline !important; - color: rgb(0, 117, 180); - font: normal normal 400 normal 16px / 25.6px 'Open Sans'; - } - .content-paywall img { - height: 60px; - } - """)) + frag = Fragment(render_to_string('content_type_gating/access_denied_message.html', { + 'mobile_app': is_request_from_mobile_app(request), + 'ecommerce_checkout_link': ecommerce_checkout_link, + 'min_price': str(verified_mode.min_price) + })) return frag + def access_denied_message(self, block, user, user_group, allowed_groups): + return "Graded assessments are available to Verified Track learners. Upgrade to Unlock." + + def _get_checkout_link(self, user, sku): + ecomm_service = EcommerceService() + ecommerce_checkout = ecomm_service.is_enabled(user) + if ecommerce_checkout and sku: + return ecomm_service.get_checkout_page_url(sku) or '' + class ContentTypeGatingPartitionScheme(object): """ diff --git a/openedx/features/content_type_gating/templates/content_type_gating/access_denied_message.html b/openedx/features/content_type_gating/templates/content_type_gating/access_denied_message.html new file mode 100644 index 0000000000..c220f8730d --- /dev/null +++ b/openedx/features/content_type_gating/templates/content_type_gating/access_denied_message.html @@ -0,0 +1,21 @@ +{% load i18n %} +{% load static %} +
+
+

+ + {% trans "Verified Track Access" %} +

+ + {% trans "Graded assessments are available to Verified Track learners." %} + + {% if not mobile_app and ecommerce_checkout_link %} + + + {% trans "Upgrade to unlock" %} (${{min_price}}) + + + {% endif %} +
+ +
diff --git a/openedx/features/content_type_gating/tests/test_access.py b/openedx/features/content_type_gating/tests/test_access.py index 45ea732406..b1e4eb9fef 100644 --- a/openedx/features/content_type_gating/tests/test_access.py +++ b/openedx/features/content_type_gating/tests/test_access.py @@ -104,7 +104,8 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): self.audit_user = UserFactory.create() self.enrollment = CourseEnrollmentFactory.create(user=self.audit_user, course_id=self.course.id, mode='audit') - def assert_block_is_gated(self, block, is_gated): + @patch("crum.get_current_request") + def assert_block_is_gated(self, block, is_gated, mock_get_current_request): ''' This functions asserts whether the passed in block is gated by content type gating. This is determined by checking whether the has_access method called the IncorrectPartitionGroupError. @@ -113,6 +114,8 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): whether the IncorrectPartitionGroupError was called. ''' fake_request = Mock() + fake_request = self.factory.get('') + mock_get_current_request.return_value = fake_request with patch.object(IncorrectPartitionGroupError, '__init__', wraps=IncorrectPartitionGroupError.__init__) as mock_access_error: diff --git a/openedx/tests/settings.py b/openedx/tests/settings.py index 05cc204acc..840dc3f196 100644 --- a/openedx/tests/settings.py +++ b/openedx/tests/settings.py @@ -25,6 +25,8 @@ BLOCK_STRUCTURES_SETTINGS = dict( COURSE_KEY_PATTERN = r'(?P[^/+]+(/|\+)[^/+]+(/|\+)[^/?]+)' COURSE_ID_PATTERN = COURSE_KEY_PATTERN.replace('course_key_string', 'course_id') +USAGE_KEY_PATTERN = r'(?P(?:i4x://?[^/]+/[^/]+/[^/]+/[^@]+(?:@[^/]+)?)|(?:[^/]+))' + COURSE_MODE_DEFAULTS = { 'bulk_sku': None,