diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
index c20499ec22..6995f6b858 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
@@ -284,9 +284,14 @@ class ItemFactory(XModuleFactory):
category = 'chapter'
parent = None
+ descriptive_tag = None
+
@lazy_attribute_sequence
def display_name(self, n):
- return "{} {}".format(self.category, n)
+ if self.descriptive_tag:
+ return "{} {} - {}".format(self.category, n, self.descriptive_tag)
+ else:
+ return "{} {}".format(self.category, n)
@lazy_attribute
def location(self):
@@ -355,6 +360,10 @@ class ItemFactory(XModuleFactory):
user_id = kwargs.pop('user_id', ModuleStoreEnum.UserID.test)
publish_item = kwargs.pop('publish_item', True)
+ # Remove the descriptive_tag, it's just for generating display_name,
+ # and doesn't need to be passed into the object constructor
+ kwargs.pop('descriptive_tag')
+
assert isinstance(location, UsageKey)
assert location != parent_location
diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py
index 60391f3fb0..ae351b2ebd 100644
--- a/lms/djangoapps/courseware/access.py
+++ b/lms/djangoapps/courseware/access.py
@@ -549,10 +549,16 @@ def _has_access_descriptor(user, action, descriptor, course_key=None):
return staff_access_response
return (
- _visible_to_nonstaff_users(descriptor) and
+ _visible_to_nonstaff_users(descriptor, display_error_to_user=False) and
(
_has_detached_class_tag(descriptor) or
- check_start_date(user, descriptor.days_early_for_beta, descriptor.start, course_key)
+ check_start_date(
+ user,
+ descriptor.days_early_for_beta,
+ descriptor.start,
+ course_key,
+ display_error_to_user=False
+ )
)
)
@@ -782,14 +788,19 @@ def _has_staff_access_to_descriptor(user, descriptor, course_key):
return _has_staff_access_to_location(user, descriptor.location, course_key)
-def _visible_to_nonstaff_users(descriptor):
+def _visible_to_nonstaff_users(descriptor, display_error_to_user=True):
"""
Returns if the object is visible to nonstaff users.
Arguments:
descriptor: object to check
+ display_error_to_user: If True, show an error message to the user say the content was hidden. Otherwise,
+ hide the content silently.
"""
- return VisibilityError() if descriptor.visible_to_staff_only else ACCESS_GRANTED
+ if descriptor.visible_to_staff_only:
+ return VisibilityError(display_error_to_user=display_error_to_user)
+ else:
+ return ACCESS_GRANTED
def _can_access_descriptor_with_milestones(user, descriptor, course_key):
diff --git a/lms/djangoapps/courseware/access_response.py b/lms/djangoapps/courseware/access_response.py
index 3dd810998c..2bc02c209b 100644
--- a/lms/djangoapps/courseware/access_response.py
+++ b/lms/djangoapps/courseware/access_response.py
@@ -121,7 +121,11 @@ class StartDateError(AccessError):
Access denied because the course has not started yet and the user
is not staff
"""
- def __init__(self, start_date):
+ def __init__(self, start_date, display_error_to_user=True):
+ """
+ Arguments:
+ display_error_to_user: If True, display this error to users in the UI.
+ """
error_code = "course_not_started"
if start_date == DEFAULT_START_DATE:
developer_message = u"Course has not started"
@@ -130,7 +134,11 @@ class StartDateError(AccessError):
developer_message = u"Course does not start until {}".format(start_date)
user_message = _(u"Course does not start until {}"
.format(u"{:%B %d, %Y}".format(start_date)))
- super(StartDateError, self).__init__(error_code, developer_message, user_message)
+ super(StartDateError, self).__init__(
+ error_code,
+ developer_message,
+ user_message if display_error_to_user else None
+ )
class MilestoneAccessError(AccessError):
@@ -149,11 +157,20 @@ class VisibilityError(AccessError):
Access denied because the user does have the correct role to view this
course.
"""
- def __init__(self):
+ def __init__(self, display_error_to_user=True):
+ """
+ Arguments:
+ display_error_to_user: Should a message showing that access was denied to this content
+ be shown to the user?
+ """
error_code = "not_visible_to_user"
developer_message = u"Course is not visible to this user"
user_message = _(u"You do not have access to this course")
- super(VisibilityError, self).__init__(error_code, developer_message, user_message)
+ super(VisibilityError, self).__init__(
+ error_code,
+ developer_message,
+ user_message if display_error_to_user else None
+ )
class MobileAvailabilityError(AccessError):
diff --git a/lms/djangoapps/courseware/access_utils.py b/lms/djangoapps/courseware/access_utils.py
index e76b66e959..58f4afa8dd 100644
--- a/lms/djangoapps/courseware/access_utils.py
+++ b/lms/djangoapps/courseware/access_utils.py
@@ -58,11 +58,14 @@ def adjust_start_date(user, days_early_for_beta, start, course_key):
return start
-def check_start_date(user, days_early_for_beta, start, course_key):
+def check_start_date(user, days_early_for_beta, start, course_key, display_error_to_user=True):
"""
Verifies whether the given user is allowed access given the
start date and the Beta offset for the given course.
+ Arguments:
+ display_error_to_user: If True, display this error to users in the UI.
+
Returns:
AccessResponse: Either ACCESS_GRANTED or StartDateError.
"""
@@ -80,7 +83,7 @@ def check_start_date(user, days_early_for_beta, start, course_key):
if now > effective_start:
return ACCESS_GRANTED
- return StartDateError(start)
+ return StartDateError(start, display_error_to_user=display_error_to_user)
def in_preview_mode():
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 2fbeb4e259..5f41c83d3b 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -313,6 +313,9 @@ def get_module(user, request, usage_key, field_data_cache,
by get_course_info_section, because info section modules
do not have a course as the parent module, and thus do not
inherit this lms key value.
+ - will_recheck_access : If True, the caller commits to re-checking access on each child XBlock
+ before rendering the content in order to display access error messages
+ to the user.
Returns: xmodule instance, or None if the user does not have access to the
module. If there's an error, will try to return an instance of ErrorModule
@@ -445,7 +448,7 @@ def get_module_system_for_user(
user_location=None,
disable_staff_debug_info=False,
course=None,
- will_recheck_access=False
+ will_recheck_access=False,
):
"""
Helper function that returns a module system and student_data bound to a user and a descriptor.
diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py
index 89a186ee8d..d40cf593cc 100644
--- a/lms/djangoapps/courseware/views/index.py
+++ b/lms/djangoapps/courseware/views/index.py
@@ -383,7 +383,7 @@ class CoursewareIndex(View):
self.course_key,
self.position,
course=self.course,
- will_recheck_access=True
+ will_recheck_access=True,
)
def _save_positions(self):
diff --git a/lms/templates/seq_module.html b/lms/templates/seq_module.html
index de1df5628a..6ec3acdc86 100644
--- a/lms/templates/seq_module.html
+++ b/lms/templates/seq_module.html
@@ -54,8 +54,8 @@
id="tab_${idx}">
% if 'complete' in item:
-
diff --git a/openedx/features/content_type_gating/tests/test_access.py b/openedx/features/content_type_gating/tests/test_access.py
index ff81bd5f63..2d8d2d7ad0 100644
--- a/openedx/features/content_type_gating/tests/test_access.py
+++ b/openedx/features/content_type_gating/tests/test_access.py
@@ -4,16 +4,21 @@ Test audit user's access to various content based on content-gating features.
from __future__ import absolute_import
import json
+import os
from datetime import datetime, timedelta
import ddt
import six
from django.conf import settings
-from django.test.client import RequestFactory
+from django.test.client import RequestFactory, Client
from django.test.utils import override_settings
from django.urls import reverse
from django.utils import timezone
+from django.contrib.auth.models import User
from mock import patch
+from pyquery import PyQuery as pq
+
+from six.moves.html_parser import HTMLParser
from course_api.blocks.api import get_blocks
from course_modes.tests.factories import CourseModeFactory
@@ -57,9 +62,9 @@ METADATA = {
@patch("crum.get_current_request")
-def _get_fragment_from_block(block, user_id, course, request_factory, mock_get_current_request):
+def _get_content_from_fragment(block, user_id, course, request_factory, mock_get_current_request):
"""
- Returns the rendered fragment of a block
+ Returns the content from the rendered fragment of a block
Arguments:
block: some sort of xblock descriptor, must implement .scope_ids.usage_id
user_id (int): id of user
@@ -69,21 +74,39 @@ def _get_fragment_from_block(block, user_id, course, request_factory, mock_get_c
mock_get_current_request.return_value = fake_request
# Load a block we know will pass access control checks
- vertical_xblock = load_single_xblock(
+ block = load_single_xblock(
request=fake_request,
user_id=user_id,
course_id=six.text_type(course.id),
- usage_key_string=six.text_type(course.scope_ids.usage_id),
+ usage_key_string=six.text_type(block.scope_ids.usage_id),
course=course,
will_recheck_access=True,
)
- runtime = vertical_xblock.runtime
-
- # This method of fetching the block from the descriptor bypassess access checks
- problem_block = runtime.get_module(block)
# Attempt to render the block, this should return different fragments if the content is gated or not.
- frag = runtime.render(problem_block, 'student_view')
- return frag
+ frag = block.render('student_view')
+ return frag.content
+
+
+def _get_content_from_lms_index(block, user_id, course, request_factory):
+ """
+ Returns the content from the lms index view of the block
+ Arguments:
+ block: some sort of xblock descriptor, must implement .scope_ids.usage_id
+ user_id (int): id of user
+ course_id (CourseLocator): id of course
+ """
+ client = Client()
+ client.login(username=User.objects.get(id=user_id).username, password=TEST_PASSWORD)
+ page_content = client.get(
+ reverse('courseware', kwargs={'course_id': block.scope_ids.usage_id.course_key}),
+ follow=True,
+ )
+ page = pq(page_content.content)
+ seq_contents = page('#seq_contents_0').html()
+ seq = pq(seq_contents)
+ block_contents = seq('[data-id="{}"]'.format(block.scope_ids.usage_id))
+
+ return block_contents.html()
def _assert_block_is_gated(block, is_gated, user, course, request_factory, has_upgrade_link=True):
@@ -96,16 +119,17 @@ def _assert_block_is_gated(block, is_gated, user, course, request_factory, has_u
course_id (CourseLocator): id of course
"""
checkout_link = '#' if has_upgrade_link else None
- with patch.object(ContentTypeGatingPartition, '_get_checkout_link', return_value=checkout_link):
- frag = _get_fragment_from_block(block, user.id, course, request_factory)
- if is_gated:
- assert 'content-paywall' in frag.content
- if has_upgrade_link:
- assert 'certA_1' in frag.content
+ for content_getter in (_get_content_from_fragment, _get_content_from_lms_index):
+ with patch.object(ContentTypeGatingPartition, '_get_checkout_link', return_value=checkout_link):
+ content = content_getter(block, user.id, course, request_factory)
+ if is_gated:
+ assert 'content-paywall' in content
+ if has_upgrade_link:
+ assert 'certA_1' in content
+ else:
+ assert 'certA_1' not in content
else:
- assert 'certA_1' not in frag.content
- else:
- assert 'content-paywall' not in frag.content
+ assert 'content-paywall' not in content
fake_request = request_factory.get('')
with patch('lms.djangoapps.course_api.blocks.api.is_request_from_mobile_app', return_value=True):
@@ -126,8 +150,8 @@ def _assert_block_is_empty(block, user_id, course, request_factory):
user_id (int): id of user
course_id (CourseLocator): id of course
"""
- frag = _get_fragment_from_block(block, user_id, course, request_factory)
- assert frag.content == u''
+ content = _get_content_from_lms_index(block, user_id, course, request_factory)
+ assert content is None
@ddt.ddt
@@ -184,48 +208,88 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase):
# Therefore, we create a problem component when has_score is True
# and an html component when has_score is False.
category='problem' if has_score else 'html',
- display_name=case_name,
graded=graded,
weight=weight,
metadata=METADATA if (graded and has_score and weight) else {},
)
+ # Intersperse HTML so that the content-gating renders in all blocks
+ ItemFactory.create(
+ parent=cls.blocks_dict['vertical'],
+ category='html',
+ graded=False,
+ )
cls.graded_score_weight_blocks[(graded, has_score, weight)] = block
+ host = os.environ.get('BOK_CHOY_HOSTNAME', '127.0.0.1')
+ metadata_lti_xblock = {
+ 'lti_id': 'correct_lti_id',
+ 'launch_url': 'http://{}:{}/{}'.format(host, '8765', 'correct_lti_endpoint'),
+ 'open_in_a_new_page': False
+ }
+
+ scored_lti_metadata = {}
+ scored_lti_metadata.update(metadata_lti_xblock)
+ scored_lti_metadata.update(METADATA)
+
# add LTI blocks to default course
cls.blocks_dict['lti_block'] = ItemFactory.create(
parent=cls.blocks_dict['vertical'],
category='lti_consumer',
- display_name='lti_consumer',
has_score=True,
graded=True,
- metadata=METADATA,
+ metadata=scored_lti_metadata,
+ )
+ # Intersperse HTML so that the content-gating renders in all blocks
+ ItemFactory.create(
+ parent=cls.blocks_dict['vertical'],
+ category='html',
+ graded=False,
)
cls.blocks_dict['lti_block_not_scored'] = ItemFactory.create(
parent=cls.blocks_dict['vertical'],
category='lti_consumer',
- display_name='lti_consumer_2',
has_score=False,
+ metadata=metadata_lti_xblock,
+ )
+
+ # Intersperse HTML so that the content-gating renders in all blocks
+ ItemFactory.create(
+ parent=cls.blocks_dict['vertical'],
+ category='html',
+ graded=False,
)
# add ungraded problem for xblock_handler test
cls.blocks_dict['graded_problem'] = ItemFactory.create(
parent=cls.blocks_dict['vertical'],
category='problem',
- display_name='graded_problem',
graded=True,
metadata=METADATA,
)
+
+ # Intersperse HTML so that the content-gating renders in all blocks
+ ItemFactory.create(
+ parent=cls.blocks_dict['vertical'],
+ category='html',
+ graded=False,
+ )
+
cls.blocks_dict['ungraded_problem'] = ItemFactory.create(
parent=cls.blocks_dict['vertical'],
category='problem',
- display_name='ungraded_problem',
+ graded=False,
+ )
+
+ # Intersperse HTML so that the content-gating renders in all blocks
+ ItemFactory.create(
+ parent=cls.blocks_dict['vertical'],
+ category='html',
graded=False,
)
cls.blocks_dict['audit_visible_graded_problem'] = ItemFactory.create(
parent=cls.blocks_dict['vertical'],
category='problem',
- display_name='audit_visible_graded_problem',
graded=True,
group_access={
CONTENT_GATING_PARTITION_ID: [
@@ -235,6 +299,13 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase):
},
)
+ # Intersperse HTML so that the content-gating renders in all blocks
+ ItemFactory.create(
+ parent=cls.blocks_dict['vertical'],
+ category='html',
+ graded=False,
+ )
+
# audit_only course only has an audit track available
cls.courses['audit_only'] = cls._create_course(
run='audit_only_course_run_1',
@@ -351,11 +422,16 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase):
block = ItemFactory.create(
parent=blocks_dict['vertical'],
category=component_type,
- display_name=component_type,
graded=True,
metadata={} if (component_type == 'html' or len(modes) == 1) else METADATA
)
blocks_dict[component_type] = block
+ # Intersperse HTML so that the content-gating renders in all blocks
+ ItemFactory.create(
+ parent=blocks_dict['vertical'],
+ category='html',
+ graded=False,
+ )
return {
'course': course,
@@ -468,7 +544,7 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase):
(404) and when it is available to audit users (200). Content is always available
to verified users.
"""
- problem_location = self.course.id.make_usage_key(xblock_type, xblock_name)
+ problem_location = self.blocks_dict[xblock_name].scope_ids.usage_id
url = reverse(
'xblock_handler',
kwargs={
@@ -501,6 +577,13 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase):
user = role_factory.create()
else:
user = role_factory.create(course_key=self.course.id)
+
+ CourseEnrollmentFactory.create(
+ user=user,
+ course_id=self.course.id,
+ mode='audit',
+ )
+
# assert that course team members have access to graded content
_assert_block_is_gated(
block=self.blocks_dict['problem'],
@@ -524,6 +607,11 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase):
role = RoleFactory(name=role_name, course_id=self.course.id)
role.users.add(user)
+ CourseEnrollmentFactory.create(
+ user=user,
+ course_id=self.course.id,
+ mode='audit',
+ )
_assert_block_is_gated(
block=self.blocks_dict['problem'],
user=user,
@@ -675,6 +763,13 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase):
user = UserFactory.create()
role = RoleFactory(name=role_name, course_id=self.course.id)
role.users.add(user)
+
+ CourseEnrollmentFactory.create(
+ user=user,
+ course_id=self.course.id,
+ mode='audit'
+ )
+
self.update_masquerade(username=user.username)
_assert_block_is_gated(
@@ -734,13 +829,11 @@ class TestConditionalContentAccess(TestConditionalContent):
self.block_a = ItemFactory.create(
category='problem',
parent=self.vertical_a,
- display_name='problem_a',
metadata=METADATA,
)
self.block_b = ItemFactory.create(
category='problem',
parent=self.vertical_b,
- display_name='problem_b',
metadata=METADATA,
)
@@ -844,7 +937,6 @@ class TestMessageDeduplication(ModuleStoreTestCase):
blocks_dict['graded_1'] = ItemFactory.create(
parent=blocks_dict['vertical'],
category='problem',
- display_name='graded_problem',
graded=True,
metadata=METADATA,
)
@@ -863,14 +955,12 @@ class TestMessageDeduplication(ModuleStoreTestCase):
blocks_dict['graded_1'] = ItemFactory.create(
parent=blocks_dict['vertical'],
category='problem',
- display_name='graded_problem',
graded=True,
metadata=METADATA,
)
blocks_dict['graded_2'] = ItemFactory.create(
parent=blocks_dict['vertical'],
category='problem',
- display_name='graded_problem',
graded=True,
metadata=METADATA,
)
@@ -900,28 +990,24 @@ class TestMessageDeduplication(ModuleStoreTestCase):
blocks_dict['graded_1'] = ItemFactory.create(
parent=blocks_dict['vertical'],
category='problem',
- display_name='graded_problem',
graded=True,
metadata=METADATA,
)
blocks_dict['graded_2'] = ItemFactory.create(
parent=blocks_dict['vertical'],
category='problem',
- display_name='graded_problem',
graded=True,
metadata=METADATA,
)
blocks_dict['graded_3'] = ItemFactory.create(
parent=blocks_dict['vertical'],
category='problem',
- display_name='graded_problem',
graded=True,
metadata=METADATA,
)
blocks_dict['graded_4'] = ItemFactory.create(
parent=blocks_dict['vertical'],
category='problem',
- display_name='graded_problem',
graded=True,
metadata=METADATA,
)
@@ -963,20 +1049,17 @@ class TestMessageDeduplication(ModuleStoreTestCase):
blocks_dict['graded_1'] = ItemFactory.create(
parent=blocks_dict['vertical'],
category='problem',
- display_name='graded_problem',
graded=True,
metadata=METADATA,
)
blocks_dict['ungraded_2'] = ItemFactory.create(
parent=blocks_dict['vertical'],
category='problem',
- display_name='ungraded_problem',
graded=False,
)
blocks_dict['graded_3'] = ItemFactory.create(
parent=blocks_dict['vertical'],
category='problem',
- display_name='graded_problem',
graded=True,
metadata=METADATA,
)