diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ba8cca0921fab3b30e6d5b46883aac69dd1285d4..e8af8568b60b7663063e6508dbaf946060b170c2 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,15 @@ Please See the [releases tab](https://github.com/openedx/xblock-lti-consumer/rel Unreleased ~~~~~~~~~~ + +7.0.0 - 2022-11-29 +------------------ +* Refactor anonymous user to real user rebinding function to use `rebind_user` service. +* Refactor accessing hostname from runtime attribute to using `settings.LMS_BASE`. +* Refactor usage of `get_real_user` with `UserService`. +* Refactor deprecated usage of `runtime.course_id` and replace with `runtime.scope_ids.usage_id.context_key`. +* Refactor deprecated usage of `block.location` with `block.scope_ids.usage_id`. + 6.4.0 - 2022-11-18 ------------------ Adds support for sending an external_user_id in LTI 1.1 XBlock launches. When the diff --git a/lti_consumer/__init__.py b/lti_consumer/__init__.py index 35120e4ea45f4d6c6d8a5ca8f7f83718864a784a..df365c478d522a25ae610d06e03d1b7874da8d08 100644 --- a/lti_consumer/__init__.py +++ b/lti_consumer/__init__.py @@ -4,4 +4,4 @@ Runtime will load the XBlock class from here. from .apps import LTIConsumerApp from .lti_xblock import LtiConsumerXBlock -__version__ = '6.4.0' +__version__ = '7.0.0' diff --git a/lti_consumer/api.py b/lti_consumer/api.py index 34290d574fac720c22bfd9c30fb0b8ac919dd9f4..dc1b29d2534432ebe228dc8b1473cfda1e228042 100644 --- a/lti_consumer/api.py +++ b/lti_consumer/api.py @@ -65,24 +65,24 @@ def _get_lti_config_for_block(block): if block.config_type == 'database': lti_config = _get_or_create_local_lti_config( block.lti_version, - block.location, + block.scope_ids.usage_id, LtiConfiguration.CONFIG_ON_DB, ) elif block.config_type == 'external': config = get_external_config_from_filter( - {"course_key": block.location.course_key}, + {"course_key": block.scope_ids.usage_id.context_key}, block.external_config ) lti_config = _get_or_create_local_lti_config( config.get("version"), - block.location, + block.scope_ids.usage_id, LtiConfiguration.CONFIG_EXTERNAL, external_id=block.external_config, ) else: lti_config = _get_or_create_local_lti_config( block.lti_version, - block.location, + block.scope_ids.usage_id, LtiConfiguration.CONFIG_ON_XBLOCK, ) return lti_config diff --git a/lti_consumer/lti_1p1/oauth.py b/lti_consumer/lti_1p1/oauth.py index c1a127f23f155d07604ef1a631008f830d9f2a98..4da150efd57bb4ae4af5a3c8bd1918575ec658a7 100644 --- a/lti_consumer/lti_1p1/oauth.py +++ b/lti_consumer/lti_1p1/oauth.py @@ -147,7 +147,7 @@ def log_authorization_header(request, client_key, client_secret): oauth_body_hash = str(base64.b64encode(sha1.digest())) log.debug("[LTI] oauth_body_hash = %s", oauth_body_hash) client = oauth1.Client(client_key, client_secret) - params = client.get_oauth_params(request) + params = oauth1.rfc5849.signature.collect_parameters(headers=request.headers, exclude_oauth_signature=False) params.append(('oauth_body_hash', oauth_body_hash)) mock_request = SignedRequest( uri=str(urllib.parse.unquote(request.url)), diff --git a/lti_consumer/lti_xblock.py b/lti_consumer/lti_xblock.py index a3b1fef9605201a7506a7521cc7ca168f6b8ac2b..fb3e1d96cd1ffb52dcf3bc2099ea8e13997faf45 100644 --- a/lti_consumer/lti_xblock.py +++ b/lti_consumer/lti_xblock.py @@ -139,10 +139,10 @@ def valid_config_type_values(block): {"display_name": _("Configuration on block"), "value": "new"} ] - if database_config_enabled(block.location.course_key): + if database_config_enabled(block.scope_ids.usage_id.context_key): values.append({"display_name": _("Database Configuration"), "value": "database"}) - if external_config_filter_enabled(block.location.course_key): + if external_config_filter_enabled(block.scope_ids.usage_id.context_key): values.append({"display_name": _("Reusable Configuration"), "value": "external"}) return values @@ -161,6 +161,7 @@ class LaunchTarget: @XBlock.needs('i18n') +@XBlock.needs('rebind_user') @XBlock.wants('user') @XBlock.wants('settings') @XBlock.wants('lti-configuration') @@ -695,8 +696,8 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): editable_fields = self.editable_field_names noneditable_fields = [] - is_database_config_enabled = database_config_enabled(self.location.course_key) # pylint: disable=no-member - is_external_config_filter_enabled = external_config_filter_enabled(self.location.course_key) # pylint: disable=no-member + is_database_config_enabled = database_config_enabled(self.scope_ids.usage_id.context_key) + is_external_config_filter_enabled = external_config_filter_enabled(self.scope_ids.usage_id.context_key) # If neither additional config_types are enabled, do not display the "config_type" field to users, as "new" is # the only option and does not make sense without other options. @@ -713,7 +714,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): if config_service: is_already_sharing_learner_info = self.ask_to_send_email or self.ask_to_send_username if not config_service.configuration.lti_access_to_learners_editable( - self.course_id, + self.scope_ids.usage_id.context_key, is_already_sharing_learner_info, ): noneditable_fields.extend(['ask_to_send_username', 'ask_to_send_email']) @@ -745,7 +746,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): context_id is an opaque identifier that uniquely identifies the context (e.g., a course) that contains the link being launched. """ - return str(self.course_id) + return str(self.scope_ids.usage_id.context_key) @property def role(self): @@ -763,7 +764,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): """ Return course by course id. """ - return self.runtime.modulestore.get_course(self.runtime.course_id) + return self.runtime.modulestore.get_course(self.scope_ids.usage_id.context_key) @property def lti_provider_key_secret(self): @@ -843,7 +844,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): toggling this flag in a running course carries the risk of breaking the LTI integrations in the course. This flag should also only be enabled for new courses in which no LTI attempts have been made. """ - if external_user_id_1p1_launches_enabled(self.location.course_key): # pylint: disable=no-member + if external_user_id_1p1_launches_enabled(self.scope_ids.usage_id.context_key): return self.external_user_id return self.anonymous_user_id @@ -880,9 +881,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): i4x-2-3-lti-31de800015cf4afb973356dbe81496df this part of resource_link_id: makes resource_link_id to be unique among courses inside same system. """ - return str(urllib.parse.quote( - f"{self.runtime.hostname}-{self.location.html_id()}" # pylint: disable=no-member - )) + return str(urllib.parse.quote(f"{settings.LMS_BASE}-{self.scope_ids.usage_id.html_id()}")) @property def lis_result_sourcedid(self): @@ -1223,7 +1222,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): # Runtime import because this can only be run in the LMS/Studio Django # environments. Importing the views on the top level will cause RuntimeErorr from lti_consumer.plugin.views import access_token_endpoint # pylint: disable=import-outside-toplevel - return access_token_endpoint(request, usage_id=str(self.location)) # pylint: disable=no-member + return access_token_endpoint(request, usage_id=str(self.scope_ids.usage_id)) @XBlock.handler def outcome_service_handler(self, request, suffix=''): # pylint: disable=unused-argument @@ -1294,7 +1293,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): except LtiError: return Response(status=401) # Unauthorized in this case. 401 is right - user = self.runtime.get_real_user(anon_id) + user = self.runtime.service(self, 'user').get_user_by_anonymous_id(anon_id) if not user: # that means we can't save to database, as we do not have real user id. msg = _("[LTI]: Real user not found against anon_id: {}").format(anon_id) log.info(msg) @@ -1332,7 +1331,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): Returns: dict: response to this request as dictated by the LtiConsumer """ - self.runtime.rebind_noauth_module_to_user(self, user) + self.runtime.service(self, 'rebind_user').rebind_noauth_module_to_user(self, user) args = [] if self.module_score: args.extend([self.module_score, self.score_comment]) @@ -1420,7 +1419,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): else: scaled_score = None - self.runtime.rebind_noauth_module_to_user(self, user) + self.runtime.service(self, 'rebind_user').rebind_noauth_module_to_user(self, user) # have to publish for the progress page... self.runtime.publish( @@ -1459,8 +1458,8 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): from lti_consumer.api import config_id_for_block config_id = config_id_for_block(self) - location = self.location # pylint: disable=no-member - course_key = str(location.course_key) + location = self.scope_ids.usage_id + course_key = str(location.context_key) launch_data = Lti1p3LaunchData( user_id=self.lms_user_id, @@ -1482,7 +1481,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): Return the title attribute of the context_claim for LTI 1.3 launches. This information is included in the launch_data query or form parameter of the LTI 1.3 third-party login initiation request. """ - course_key = self.location.course_key # pylint: disable=no-member + course_key = self.scope_ids.usage_id.context_key course = compat.get_course_by_id(course_key) return " - ".join([ @@ -1555,7 +1554,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): return { 'launch_url': launch_url.strip(), 'lti_1p3_launch_url': lti_1p3_launch_url, - 'element_id': self.location.html_id(), # pylint: disable=no-member + 'element_id': self.scope_ids.usage_id.html_id(), 'element_class': self.category, 'launch_target': self.launch_target, 'display_name': self.display_name, diff --git a/lti_consumer/models.py b/lti_consumer/models.py index 1b83b0c28947fe9469b6cd64a648541986b1a0c7..f96f5f71c719d286aa5ea00e5ebea4f8ccc6bb28 100644 --- a/lti_consumer/models.py +++ b/lti_consumer/models.py @@ -241,7 +241,7 @@ class LtiConfiguration(models.Model): }) if self.version == self.LTI_1P3 and self.config_store == self.CONFIG_ON_DB: block = compat.load_enough_xblock(self.location) - if not database_config_enabled(block.location.course_key): + if not database_config_enabled(block.scope_ids.usage_id.context_key): raise ValidationError({ "config_store": _("LTI Configuration stores on database is not enabled."), }) diff --git a/lti_consumer/outcomes.py b/lti_consumer/outcomes.py index d715ff245b2a6529dcd2d5a62a1da1f0ae580bb2..b05ae0db0bfbb19be8e44557828919ae7de154b7 100644 --- a/lti_consumer/outcomes.py +++ b/lti_consumer/outcomes.py @@ -6,8 +6,8 @@ https://www.imsglobal.org/specs/ltiomv1p0 """ import logging -import urllib.parse from xml.sax.saxutils import escape +from urllib.parse import unquote from lxml import etree from xblockutils.resources import ResourceLoader @@ -183,7 +183,8 @@ class OutcomeService: log.debug("[LTI]: %s", error_message) return response_xml_template.format(**failure_values) - real_user = self.xblock.runtime.get_real_user(urllib.parse.unquote(sourced_id.split(':')[-1])) + anon_id = unquote(sourced_id.split(':')[-1]) + real_user = self.xblock.runtime.service(self, 'user').get_user_by_anonymous_id(anon_id) if not real_user: # that means we can't save to database, as we do not have real user id. failure_values['imsx_messageIdentifier'] = escape(imsx_message_identifier) failure_values['imsx_description'] = "User not found." diff --git a/lti_consumer/signals/signals.py b/lti_consumer/signals/signals.py index ccf1194fed9e31aa2cb6487bdb132f3e309c911e..593d5abde6b98aa38fbad59ec7cd8e38f7d811e9 100644 --- a/lti_consumer/signals/signals.py +++ b/lti_consumer/signals/signals.py @@ -46,7 +46,7 @@ def publish_grade_on_score_update(sender, instance, **kwargs): # pylint: disabl # the LMS database. log.info( "Publishing LTI grade from block %s to LMS. User: %s (score: %s)", - block.location, + block.scope_ids.usage_id, user, score, ) diff --git a/lti_consumer/tests/test_utils.py b/lti_consumer/tests/test_utils.py index 192505b9d213926051fbce8e49d4d11ec3cc0506..f31c0781799941ff87e14cf200c829239de9aa6b 100644 --- a/lti_consumer/tests/test_utils.py +++ b/lti_consumer/tests/test_utils.py @@ -5,7 +5,8 @@ Utility functions used within unit tests from unittest.mock import Mock import urllib -from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import LocalId from webob import Request from workbench.runtime import WorkbenchRuntime from xblock.fields import ScopeIds @@ -22,30 +23,27 @@ def make_xblock(xblock_name, xblock_cls, attributes): runtime = WorkbenchRuntime() key_store = DictKeyValueStore() db_model = KvsFieldData(key_store) - ids = generate_scope_ids(runtime, xblock_name) + course_id = 'course-v1:edX+DemoX+Demo_Course' + course_key = CourseKey.from_string(course_id) + ids = generate_scope_ids(course_key, xblock_name) + xblock = xblock_cls(runtime, db_model, scope_ids=ids) xblock.category = Mock() - xblock.location = UsageKey.from_string( - 'block-v1:edX+DemoX+Demo_Course+type@problem+block@466f474fa4d045a8b7bde1b911e095ca' - ) - xblock.runtime = Mock( hostname='localhost', ) - xblock.course_id = 'course-v1:edX+DemoX+Demo_Course' for key, value in attributes.items(): setattr(xblock, key, value) return xblock -def generate_scope_ids(runtime, block_type): +def generate_scope_ids(course_key, block_type): """ Helper to generate scope IDs for an XBlock """ - def_id = runtime.id_generator.create_definition(block_type) - usage_id = runtime.id_generator.create_usage(def_id) - return ScopeIds('user', block_type, def_id, usage_id) + usage_key = course_key.make_usage_key(block_type, str(LocalId())) + return ScopeIds('user', block_type, usage_key, usage_key) def make_request(body, method='POST'): diff --git a/lti_consumer/tests/unit/plugin/test_views.py b/lti_consumer/tests/unit/plugin/test_views.py index 7c6740327ddd0c4433d331f949cd1356a40d607e..543c24c7b0871293031aabc87936a54a64c1876c 100644 --- a/lti_consumer/tests/unit/plugin/test_views.py +++ b/lti_consumer/tests/unit/plugin/test_views.py @@ -333,7 +333,7 @@ class TestLti1p3LaunchGateEndpoint(TestCase): self.xblock.config_type = 'database' LtiConfiguration.objects.filter(id=self.config.id).update( - location=self.xblock.location, # pylint: disable=no-member + location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P3, config_store=LtiConfiguration.CONFIG_ON_DB, lti_advantage_deep_linking_enabled=dl_enabled, diff --git a/lti_consumer/tests/unit/plugin/test_views_lti_ags.py b/lti_consumer/tests/unit/plugin/test_views_lti_ags.py index a80a142fb9bb365454396a44973a5f5681312824..7a9e850f6f32d17cec0424db8929237083bebc05 100644 --- a/lti_consumer/tests/unit/plugin/test_views_lti_ags.py +++ b/lti_consumer/tests/unit/plugin/test_views_lti_ags.py @@ -51,7 +51,7 @@ class LtiAgsLineItemViewSetTestCase(APITransactionTestCase): # Create configuration self.lti_config = LtiConfiguration.objects.create( - location=self.xblock.location, # pylint: disable=no-member + location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P3, ) @@ -171,7 +171,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): line_item = LtiAgsLineItem.objects.create( lti_configuration=self.lti_config, resource_id="test", - resource_link_id=self.xblock.location, # pylint: disable=no-member + resource_link_id=self.xblock.scope_ids.usage_id, label="test label", score_maximum=100 ) @@ -192,7 +192,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): 'scoreMaximum': 100, 'label': 'test label', 'tag': '', - 'resourceLinkId': str(self.xblock.location), # pylint: disable=no-member + 'resourceLinkId': str(self.xblock.scope_ids.usage_id), 'startDateTime': None, 'endDateTime': None, } @@ -209,7 +209,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): line_item = LtiAgsLineItem.objects.create( lti_configuration=self.lti_config, resource_id="test", - resource_link_id=self.xblock.location, # pylint: disable=no-member + resource_link_id=self.xblock.scope_ids.usage_id, label="test label", score_maximum=100 ) @@ -236,7 +236,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): 'scoreMaximum': 100, 'label': 'test label', 'tag': '', - 'resourceLinkId': str(self.xblock.location), # pylint: disable=no-member + 'resourceLinkId': str(self.xblock.scope_ids.usage_id), 'startDateTime': None, 'endDateTime': None, } @@ -256,7 +256,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): 'scoreMaximum': 100, 'label': 'test', 'tag': 'score', - 'resourceLinkId': str(self.xblock.location), # pylint: disable=no-member + 'resourceLinkId': str(self.xblock.scope_ids.usage_id), }), content_type="application/vnd.ims.lis.v2.lineitem+json", ) @@ -270,7 +270,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): 'scoreMaximum': 100, 'label': 'test', 'tag': 'score', - 'resourceLinkId': str(self.xblock.location), # pylint: disable=no-member + 'resourceLinkId': str(self.xblock.scope_ids.usage_id), 'startDateTime': None, 'endDateTime': None, } @@ -281,7 +281,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): self.assertEqual(line_item.score_maximum, 100) self.assertEqual(line_item.label, 'test') self.assertEqual(line_item.tag, 'score') - self.assertEqual(str(line_item.resource_link_id), str(self.xblock.location)) # pylint: disable=no-member + self.assertEqual(str(line_item.resource_link_id), str(self.xblock.scope_ids.usage_id)) def test_create_lineitem_invalid_resource_link_id(self): """ @@ -318,7 +318,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase): self.line_item = LtiAgsLineItem.objects.create( lti_configuration=self.lti_config, resource_id="test", - resource_link_id=self.xblock.location, # pylint: disable=no-member + resource_link_id=self.xblock.scope_ids.usage_id, label="test label", score_maximum=100 ) @@ -850,7 +850,7 @@ class LtiAgsViewSetResultsTests(LtiAgsLineItemViewSetTestCase): self.line_item = LtiAgsLineItem.objects.create( lti_configuration=self.lti_config, resource_id="test", - resource_link_id=self.xblock.location, # pylint: disable=no-member + resource_link_id=self.xblock.scope_ids.usage_id, label="test label", score_maximum=100 ) diff --git a/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py b/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py index 5d29d12dd0529d5ffdc4865657d71dcf2e07d109..a0ddc8b4188ce62c9f429431827df101592bbfd3 100644 --- a/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py +++ b/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py @@ -31,7 +31,7 @@ class LtiDeepLinkingTestCase(APITransactionTestCase): # Create configuration self.lti_config = LtiConfiguration.objects.create( - location=self.xblock.location, # pylint: disable=no-member + location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P3, ) diff --git a/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py b/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py index a9d3e1e2a76c0256eaf55884266747f61148805f..352e4533e2bfd1f9ef617fac0bb323611cd85e93 100644 --- a/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py +++ b/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py @@ -137,7 +137,7 @@ class LtiNrpsTestCase(APITransactionTestCase): # Create configuration self.lti_config = LtiConfiguration.objects.create( - location=self.xblock.location, # pylint: disable=no-member + location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P3, ) diff --git a/lti_consumer/tests/unit/test_api.py b/lti_consumer/tests/unit/test_api.py index 19ff9f2f7292e48b9438cde263c77486e2195675..083f850e4a56900bcb4322cdd963c8c6a92679d9 100644 --- a/lti_consumer/tests/unit/test_api.py +++ b/lti_consumer/tests/unit/test_api.py @@ -72,7 +72,7 @@ class Lti1P3TestCase(TestCase): # Create lti configuration self.lti_config = LtiConfiguration.objects.create( config_id=_test_config_id, - location=self.xblock.location, # pylint: disable=no-member + location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P3, config_store=LtiConfiguration.CONFIG_ON_XBLOCK, ) @@ -464,15 +464,11 @@ class TestGetLti1p3LaunchInfo(TestCase): """ Check if the API creates the model and retrieved correct info. """ - block = Mock() - block.location = 'block-v1:course+test+2020+type@problem+block@test' - block.lti_version = LtiConfiguration.LTI_1P3 - launch_data = self._get_lti_1p3_launch_data() # Create LTI Config and Deep linking object lti_config = LtiConfiguration.objects.create( - location=block.location, + location='block-v1:course+test+2020+type@problem+block@test', config_id=_test_config_id, ) LtiDlContentItem.objects.create( diff --git a/lti_consumer/tests/unit/test_lti_xblock.py b/lti_consumer/tests/unit/test_lti_xblock.py index 81178dfe379d3591632fcb27b651afea5c28ab22..0b8631cd1ab3ef0a92ff381395a86d24cd9417ee 100644 --- a/lti_consumer/tests/unit/test_lti_xblock.py +++ b/lti_consumer/tests/unit/test_lti_xblock.py @@ -130,7 +130,7 @@ class TestProperties(TestLtiConsumerXBlock): """ Test `context_id` returns unicode course id """ - self.assertEqual(self.xblock.context_id, str(self.xblock.course_id)) + self.assertEqual(self.xblock.context_id, str(self.xblock.scope_ids.usage_id.context_key)) def test_validate(self): """ @@ -292,13 +292,15 @@ class TestProperties(TestLtiConsumerXBlock): with self.assertRaises(LtiError): __ = self.xblock.external_user_id + @override_settings(LMS_BASE="edx.org") def test_resource_link_id(self): """ Test `resource_link_id` returns appropriate string """ + hostname = "edx.org" self.assertEqual( self.xblock.resource_link_id, - f"{self.xblock.runtime.hostname}-{self.xblock.location.html_id()}" # pylint: disable=no-member + f"{hostname}-{self.xblock.scope_ids.usage_id.html_id()}" ) @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.context_id') @@ -996,7 +998,11 @@ class TestResultServiceHandler(TestLtiConsumerXBlock): Test 404 response returned when a user cannot be found """ mock_parse_suffix.return_value = FAKE_USER_ID - self.xblock.runtime.get_real_user.return_value = None + self.xblock.runtime.service = Mock( + return_value=Mock( + get_user_by_anonymous_id=Mock(return_value=None) + ) + ) response = self.xblock.result_service_handler(make_request('', 'GET')) self.assertEqual(response.status_code, 404) @@ -1074,10 +1080,12 @@ class TestResultServiceHandler(TestLtiConsumerXBlock): mock_runtime = self.xblock.runtime = Mock() mock_lti_consumer = Mock() mock_user = Mock() + mock_rebind_user_service = Mock() + mock_runtime.service.return_value = mock_rebind_user_service self.xblock._result_service_get(mock_lti_consumer, mock_user) # pylint: disable=protected-access - mock_runtime.rebind_noauth_module_to_user.assert_called_with(self.xblock, mock_user) + mock_rebind_user_service.rebind_noauth_module_to_user.assert_called_with(self.xblock, mock_user) mock_lti_consumer.get_result.assert_called_with() @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.module_score', PropertyMock(return_value=0.5)) @@ -1210,12 +1218,15 @@ class TestSetScore(TestLtiConsumerXBlock): def test_rebind_called(self): """ - Test that `runtime.rebind_noauth_module_to_user` is called + Test that `rebind_noauth_module_to_user` service is called """ + mock_rebind_user_service = Mock() + self.xblock.runtime.service = Mock() + self.xblock.runtime.service.return_value = mock_rebind_user_service user = Mock(user_id=FAKE_USER_ID) self.xblock.set_user_module_score(user, 0.92, 1.0, 'Great Job!') - self.xblock.runtime.rebind_noauth_module_to_user.assert_called_with(self.xblock, user) + mock_rebind_user_service.rebind_noauth_module_to_user.assert_called_with(self.xblock, user) def test_publish_grade_event_called(self): """ @@ -1518,12 +1529,12 @@ class TestLtiConsumer1p3XBlock(TestCase): launch_data = self.xblock.get_lti_1p3_launch_data() - course_key = str(self.xblock.location.course_key) # pylint: disable=no-member + course_key = str(self.xblock.scope_ids.usage_id.course_key) expected_launch_data = Lti1p3LaunchData( user_id=1, user_role="instructor", config_id=config_id_for_block(self.xblock), - resource_link_id=str(self.xblock.location), # pylint: disable=no-member + resource_link_id=str(self.xblock.scope_ids.usage_id), external_user_id="external_user_id", launch_presentation_document_target="iframe", message_type="LtiResourceLinkRequest", diff --git a/lti_consumer/tests/unit/test_models.py b/lti_consumer/tests/unit/test_models.py index f9b189c9eee6e14b298e0c23c5d2627604df7b77..9f9a37e0f85fb6a68b16fa822141c3d887eb95fa 100644 --- a/lti_consumer/tests/unit/test_models.py +++ b/lti_consumer/tests/unit/test_models.py @@ -59,17 +59,17 @@ class TestLtiConfigurationModel(TestCase): # Creates an LTI configuration objects for testing self.lti_1p1_config = LtiConfiguration.objects.create( - location=self.xblock.location, # pylint: disable=no-member + location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P1 ) self.lti_1p3_config = LtiConfiguration.objects.create( - location=self.xblock.location, # pylint: disable=no-member + location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P3 ) self.lti_1p3_config_db = LtiConfiguration.objects.create( - location=self.xblock.location, # pylint: disable=no-member + location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P3, config_store=LtiConfiguration.CONFIG_ON_DB, lti_advantage_ags_mode='programmatic', @@ -92,7 +92,7 @@ class TestLtiConfigurationModel(TestCase): Helper function to create a LtiConfiguration object with specific attributes """ return LtiConfiguration.objects.create( - location=self.xblock.location, # pylint: disable=no-member + location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P3, **kwargs ) @@ -472,7 +472,7 @@ class TestLtiDlContentItemModel(TestCase): self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, self.xblock_attributes) self.lti_1p3_config = LtiConfiguration.objects.create( - location=self.xblock.location, # pylint: disable=no-member + location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P3 ) @@ -489,7 +489,7 @@ class TestLtiDlContentItemModel(TestCase): self.assertEqual( str(content_item), "[CONFIG_ON_XBLOCK] lti_1p3 - " - "block-v1:edX+DemoX+Demo_Course+type@problem+block@466f474fa4d045a8b7bde1b911e095ca: image" + f"{content_item.lti_configuration.location}: image" ) diff --git a/lti_consumer/tests/unit/test_outcomes.py b/lti_consumer/tests/unit/test_outcomes.py index d97d926f826436af4518df60645c79f69da9084a..dcc1e5de3462402cd9d3baeb493c00517007fa19 100644 --- a/lti_consumer/tests/unit/test_outcomes.py +++ b/lti_consumer/tests/unit/test_outcomes.py @@ -416,7 +416,11 @@ class TestOutcomeService(TestLtiConsumerXBlock): Test user not found returns failure response """ request = make_request('') - self.xblock.runtime.get_real_user.return_value = None + self.xblock.runtime.service = Mock( + return_value=Mock( + get_user_by_anonymous_id=Mock(return_value=None) + ) + ) response = self.outcome_servce.handle_request(request) self.assertIn('failure', response) diff --git a/pylintrc b/pylintrc index 6830c8681aca6d66a89c1dda130d7bd9c8985696..abec8fa62ecd7da91aafcce8d9fc90164caf0629 100644 --- a/pylintrc +++ b/pylintrc @@ -64,18 +64,18 @@ # SERIOUSLY. # # ------------------------------ -# Generated by edx-lint version: 5.2.5 +# Generated by edx-lint version: 5.3.0 # ------------------------------ [MASTER] -ignore = +ignore = persistent = yes load-plugins = edx_lint.pylint,pylint_django,pylint_celery [MESSAGES CONTROL] -enable = +enable = blacklisted-name, line-too-long, - + abstract-class-instantiated, abstract-method, access-member-before-definition, @@ -184,26 +184,26 @@ enable = used-before-assignment, using-constant-test, yield-outside-function, - + astroid-error, fatal, method-check-failed, parse-error, raw-checker-failed, - + empty-docstring, invalid-characters-in-docstring, missing-docstring, wrong-spelling-in-comment, wrong-spelling-in-docstring, - + unused-argument, unused-import, unused-variable, - + eval-used, exec-used, - + bad-classmethod-argument, bad-mcs-classmethod-argument, bad-mcs-method-argument, @@ -234,30 +234,30 @@ enable = unneeded-not, useless-else-on-loop, wrong-assert-type, - + deprecated-method, deprecated-module, - + too-many-boolean-expressions, too-many-nested-blocks, too-many-statements, - + wildcard-import, wrong-import-order, wrong-import-position, - + missing-final-newline, mixed-line-endings, trailing-newlines, trailing-whitespace, unexpected-line-ending-format, - + bad-inline-option, bad-option-value, deprecated-pragma, unrecognized-inline-option, useless-suppression, -disable = +disable = bad-indentation, consider-using-f-string, duplicate-code, @@ -281,10 +281,10 @@ disable = unspecified-encoding, unused-wildcard-import, use-maxsplit-arg, - + feature-toggle-needs-doc, illegal-waffle-usage, - + logging-fstring-interpolation, django-not-configured, @@ -329,7 +329,7 @@ ignore-imports = no ignore-mixin-members = yes ignored-classes = SQLObject unsafe-load-any-extension = yes -generated-members = +generated-members = REQUEST, acl_users, aq_parent, @@ -355,7 +355,7 @@ generated-members = [VARIABLES] init-import = no dummy-variables-rgx = _|dummy|unused|.*_unused -additional-builtins = +additional-builtins = [CLASSES] defining-attr-methods = __init__,__new__,setUp @@ -376,11 +376,11 @@ max-public-methods = 20 [IMPORTS] deprecated-modules = regsub,TERMIOS,Bastion,rexec -import-graph = -ext-import-graph = -int-import-graph = +import-graph = +ext-import-graph = +int-import-graph = [EXCEPTIONS] overgeneral-exceptions = Exception -# d1da7b0099fad5ecd841f40012c2a0bc326a4c1b +# acb0bd5fab1bee88c0721bdd68196a478007223c diff --git a/test_settings.py b/test_settings.py index c1b33d91ab7a9d5a9841f007271224ded214119a..ec88f5730e481a4ec7b001e4b8ed18160a577184 100644 --- a/test_settings.py +++ b/test_settings.py @@ -12,6 +12,7 @@ ROOT_URLCONF = 'test_urls' # LMS Urls - for LTI 1.3 testing LMS_ROOT_URL = "https://example.com" +LMS_BASE = "example.com" # Dummy FEATURES dict FEATURES = {}