PADV-1528: Create ViewSet CourseContentItemViewSet #41

kuipumu · 2024-08-12T13:43:08Z

Ticket

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

Description

This PR adds a CourseContentItemViewSet ViewSet to list all the Course content items available for a launch ID. This ViewSet will be used to render a paginated form table in the DeepLinkingFormView template.

Additionally this PR also includes an upgrade to the project requirements.

Type of Change

Upgrade project requirements.
Move pylti1.3 attributes and functions used on LTI Tool Views to a LTIToolMixin class.
Create LTIToolView that inherits from LTIToolMixin and View class.
Replace views using LtiToolBaseView with LTIToolView.
Add djangorestframework to project requirements.
Add base setup for Django REST Framework API.
Add CourseContentItemViewSet.
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".
You should be redirected to the Deep Linking Form View.
Copy the launch ID from the browser URL (Example: https://lms.ngrok.io/openedx_lti_tool_plugin/1.3/deep_linking//).
Go to the URL https://lms.ngrok.io/openedx_lti_tool_plugin/1.3/deep_linking/api/v1//content_items/courses
You should be presented with a list of paginated content items for all the courses available for the LtiTool.

anfbermudezme · 2024-08-14T10:07:39Z

openedx_lti_tool_plugin/deep_linking/api/v1/views.py

+    max_page_size = 100
+
+
+class CourseContentItemViewSet(


@kuipumu Aren't authentication_classes and permission_classes necessary?

@anfbermudezme I forgot to add the JwtAuthentication class, but there are no permission classes necessary since this API endpoint is used by unauthenticated users.

anfbermudezme · 2024-08-14T10:08:45Z

openedx_lti_tool_plugin/deep_linking/api/v1/views.py

+from openedx_lti_tool_plugin.utils import get_identity_claims
+
+
+class ContentItemPagination(PageNumberPagination):


@kuipumu How about having a separate module for pagination? (pagination.py)

anfbermudezme · 2024-08-14T10:09:56Z

openedx_lti_tool_plugin/deep_linking/api/v1/views.py

+            CourseContext QuerySet.
+
+        """
+        iss, aud, _sub, _pii = get_identity_claims(self.launch_data)


@kuipumu Please explain what is iss, aud, _sub, _pii.

@anfbermudezme The get_identity_claims function has a docstring with references to the OpenID Connect Core 1.0 ID token claims and standard claims, I will add a comment in this line to explain why we are obtaining the iss and aud claim

anfbermudezme · 2024-08-14T10:10:33Z

openedx_lti_tool_plugin/deep_linking/api/v1/views.py

+        """
+        iss, aud, _sub, _pii = get_identity_claims(self.launch_data)
+
+        return CourseContext.objects\


@kuipumu

Suggested change

return CourseContext.objects\

return CourseContext.objects.all_for_lti_tool(iss, aud).filter_by_site_orgs()

anfbermudezme · 2024-08-14T10:13:06Z

openedx_lti_tool_plugin/deep_linking/api/views.py

+from openedx_lti_tool_plugin.views import requires_plugin_enabled
+
+
+@method_decorator(requires_plugin_enabled, name='dispatch')


Suggested change

@method_decorator(requires_plugin_enabled, name='dispatch')

@method_decorator(requires_plugin_enabled, name='dispatch')

@kuipumu The name of this decorator is too general. Which plugin is required to be enabled?

alexjmpb · 2024-08-14T20:13:18Z

openedx_lti_tool_plugin/deep_linking/api/views.py

+
+
+@method_decorator(requires_openedx_lti_tool_plugin_enabled, name='dispatch')
+class DeepLinkingViewSet(


Wouldn't this be more of a base view?

@alexjmpb It's a base view, I followed a similar structure found in rest_framework/viewsets.py for other ViewSets such has ModelViewSet, are you suggesting we use a different name for this View or are you suggesting a different structure for the API Views?

Yes it was more of a naming question, but it's not that relevant.

alexjmpb · 2024-08-14T20:15:37Z

openedx_lti_tool_plugin/models.py

@@ -274,10 +275,10 @@ def __str__(self) -> str:
        return f'<CourseAccessConfiguration, ID: {self.id}>'


-class CourseContextManager(models.Manager):
-    """CourseContext Model Manager."""
+class CourseContextQuerySet(models.QuerySet):


Why did you decided to change this to a queryset?

@alexjmpb I changed it to a queryset so I was able to chain the all_for_lti_tool and filter_by_site_orgs querysets, with a Manager I would have to create a Queryset and duplicate the methods of such Queryset in the manager, the as_manager method does that for you automatically: https://docs.djangoproject.com/en/5.0/topics/db/managers/#creating-a-manager-with-queryset-methods

refactor: LTI Tool Views feat: Upgrade requirements

kuipumu requested review from anfbermudezme and alexjmpb August 12, 2024 13:43

kuipumu mentioned this pull request Aug 14, 2024

PADV-1529: Modify DeepLinkingForm, DeepLinkingFormView #42

Merged

9 tasks

anfbermudezme requested changes Aug 14, 2024

View reviewed changes

alexjmpb reviewed Aug 14, 2024

View reviewed changes

alexjmpb approved these changes Aug 16, 2024

View reviewed changes

anfbermudezme approved these changes Aug 16, 2024

View reviewed changes

feat: Add CourseContentItemViewSet

bed034c

refactor: LTI Tool Views feat: Upgrade requirements

kuipumu force-pushed the vue/PADV-1528 branch from 86739ee to bed034c Compare August 19, 2024 13:45

kuipumu merged commit aa1c27c into main Aug 19, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PADV-1528: Create ViewSet CourseContentItemViewSet #41

PADV-1528: Create ViewSet CourseContentItemViewSet #41

kuipumu commented Aug 12, 2024 •

edited

Loading

anfbermudezme Aug 14, 2024

kuipumu Aug 14, 2024

anfbermudezme Aug 14, 2024

anfbermudezme Aug 14, 2024

kuipumu Aug 14, 2024

anfbermudezme Aug 14, 2024

anfbermudezme Aug 14, 2024

alexjmpb Aug 14, 2024

kuipumu Aug 14, 2024

alexjmpb Aug 15, 2024

alexjmpb Aug 14, 2024

kuipumu Aug 14, 2024

		from openedx_lti_tool_plugin.utils import get_identity_claims


		class ContentItemPagination(PageNumberPagination):

	return CourseContext.objects\
	return CourseContext.objects.all_for_lti_tool(iss, aud).filter_by_site_orgs()

		from openedx_lti_tool_plugin.views import requires_plugin_enabled


		@method_decorator(requires_plugin_enabled, name='dispatch')



		@method_decorator(requires_openedx_lti_tool_plugin_enabled, name='dispatch')
		class DeepLinkingViewSet(

PADV-1528: Create ViewSet CourseContentItemViewSet #41

PADV-1528: Create ViewSet CourseContentItemViewSet #41

Conversation

kuipumu commented Aug 12, 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

Choose a reason for hiding this comment

kuipumu commented Aug 12, 2024 •

edited

Loading