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

Convert LTIModule into LTIBlock. (#25713)

Browse Source
This commit is contained in:
Usman Khalid
2021-02-16 19:09:13 +05:00
committed by GitHub
parent 774f2956de
commit 62ed654b31
9 changed files with 139 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
common/lib/xmodule/setup.py
View File

@@ -18,7 +18,6 @@ XMODULES = [
"videosequence = xmodule.seq_module:SequenceDescriptor",
"custom_tag_template = xmodule.raw_module:RawDescriptor",
"raw = xmodule.raw_module:RawDescriptor",
"lti = xmodule.lti_module:LTIDescriptor",
]
XBLOCKS = [
"about = xmodule.html_module:AboutBlock",
@@ -31,6 +30,7 @@ XBLOCKS = [
"library = xmodule.library_root_xblock:LibraryRoot",
"library_content = xmodule.library_content_module:LibraryContentBlock",
"library_sourced = xmodule.library_sourced_block:LibrarySourcedBlock",
"lti = xmodule.lti_module:LTIBlock",
"nonstaff_error = xmodule.error_module:NonStaffErrorBlock",
"problem = xmodule.capa_module:ProblemBlock",
"randomize = xmodule.randomize_module:RandomizeBlock",

9
common/lib/xmodule/xmodule/lti_2_util.py
View File

@@ -1,6 +1,6 @@
"""
A mixin class for LTI 2.0 functionality. This is really just done to refactor the code to
keep the LTIModule class from getting too big
keep the LTIBlock class from getting too big
"""
@@ -26,13 +26,12 @@ LTI_2_0_JSON_CONTENT_TYPE = 'application/vnd.ims.lis.v2.result+json'
class LTIError(Exception):
"""Error class for LTIModule and LTI20ModuleMixin"""
pass # lint-amnesty, pylint: disable=unnecessary-pass
"""Error class for LTIBlock and LTI20BlockMixin"""
class LTI20ModuleMixin(object):
class LTI20BlockMixin(object):
"""
This class MUST be mixed into LTIModule. It does not do anything on its own. It's just factored
This class MUST be mixed into LTIBlock. It does not do anything on its own. It's just factored
out for modularity.
"""

109
common/lib/xmodule/xmodule/lti_module.py
View File

@@ -71,14 +71,27 @@ from pkg_resources import resource_string
from pytz import UTC
from six import text_type
from webob import Response
from web_fragments.fragment import Fragment
from xblock.core import List, Scope, String, XBlock
from xblock.fields import Boolean, Float
from xmodule.mako_module import MakoTemplateBlockBase
from openedx.core.djangolib.markup import HTML, Text
from xmodule.editing_module import MetadataOnlyEditingDescriptor
from xmodule.lti_2_util import LTI20ModuleMixin, LTIError
from xmodule.raw_module import EmptyDataRawDescriptor
from xmodule.x_module import XModule, module_attr
from xmodule.editing_module import EditingMixin
from xmodule.lti_2_util import LTI20BlockMixin, LTIError
from xmodule.raw_module import EmptyDataRawMixin
from xmodule.util.xmodule_django import add_webpack_to_fragment
from xmodule.xml_module import XmlMixin
from xmodule.x_module import (
HTMLSnippet,
ResourceTemplates,
shim_xmodule_js,
XModuleDescriptorToXBlockMixin,
XModuleMixin,
XModuleToXBlockMixin,
)
log = logging.getLogger(__name__)
@@ -256,7 +269,20 @@ class LTIFields(object):
)
class LTIModule(LTIFields, LTI20ModuleMixin, XModule):
@XBlock.needs("i18n")
class LTIBlock(
LTIFields,
LTI20BlockMixin,
EmptyDataRawMixin,
XmlMixin,
EditingMixin,
MakoTemplateBlockBase,
XModuleDescriptorToXBlockMixin,
XModuleToXBlockMixin,
HTMLSnippet,
ResourceTemplates,
XModuleMixin,
): # pylint: disable=abstract-method
"""
THIS MODULE IS DEPRECATED IN FAVOR OF https://github.com/edx/xblock-lti-consumer
@@ -338,14 +364,50 @@ class LTIModule(LTIFields, LTI20ModuleMixin, XModule):
Otherwise error message from LTI provider is generated.
"""
resources_dir = None
uses_xmodule_styles_setup = True
js = {
preview_view_js = {
'js': [
resource_string(__name__, 'js/src/lti/lti.js')
]
],
'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'),
}
css = {'scss': [resource_string(__name__, 'css/lti/lti.scss')]}
js_module_name = 'LTI'
preview_view_css = {
'scss': [
resource_string(__name__, 'css/lti/lti.scss')
],
}
mako_template = 'widgets/metadata-only-edit.html'
studio_js_module_name = 'MetadataOnlyEditingDescriptor'
studio_view_js = {
'js': [
resource_string(__name__, 'js/src/raw/edit/metadata-only.js')
],
'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'),
}
studio_view_css = {
'scss': [],
}
def studio_view(self, _context):
"""
Return the studio view.
"""
context = MakoTemplateBlockBase.get_context(self)
# Add our specific template information (the raw data body)
context.update({'data': self.data})
fragment = Fragment(
self.system.render_template(self.mako_template, context)
)
add_webpack_to_fragment(fragment, 'LTIBlockStudio')
shim_xmodule_js(fragment, self.studio_js_module_name)
return fragment
def max_score(self):
return self.weight if self.has_score else None
def get_input_fields(self): # lint-amnesty, pylint: disable=missing-function-docstring
# LTI provides a list of default parameters that might be passed as
@@ -449,11 +511,15 @@ class LTIModule(LTIFields, LTI20ModuleMixin, XModule):
'accept_grades_past_due': self.accept_grades_past_due,
}
def get_html(self):
def student_view(self, _context):
"""
Renders parameters to template.
Return the student view.
"""
return self.system.render_template('lti.html', self.get_context())
fragment = Fragment()
fragment.add_content(self.system.render_template('lti.html', self.get_context()))
add_webpack_to_fragment(fragment, 'LTIBlockPreview')
shim_xmodule_js(fragment, 'LTI')
return fragment
@XBlock.handler
def preview_handler(self, _, __):
@@ -533,7 +599,7 @@ class LTIModule(LTIFields, LTI20ModuleMixin, XModule):
"""
Return course by course id.
"""
return self.descriptor.runtime.modulestore.get_course(self.course_id)
return self.runtime.modulestore.get_course(self.course_id)
@property
def context_id(self):
@@ -907,20 +973,3 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
else:
close_date = due_date
return close_date is not None and datetime.datetime.now(UTC) > close_date
class LTIDescriptor(LTIFields, MetadataOnlyEditingDescriptor, EmptyDataRawDescriptor):
"""
Descriptor for LTI Xmodule.
"""
def max_score(self):
return self.weight if self.has_score else None
module_class = LTIModule
resources_dir = None
grade_handler = module_attr('grade_handler')
preview_handler = module_attr('preview_handler')
lti_2_0_result_rest_handler = module_attr('lti_2_0_result_rest_handler')
clear_user_module_score = module_attr('clear_user_module_score')
get_outcome_service_url = module_attr('get_outcome_service_url')

