PADV-1201: Filter courses in DeepLinkingForm #36

kuipumu · 2024-05-22T13:49:28Z

Ticket

https://agile-jira.pearson.com/browse/PADV-1201

Description

This PR adds a filter to the courses available on the DeepLinkingForm. This PR adds a DeepLinkingForm method that either returns all CourseContext objects with all courses on the LMS or a filtered CourseContext queryset with only the courses available per LtiTool using the CourseAccesConfiguration allowed_course_ids field.

Type of Change

Add get_course_contexts method to DeepLinkingForm Django form.
Modify get_content_items_choices method in the DeepLinkingForm Django form.
Add or modify all tests related to this code.

Testing:

Run the LMS: make dev.up.lms.
Install openedx_lti_tool_plugin on the LMS.
Run ngrok or any other similar service on LMS: ngrok http 18000
Add required settings to LMS.

# Enable LTI Tool Provider Plugin.
OLTITP_ENABLE_LTI_TOOL = True

Create a new LTI 1.3 tool key config: http://localhost:18000/admin/lti1p3_tool_config/ltitoolkey/add/ (You can use IMS RI to create a key pair: https://lti-ri.imsglobal.org/keygen/index)
Create a new LTI 1.3 tool config: http://localhost:18000/admin/lti1p3_tool_config/ltitool/add/

Issuer: https://saltire.lti.app/platform
Client id: saltire.lti.app
Auth login: url: https://saltire.lti.app/platform/auth
Auth token url: https://saltire.lti.app/platform/token/sda42bb0cf2352259a367a404d48d54e8
Key set url: https://saltire.lti.app/platform/jwks/sda42bb0cf2352259a367a404d48d54e8
Deployment ids: ["cLWwj9cbmkSrCNsckEFBmA"]

Enable the Django Waffle switch: openedx_lti_tool_plugin.course_access_configuration.
Go to edit the course access configuration of the previously created LTI 1.3 tool config: http://localhost:18000/admin/openedx_lti_tool_plugin/courseaccessconfiguration/
Edit the "Allowed Course IDs" field and add the courses that should be allowed.
Go to the saLTIre platform https://saltire.lti.app/platform
Go to "Security Model" on the left sidebar and set these settings:

Message URL: https://lms-dstack.kuje.net.eu.org/openedx_lti_tool_plugin/1.3/deep_linking/
Initiate login URL: https://lms.ngrok.io/openedx_lti_tool_plugin/1.3/login
Redirection URI(s): https://lms-dstack.kuje.net.eu.org/openedx_lti_tool_plugin/1.3/deep_linking/
Public keyset URL: https://lms.ngrok.io/openedx_lti_tool_plugin/1.3/pub/jwks

Go to "Message" on the left sidebar and set these settings:

Message type: LtiDeepLinkingRequest

Click on the "Save" button on the top navbar.
Click on the dropdown next to the "Connect" button on the top navbar.
Click on "Open in new window".
You should be presented with a form with only the courses that where added to the course access configuration "Allowed Course IDs" field.
Select one or more courses from the deep linking UI form.
Click on the "Submit" button.
The browser should be redirected back to saLTIre,
The "Sumary" section should show "Verification: Passed" and the "JWT" section should contain the requested courses on the "https://purl.imsglobal.org/spec/lti-dl/claim/content_items" key.

anfbermudezme · 2024-05-23T15:11:52Z

openedx_lti_tool_plugin/deep_linking/forms.py

        Returns:
            List of tuples with choices for the `content_items` field.

        """
        return [
-            self.get_content_items_choice(course, request)
-            for course in course_context().objects.all()
+            self.get_content_items_choice(course)


@kuipumu Same question of the last PR review, Is it necessary that the return of the function be a list? I mention it because of the performance where for example, searching through 100,000 items takes 49.663 seconds with a list, but only 0.007 seconds with a set, taking into account that there are no repeated values in this array.

@anfbermudezme I didn't tried with a set since the Django documentation specifies the choices field to be a list of tuples, however it works with a set so I change this.

anfbermudezme · 2024-05-23T15:15:42Z

openedx_lti_tool_plugin/deep_linking/views.py

            validate_deep_linking_message(message)
-
+            # Redirect to DeepLinkingForm view.


@kuipumu Add blank line.

@anfbermudezme I don't think is necessary to add a blank line

anfbermudezme · 2024-05-23T15:15:55Z

openedx_lti_tool_plugin/deep_linking/views.py

-
+            # Get identity claims from launch data.
+            iss, aud, _sub, _pii = get_identity_claims(message.get_launch_data())
+            # Render form template.


@kuipumu Add blank line.

@anfbermudezme I don't think is necessary to add a blank line

openedx_lti_tool_plugin/deep_linking/views.py

anfbermudezme · 2024-05-23T15:16:42Z

openedx_lti_tool_plugin/resource_link_launch/views.py

@@ -99,8 +99,12 @@ def post(
                    _('Message type is not LtiResourceLinkRequest.'),
                )
            # Get launch data.
+            # TODO: Replace redundant get_launch_data method with


@kuipumu There are already tickets for these TODO?

@anfbermudezme Just created PADV-1338 so there is also a reference on the backlog for these TODOs.

anfbermudezme · 2024-05-23T15:17:47Z

openedx_lti_tool_plugin/utils.py

+        launch_data: Launch data dictionary.
+
+    Returns:
+        A tuple containing the `iss`, `aud`, `sub` and PII claims.


@kuipumu Add the reference of PII in the docstring please.

kuipumu requested review from anfbermudezme and alexjmpb May 22, 2024 13:49

kuipumu self-assigned this May 22, 2024

kuipumu mentioned this pull request May 22, 2024

PADV-1191: Add DeepLinkingForm to DeepLinkingView. #35

Merged

5 tasks

kuipumu force-pushed the vue/PADV-1191 branch from 3adfbab to 08015cf Compare May 22, 2024 15:38

kuipumu force-pushed the vue/PADV-1201 branch from 3d289c5 to 3eaa6c0 Compare May 22, 2024 15:52

kuipumu changed the base branch from vue/PADV-1191 to main May 22, 2024 15:52

anfbermudezme requested changes May 23, 2024

View reviewed changes

anfbermudezme approved these changes May 23, 2024

View reviewed changes

alexjmpb approved these changes May 23, 2024

View reviewed changes

feat: Add CourseAccessConfiguration filter to DeepLinkingForm

11ea567

kuipumu force-pushed the vue/PADV-1201 branch from 9acdd92 to 11ea567 Compare May 23, 2024 20:34

kuipumu merged commit 456a76a into main May 23, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PADV-1201: Filter courses in DeepLinkingForm #36

PADV-1201: Filter courses in DeepLinkingForm #36

kuipumu commented May 22, 2024

anfbermudezme May 23, 2024

kuipumu May 23, 2024

anfbermudezme May 23, 2024

kuipumu May 23, 2024

anfbermudezme May 23, 2024

kuipumu May 23, 2024

anfbermudezme May 23, 2024

kuipumu May 23, 2024

anfbermudezme May 23, 2024

		validate_deep_linking_message(message)

		# Redirect to DeepLinkingForm view.

PADV-1201: Filter courses in DeepLinkingForm #36

PADV-1201: Filter courses in DeepLinkingForm #36

Conversation

kuipumu commented May 22, 2024

Ticket

Description

Type of Change

Testing:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment