PADV-1191: Add DeepLinkingForm to DeepLinkingView. #35

kuipumu · 2024-05-16T21:03:13Z

Ticket

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

Description

This PR add the DeepLinkingForm to the DeepLinkingFormView that was created on the PR for PADV-1190. This PR handles the second and third step on the Deep Linking UI process.

Type of Change

Add deep_linking/forms.py module.
Add learning_sequences edxapp_wrapper backend.
Add DeepLinkingForm Django form.
Add DeepLinkingForm to DeepLinkingFormView Django view logic.
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: https://lms.ngrok.io/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: https://lms.ngrok.io/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"]

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.ngrok.io/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.ngrok.io/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".
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-21T14:57:41Z

openedx_lti_tool_plugin/edxapp_wrapper/backends/learning_sequences_o_v1.py

@@ -0,0 +1,8 @@
+"""learning_sequences module backend (olive v1)."""


@kuipumu Why is this olive v1?

@anfbermudezme This is following the same formula from previously defined edx_wrapper backends, which was also done following the structure from course_operations. I'm not sure what could be done to improve this but the (olive v1) comment is just to make clear that the o_v1 suffix on the module name refers to Olive and that this is the v1 of such backend.

@kuipumu Understood I thought that was the last stable version of olive https://github.com/openedx/edx-platform/tree/open-release/olive.4.

anfbermudezme · 2024-05-21T15:03:23Z

openedx_lti_tool_plugin/deep_linking/tests/test_forms.py

+    @patch.object(DeepLinkingForm, '__init__', return_value=None)
+    def test_get_deep_link_resources(
+        self,
+        deep_linking_form_init: MagicMock,  # pylint: disable=W0613


@kuipumu Please use the explicit pylint title.

Suggested change

deep_linking_form_init: MagicMock, # pylint: disable=W0613

deep_linking_form_init: MagicMock, # pylint: disable=unused-argument

anfbermudezme · 2024-05-21T15:06:07Z

openedx_lti_tool_plugin/deep_linking/tests/test_forms.py

+    @patch.object(DeepLinkingForm, '__init__', return_value=None)
+    def test_get_content_items_choice(
+        self,
+        deep_linking_form_init: MagicMock,  # pylint: disable=W0613


@kuipumu Please use the explicit pylint title.

anfbermudezme · 2024-05-21T15:08:14Z

openedx_lti_tool_plugin/deep_linking/tests/test_forms.py

+    @patch.object(DeepLinkingForm, '__init__', return_value=None)
+    def test_get_content_items_choices(
+        self,
+        deep_linking_form_init: MagicMock,  # pylint: disable=W0613


@kuipumu Please use the explicit pylint title.

anfbermudezme · 2024-05-21T15:11:27Z

openedx_lti_tool_plugin/deep_linking/forms.py

+            List of DeepLinkResource objects or an empty list
+
+        """
+        return [


@kuipumu It is necessary to use a list, what would happen if a set were used?

@anfbermudezme It will work the same, the return just needs to be iterable, I will change it to a set since avoids having multiple occurrences and we don't care about order in this case, however both a list or a set will work in this case

alexjmpb · 2024-05-21T22:46:58Z

openedx_lti_tool_plugin/deep_linking/forms.py

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


This query can become pretty big, do you think it is worth to implement a more robust field? One that has a search option and progressive loading? or maybe only retrieve the courses which are specified in a site configuration.

@alexjmpb Right now there is this other PR that will separate the queryset into another method of this form: #36

I do think we need to come up with a more robust way to handle this form since there seems to be the future requirement to filter per institutions, licenses or even select unit or components, I'm thinking on using a more dynamic form using a JSONField and a custom form template, I'm still not sure of what will be the UI requirements so for now I think is OK to handle it has we are doing on #36.

What do you think?, do you have any resources that could be useful to create a more dynamic form?.

No I was worried of not applying a filter on this since it would query all the courses in prod. But #36 does the job, maybe it's worth discussing later if a select field with a search bar can be implemented.

kuipumu requested review from anfbermudezme and alexjmpb May 16, 2024 21:03

anfbermudezme requested changes May 21, 2024

View reviewed changes

anfbermudezme previously approved these changes May 21, 2024

View reviewed changes

kuipumu dismissed anfbermudezme’s stale review via 3adfbab May 21, 2024 20:13

kuipumu self-assigned this May 21, 2024

alexjmpb reviewed May 21, 2024

View reviewed changes

alexjmpb approved these changes May 22, 2024

View reviewed changes

feat: Add DeepLinkingForm to DeepLinkingView.

08015cf

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

kuipumu merged commit 234bf29 into main May 22, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PADV-1191: Add DeepLinkingForm to DeepLinkingView. #35

PADV-1191: Add DeepLinkingForm to DeepLinkingView. #35

kuipumu commented May 16, 2024 •

edited

Loading

anfbermudezme May 21, 2024

kuipumu May 21, 2024

anfbermudezme May 21, 2024

anfbermudezme May 21, 2024

anfbermudezme May 21, 2024

anfbermudezme May 21, 2024

anfbermudezme May 21, 2024

kuipumu May 21, 2024

alexjmpb May 21, 2024

kuipumu May 22, 2024

alexjmpb May 22, 2024

		@@ -0,0 +1,8 @@
		"""learning_sequences module backend (olive v1)."""

	deep_linking_form_init: MagicMock, # pylint: disable=W0613
	deep_linking_form_init: MagicMock, # pylint: disable=unused-argument

PADV-1191: Add DeepLinkingForm to DeepLinkingView. #35

PADV-1191: Add DeepLinkingForm to DeepLinkingView. #35

Conversation

kuipumu commented May 16, 2024 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kuipumu commented May 16, 2024 •

edited

Loading