2
common/lib/xmodule/xmodule/static_content.py
View File

@@ -25,6 +25,7 @@ from xmodule.capa_module import ProblemBlock
from xmodule.conditional_module import ConditionalBlock
from xmodule.html_module import AboutBlock, CourseInfoBlock, HtmlBlock, StaticTabBlock
from xmodule.library_content_module import LibraryContentBlock
from xmodule.lti_module import LTIBlock
from xmodule.word_cloud_module import WordCloudBlock
from xmodule.x_module import XModuleDescriptor, HTMLSnippet
@@ -73,6 +74,7 @@ XBLOCK_CLASSES = [
CourseInfoBlock,
HtmlBlock,
LibraryContentBlock,
LTIBlock,
ProblemBlock,
StaticTabBlock,
VideoBlock,

17
common/lib/xmodule/xmodule/tests/test_lti20_unit.py
View File

@@ -4,27 +4,30 @@
import datetime
import textwrap
import unittest
from mock import Mock
from pytz import UTC
from xblock.field_data import DictFieldData
from xmodule.lti_2_util import LTIError
from xmodule.lti_module import LTIDescriptor
from xmodule.lti_module import LTIBlock
from . import LogicTest
from . import get_test_system
class LTI20RESTResultServiceTest(LogicTest):
class LTI20RESTResultServiceTest(unittest.TestCase):
"""Logic tests for LTI module. LTI2.0 REST ResultService"""
descriptor_class = LTIDescriptor
def setUp(self):
super(LTI20RESTResultServiceTest, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments
self.system = get_test_system()
self.environ = {'wsgi.url_scheme': 'http', 'REQUEST_METHOD': 'POST'}
self.system.get_real_user = Mock()
self.system.publish = Mock()
self.system.rebind_noauth_module_to_user = Mock()
self.user_id = self.xmodule.runtime.anonymous_student_id
self.xmodule = LTIBlock(self.system, DictFieldData({}), Mock())
self.lti_id = self.xmodule.lti_id
self.xmodule.due = None
self.xmodule.graceperiod = None
@@ -35,8 +38,8 @@ class LTI20RESTResultServiceTest(LogicTest):
mocked_course = Mock(name='mocked_course', lti_passports=['lti_id:test_client:test_secret'])
modulestore = Mock(name='modulestore')
modulestore.get_course.return_value = mocked_course
runtime = Mock(name='runtime', modulestore=modulestore)
self.xmodule.descriptor.runtime = runtime
runtime = Mock(name='runtime', modulestore=modulestore, anonymous_student_id='student')
self.xmodule.runtime = runtime
self.xmodule.lti_id = "lti_id"
test_cases = ( # (before sanitize, after sanitize)

42
common/lib/xmodule/xmodule/tests/test_lti_unit.py
View File

@@ -4,28 +4,31 @@
import datetime
import textwrap
import unittest
from copy import copy
import six
from lxml import etree
from mock import Mock, PropertyMock, patch
from opaque_keys.edx.locator import BlockUsageLocator
from pytz import UTC
from six import text_type
from webob.request import Request
from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds
from xmodule.fields import Timedelta
from xmodule.lti_2_util import LTIError
from xmodule.lti_module import LTIDescriptor
from xmodule.lti_module import LTIBlock
from . import LogicTest
from . import get_test_system
class LTIModuleTest(LogicTest):
class LTIBlockTest(unittest.TestCase):
"""Logic tests for LTI module."""
descriptor_class = LTIDescriptor
def setUp(self):
super(LTIModuleTest, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments
super().setUp()
self.environ = {'wsgi.url_scheme': 'http', 'REQUEST_METHOD': 'POST'}
self.request_body_xml_template = textwrap.dedent(u"""
<?xml version = "1.0" encoding = "UTF-8"?>
@@ -53,11 +56,17 @@ class LTIModuleTest(LogicTest):
</imsx_POXBody>
</imsx_POXEnvelopeRequest>
""")
self.system = get_test_system()
self.system.get_real_user = Mock()
self.system.publish = Mock()
self.system.rebind_noauth_module_to_user = Mock()
self.user_id = self.system.anonymous_student_id
self.user_id = self.xmodule.runtime.anonymous_student_id
self.xmodule = LTIBlock(
self.system,
DictFieldData({}),
ScopeIds(None, None, None, BlockUsageLocator(self.system.course_id, 'lti', 'name'))
)
self.lti_id = self.xmodule.lti_id
self.unquoted_resource_link_id = u'{}-i4x-2-3-lti-31de800015cf4afb973356dbe81496df'.format(
self.xmodule.runtime.hostname
@@ -75,7 +84,6 @@ class LTIModuleTest(LogicTest):
self.xmodule.due = None
self.xmodule.graceperiod = None
self.xmodule.descriptor = self.system.construct_xblock_from_class(self.descriptor_class, self.xmodule.scope_ids)
def get_request_body(self, params=None):
"""Fetches the body of a request specified by params"""
@@ -111,7 +119,7 @@ class LTIModuleTest(LogicTest):
}
@patch(
'xmodule.lti_module.LTIModule.get_client_key_secret',
'xmodule.lti_module.LTIBlock.get_client_key_secret',
return_value=('test_client_key', u'test_client_secret')
)
def test_authorization_header_not_present(self, _get_key_secret):
@@ -135,7 +143,7 @@ class LTIModuleTest(LogicTest):
self.assertDictEqual(expected_response, real_response)
@patch(
'xmodule.lti_module.LTIModule.get_client_key_secret',
'xmodule.lti_module.LTIBlock.get_client_key_secret',
return_value=('test_client_key', u'test_client_secret')
)
def test_authorization_header_empty(self, _get_key_secret):
@@ -300,7 +308,7 @@ class LTIModuleTest(LogicTest):
self.assertEqual(real_outcome_service_url, mock_url_prefix + test_service_name)
def test_resource_link_id(self):
with patch('xmodule.lti_module.LTIModule.location', new_callable=PropertyMock):
with patch('xmodule.lti_module.LTIBlock.location', new_callable=PropertyMock):
self.xmodule.location.html_id = lambda: 'i4x-2-3-lti-31de800015cf4afb973356dbe81496df'
expected_resource_link_id = text_type(six.moves.urllib.parse.quote(self.unquoted_resource_link_id))
real_resource_link_id = self.xmodule.get_resource_link_id()
@@ -324,7 +332,7 @@ class LTIModuleTest(LogicTest):
modulestore = Mock()
modulestore.get_course.return_value = mocked_course
runtime = Mock(modulestore=modulestore)
self.xmodule.descriptor.runtime = runtime
self.xmodule.runtime = runtime
self.xmodule.lti_id = "lti_id"
key, secret = self.xmodule.get_client_key_secret()
expected = ('test_client', 'test_secret')
@@ -342,7 +350,7 @@ class LTIModuleTest(LogicTest):
modulestore = Mock()
modulestore.get_course.return_value = mocked_course
runtime = Mock(modulestore=modulestore)
self.xmodule.descriptor.runtime = runtime
self.xmodule.runtime = runtime
# set another lti_id
self.xmodule.lti_id = "another_lti_id"
key_secret = self.xmodule.get_client_key_secret()
@@ -360,14 +368,14 @@ class LTIModuleTest(LogicTest):
modulestore = Mock()
modulestore.get_course.return_value = mocked_course
runtime = Mock(modulestore=modulestore)
self.xmodule.descriptor.runtime = runtime
self.xmodule.runtime = runtime
self.xmodule.lti_id = 'lti_id'
with self.assertRaises(LTIError):
self.xmodule.get_client_key_secret()
@patch('xmodule.lti_module.signature.verify_hmac_sha1', Mock(return_value=True))
@patch(
'xmodule.lti_module.LTIModule.get_client_key_secret',
'xmodule.lti_module.LTIBlock.get_client_key_secret',
Mock(return_value=('test_client_key', u'test_client_secret'))
)
def test_successful_verify_oauth_body_sign(self):
@@ -376,8 +384,8 @@ class LTIModuleTest(LogicTest):
"""
self.xmodule.verify_oauth_body_sign(self.get_signed_grade_mock_request())
@patch('xmodule.lti_module.LTIModule.get_outcome_service_url', Mock(return_value=u'https://testurl/'))
@patch('xmodule.lti_module.LTIModule.get_client_key_secret',
@patch('xmodule.lti_module.LTIBlock.get_outcome_service_url', Mock(return_value=u'https://testurl/'))
@patch('xmodule.lti_module.LTIBlock.get_client_key_secret',
Mock(return_value=(u'__consumer_key__', u'__lti_secret__')))
def test_failed_verify_oauth_body_sign_proxy_mangle_url(self):
"""
@@ -449,7 +457,7 @@ class LTIModuleTest(LogicTest):
@patch('xmodule.lti_module.signature.verify_hmac_sha1', Mock(return_value=False))
@patch(
'xmodule.lti_module.LTIModule.get_client_key_secret',
'xmodule.lti_module.LTIBlock.get_client_key_secret',
Mock(return_value=('test_client_key', u'test_client_secret'))
)
def test_failed_verify_oauth_body_sign(self):

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

@@ -96,7 +96,6 @@ from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
from xmodule.contentstore.django import contentstore
from xmodule.error_module import ErrorBlock, NonStaffErrorBlock
from xmodule.exceptions import NotFoundError, ProcessingError
from xmodule.lti_module import LTIModule
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.util.sandboxing import can_execute_unsafe_code, get_python_lib_zip
@@ -641,7 +640,7 @@ def get_module_system_for_user(
field_data_cache_real_user = FieldDataCache.cache_for_descriptor_descendents(
course_id,
real_user,
module.descriptor,
module,
asides=XBlockAsidesConfig.possible_asides(),
)
student_data_real_user = KvsFieldData(DjangoKeyValueStore(field_data_cache_real_user))
@@ -649,7 +648,7 @@ def get_module_system_for_user(
(inner_system, inner_student_data) = get_module_system_for_user(
user=real_user,
student_data=student_data_real_user, # These have implicit user bindings, rest of args considered not to
descriptor=module.descriptor,
descriptor=module,
course_id=course_id,
track_function=track_function,
xqueue_callback_url_prefix=xqueue_callback_url_prefix,
@@ -663,7 +662,7 @@ def get_module_system_for_user(
will_recheck_access=will_recheck_access,
)
module.descriptor.bind_for_student(
module.bind_for_student(
inner_system,
real_user.id,
[
@@ -673,10 +672,9 @@ def get_module_system_for_user(
],
)
module.descriptor.scope_ids = (
module.descriptor.scope_ids._replace(user_id=real_user.id)
module.scope_ids = (
module.scope_ids._replace(user_id=real_user.id)
)
module.scope_ids = module.descriptor.scope_ids # this is needed b/c NamedTuples are immutable
# now bind the module to the new ModuleSystem instance and vice-versa
module.runtime = inner_system
inner_system.xmodule_instance = module
@@ -757,9 +755,7 @@ def get_module_system_for_user(
# As we have the time to manually test more modules, we can add to the list
# of modules that get the per-course anonymized id.
is_pure_xblock = isinstance(descriptor, XBlock) and not isinstance(descriptor, XModuleDescriptor)
module_class = getattr(descriptor, 'module_class', None)
is_lti_module = not is_pure_xblock and issubclass(module_class, LTIModule)
if (is_pure_xblock and not getattr(descriptor, 'requires_per_student_anonymous_id', False)) or is_lti_module:
if (is_pure_xblock and not getattr(descriptor, 'requires_per_student_anonymous_id', False)):
anonymous_student_id = anonymous_id_for_user(user, course_id)
else:
anonymous_student_id = anonymous_id_for_user(user, None)

4
lms/djangoapps/courseware/tests/test_lti_integration.py
View File

@@ -126,7 +126,7 @@ class TestLTI(BaseTestXmodule):
self.assertEqual(generated_content.decode('utf-8'), expected_content)
class TestLTIModuleListing(SharedModuleStoreTestCase):
class TestLTIBlockListing(SharedModuleStoreTestCase):
"""
a test for the rest endpoint that lists LTI modules in a course
"""
@@ -136,7 +136,7 @@ class TestLTIModuleListing(SharedModuleStoreTestCase):
@classmethod
def setUpClass(cls):
super(TestLTIModuleListing, cls).setUpClass()
super(TestLTIBlockListing, cls).setUpClass()
cls.course = CourseFactory.create(display_name=cls.COURSE_NAME, number=cls.COURSE_SLUG)
cls.chapter1 = ItemFactory.create(
parent_location=cls.course.location,

19
lms/djangoapps/courseware/tests/test_module_render.py
View File

@@ -78,7 +78,7 @@ from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVer
from common.djangoapps.xblock_django.models import XBlockConfiguration
from xmodule.capa_module import ProblemBlock
from xmodule.html_module import AboutBlock, CourseInfoBlock, HtmlBlock, StaticTabBlock
from xmodule.lti_module import LTIDescriptor
from xmodule.lti_module import LTIBlock
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import (
@@ -1917,7 +1917,9 @@ class TestStaffDebugInfo(SharedModuleStoreTestCase):
self.assertTrue(mock_grade_histogram.called)
PER_COURSE_ANONYMIZED_DESCRIPTORS = (LTIDescriptor, )
PER_COURSE_ANONYMIZED_XBLOCKS = (
LTIBlock,
)
PER_STUDENT_ANONYMIZED_XBLOCKS = [
AboutBlock,
CourseInfoBlock,
@@ -1930,7 +1932,6 @@ PER_STUDENT_ANONYMIZED_XBLOCKS = [
# The "set" here is to work around the bug that load_classes returns duplicates for multiply-declared classes.
PER_STUDENT_ANONYMIZED_DESCRIPTORS = sorted(set([
class_ for (name, class_) in XModuleDescriptor.load_classes()
if not issubclass(class_, PER_COURSE_ANONYMIZED_DESCRIPTORS)
] + PER_STUDENT_ANONYMIZED_XBLOCKS), key=str)
@@ -1999,20 +2000,20 @@ class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase)
self._get_anonymous_id(CourseKey.from_string(course_id), descriptor_class)
)
@ddt.data(*PER_COURSE_ANONYMIZED_DESCRIPTORS)
def test_per_course_anonymized_id(self, descriptor_class):
@ddt.data(*PER_COURSE_ANONYMIZED_XBLOCKS)
def test_per_course_anonymized_id(self, xblock_class):
self.assertEqual(
# This value is set by observation, so that later changes to the student
# id computation don't break old data
'0c706d119cad686d28067412b9178454',
self._get_anonymous_id(CourseKey.from_string('MITx/6.00x/2012_Fall'), descriptor_class)
self._get_anonymous_id(CourseKey.from_string('MITx/6.00x/2012_Fall'), xblock_class)
)
self.assertEqual(
# This value is set by observation, so that later changes to the student
# id computation don't break old data
'e9969c28c12c8efa6e987d6dbeedeb0b',
self._get_anonymous_id(CourseKey.from_string('MITx/6.00x/2013_Spring'), descriptor_class)
self._get_anonymous_id(CourseKey.from_string('MITx/6.00x/2013_Spring'), xblock_class)
)
@@ -2288,11 +2289,11 @@ class TestRebindModule(TestSubmittingProblems):
module = self.get_module_for_user(self.anon_user)
user2 = UserFactory()
user2.id = 2
module.system.rebind_noauth_module_to_user(module._xmodule, user2) # pylint: disable=protected-access
module.system.rebind_noauth_module_to_user(module, user2)
self.assertTrue(module)
self.assertEqual(module.system.anonymous_student_id, anonymous_id_for_user(user2, self.course.id))
self.assertEqual(module.scope_ids.user_id, user2.id)
self.assertEqual(module._xmodule.scope_ids.user_id, user2.id) # pylint: disable=protected-access
self.assertEqual(module.scope_ids.user_id, user2.id)
@ddt.ddt