Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Merge pull request #20397 from cpennington/revmi-34-attempt-2

Browse Source 
Revmi 34 attempt 2
This commit is contained in:
Calen Pennington
2019-05-20 10:35:39 -04:00
committed by GitHub
parent deebe34652 755a5c7db6
commit ff73f7d746
9 changed files with 215 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
common/lib/xmodule/xmodule/modulestore/tests/factories.py
View File

@@ -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

19
lms/djangoapps/courseware/access.py
View File

@@ -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):

25
lms/djangoapps/courseware/access_response.py
View File

@@ -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):

7
lms/djangoapps/courseware/access_utils.py
View File

@@ -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():

36
lms/djangoapps/courseware/module_render.py
View File

@@ -290,7 +290,7 @@ def _add_timed_exam_info(user, course, section, section_context):
def get_module(user, request, usage_key, field_data_cache,
position=None, log_if_not_found=True, wrap_xmodule_display=True,
grade_bucket_type=None, depth=0,
static_asset_path='', course=None):
static_asset_path='', course=None, will_recheck_access=False):
"""
Get an instance of the xmodule class identified by location,
setting the state based on an existing StudentModule, or creating one if none
@@ -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
@@ -325,7 +328,7 @@ def get_module(user, request, usage_key, field_data_cache,
wrap_xmodule_display=wrap_xmodule_display,
grade_bucket_type=grade_bucket_type,
static_asset_path=static_asset_path,
course=course)
course=course, will_recheck_access=will_recheck_access)
except ItemNotFoundError:
if log_if_not_found:
log.debug("Error in get_module: ItemNotFoundError")
@@ -392,7 +395,7 @@ def get_xqueue_callback_url_prefix(request):
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,
course=None):
course=None, will_recheck_access=False):
"""
Implements get_module, extracting out the request-specific functionality.
@@ -424,7 +427,8 @@ def get_module_for_descriptor(user, request, descriptor, field_data_cache, cours
user_location=user_location,
request_token=xblock_request_token(request),
disable_staff_debug_info=disable_staff_debug_info,
course=course
course=course,
will_recheck_access=will_recheck_access,
)
@@ -443,7 +447,8 @@ def get_module_system_for_user(
static_asset_path='',
user_location=None,
disable_staff_debug_info=False,
course=None
course=None,
will_recheck_access=False,
):
"""
Helper function that returns a module system and student_data bound to a user and a descriptor.
@@ -514,7 +519,7 @@ def get_module_system_for_user(
user_location=user_location,
request_token=request_token,
course=course,
will_recheck_access=True,
will_recheck_access=will_recheck_access,
)
def get_event_handler(event_type):
@@ -651,7 +656,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=will_recheck_access,
)
module.descriptor.bind_for_student(
@@ -872,7 +878,8 @@ def get_module_for_descriptor_internal(user, descriptor, student_data, course_id
user_location=user_location,
request_token=request_token,
disable_staff_debug_info=disable_staff_debug_info,
course=course
course=course,
will_recheck_access=will_recheck_access,
)
descriptor.bind_for_student(
@@ -900,7 +907,6 @@ def get_module_for_descriptor_internal(user, descriptor, student_data, course_id
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
@@ -908,7 +914,7 @@ def get_module_for_descriptor_internal(user, descriptor, student_data, course_id
return descriptor
def load_single_xblock(request, user_id, course_id, usage_key_string, course=None):
def load_single_xblock(request, user_id, course_id, usage_key_string, course=None, will_recheck_access=False):
"""
Load a single XBlock identified by usage_key_string.
"""
@@ -922,7 +928,15 @@ def load_single_xblock(request, user_id, course_id, usage_key_string, course=Non
modulestore().get_item(usage_key),
depth=0,
)
instance = get_module(user, request, usage_key, field_data_cache, grade_bucket_type='xqueue', course=course)
instance = get_module(
user,
request,
usage_key,
field_data_cache,
grade_bucket_type='xqueue',
course=course,
will_recheck_access=will_recheck_access
)
if instance is None:
msg = u"No module {0} for user {1}--access denied?".format(usage_key_string, user)
log.debug(msg)

2
lms/djangoapps/courseware/views/index.py
View File

@@ -362,6 +362,7 @@ class CoursewareIndex(View):
self.field_data_cache,
self.course_key,
course=self.course,
will_recheck_access=True,
)
def _prefetch_and_bind_section(self):
@@ -382,6 +383,7 @@ class CoursewareIndex(View):
self.course_key,
self.position,
course=self.course,
will_recheck_access=True,
)
def _save_positions(self):

8
lms/djangoapps/courseware/views/views.py
View File

@@ -322,7 +322,13 @@ def course_info(request, course_id):
course.id, request.user, course, depth=2
)
course_module = get_module_for_descriptor(
user, request, course, field_data_cache, course.id, course=course
user,
request,
course,
field_data_cache,
course.id,
course=course,
will_recheck_access=True,
)
chapter_module = get_current_child(course_module)
if chapter_module is not None:

4
lms/templates/seq_module.html
View File

@@ -54,8 +54,8 @@
id="tab_${idx}">
<span class="icon fa seq_${item['type']}" aria-hidden="true"></span>
% if 'complete' in item:
<span
class="fa fa-check-circle check-circle ${"is-hidden" if not item['complete'] else ""}"
<span
class="fa fa-check-circle check-circle ${"is-hidden" if not item['complete'] else ""}"
style="color:green"
aria-hidden="true"
></span>

172
openedx/features/content_type_gating/tests/test_access.py
View File

@@ -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,20 +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),
course=course
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):
@@ -95,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):
@@ -125,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
@@ -183,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: [
@@ -234,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',
@@ -350,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,
@@ -467,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={
@@ -500,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'],
@@ -523,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,
@@ -674,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(
@@ -733,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,
)
@@ -843,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,
)
@@ -862,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,
)
@@ -899,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,
)
@@ -962,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,
)