Forward auto-grading settings to DL assignments #6641

marcospri · 2024-09-03T14:18:19Z

For:

Backend: Participation grading #6647

Deep linking summary:

The frontend calls the deep_linking_fields API with the assignment's config.
The BE returns a JWT that must be POSTed to the LMS client side.
That JWT contains an encoded representation of the assigned configuration. Some of those elements are encoded in and URL other are port of the launch's custom parameters.
The client POSTs the config the LMS, the assignment is created
Next launches of the assignments forward to us the configuration.

This PR handles encoding the auto grading configuration as part of the deep linked parameters.

Next steps will need to read it on launches and persist it to the DB.

Testing

Requires: Add UI components to configure assignments auto-grading #6632
Configure a LTI1.3 assignment with autograding, any parameters.
Save the config on the canvas side
Apply a diff like:

diff --git a/lms/views/lti/basic_launch.py b/lms/views/lti/basic_launch.py
index b2404a85b..c40859098 100644
--- a/lms/views/lti/basic_launch.py
+++ b/lms/views/lti/basic_launch.py
@@ -81,6 +81,11 @@ class BasicLaunchViews:
                     request=self.request, type_=LTIEvent.Type.CONFIGURED_LAUNCH
                 )
             )
+            print(
+                self.request.lti_params.v13.get(
+                    "https://purl.imsglobal.org/spec/lti/claim/custom"
+                )
+            )
             return {}
 
         # Show the file-picker for the user to choose a document.

Launch the assignment (example done in https://hypothesis.instructure.com/courses/319/assignments/7296)
You'll see the auto grading settings as part of the launch parameters.

lms/views/lti/deep_linking.py

marcospri · 2024-09-03T14:51:22Z

lms/views/lti/deep_linking.py

+    )
+
+    required_annotations = fields.Int(
+        required=False, allow_none=True, validate=validate.Range(min=0)


marcospri · 2024-09-04T11:06:23Z

lms/views/lti/deep_linking.py

+    grading_type = fields.Str(
+        required=True, validate=validate.OneOf(["all_or_nothing", "scaled"])
+    )
+    activity_calculation = fields.Str(


Changed this to "separate"

marcospri · 2024-09-04T12:45:14Z

lms/views/lti/deep_linking.py

+        required=True, validate=validate.OneOf(["cumulative", "separate"])
+    )
+
+    required_annotations = fields.Int(required=True, validate=validate.Range(min=0))


required_annotations always required

acelaya

I noticed the tests are only covering the usage of invalid values, but in that case I would expect the test to fail.

acelaya · 2024-09-04T13:00:48Z

tests/unit/lms/views/lti/deep_linking_test.py

@@ -211,6 +211,10 @@ def test_it_for_v11(
                {"group_set": "1", "title": "title"},
                {"group_set": "1", "title": "title"},
            ),
+            (
+                {"auto_grading_config": {"key": "value"}},
+                {"auto_grading_config": '{"key": "value"}'},


Are we not validating that this fulfills _AutoGradingConfig definition? 🤔

I would expect these values to be invalid.

Yes, good point but I'm afraid I don't a satisfactory answer.

The method here is unaware of the types, it just passes along what it receives, and in the case of these settings it serlizes it as json. That's what the test does. I included the private name of the method in the test name (one extra _)

The marshmallow schema is validated before we get into the the code of individual views and it gets serialized into request.parsed_params, if that not valid the view is never called.

That being said I added a very broad valid/invalid tests for the schema itself 👍

The marshmallow schema is validated before we get into the the code of individual views and it gets serialized into request.parsed_params, if that not valid the view is never called.

Oh, I see 👍🏼

marcospri · 2024-09-04T14:53:07Z

tests/unit/lms/views/lti/deep_linking_test.py

 from tests import factories


+class TestDeepLinkingFieldsRequestSchema:


Testing just the schema here.

marcospri · 2024-09-04T14:53:17Z

tests/unit/lms/views/lti/deep_linking_test.py

+                "content_item_return_url": "return_url",
+                "auto_grading_config": {
+                    "grading_type": "scaled",
+                    "activity_calculation": "RANDOM",


Best type of grading

marcospri · 2024-09-04T14:53:51Z

tests/unit/lms/views/lti/deep_linking_test.py

        ],
    )
-    def test_get_assignment_configuration(
+    def test__get_assignment_configuration(


Fixed this test's name, it testing _get_assignment_configuration not get_assignment_configuration

marcospri commented Sep 3, 2024

View reviewed changes

lms/views/lti/deep_linking.py Outdated Show resolved Hide resolved

marcospri commented Sep 3, 2024

View reviewed changes

marcospri mentioned this pull request Sep 4, 2024

Backend: Participation grading #6647

Closed

2 tasks

acelaya force-pushed the auto-grading-config branch 4 times, most recently from e25c841 to 23e49b8 Compare September 4, 2024 10:22

marcospri force-pushed the store-auto-grading-deep-linking branch 2 times, most recently from ebd9a53 to 796a974 Compare September 4, 2024 10:45

marcospri marked this pull request as ready for review September 4, 2024 10:46

marcospri requested a review from acelaya September 4, 2024 10:48

marcospri force-pushed the store-auto-grading-deep-linking branch from 796a974 to bbebde4 Compare September 4, 2024 10:50

marcospri commented Sep 4, 2024

View reviewed changes

acelaya force-pushed the auto-grading-config branch from 23e49b8 to bf7bb91 Compare September 4, 2024 11:45

Base automatically changed from auto-grading-config to main September 4, 2024 11:49

marcospri force-pushed the store-auto-grading-deep-linking branch from bbebde4 to f1d5893 Compare September 4, 2024 12:42

marcospri commented Sep 4, 2024

View reviewed changes

acelaya reviewed Sep 4, 2024

View reviewed changes

acelaya mentioned this pull request Sep 4, 2024

Improve type coverage for auto-grading #6655

Merged

Forward auto-grading settings to DL assignments

88e5b0d

marcospri force-pushed the store-auto-grading-deep-linking branch from f1d5893 to 88e5b0d Compare September 4, 2024 14:47

marcospri commented Sep 4, 2024

View reviewed changes

marcospri requested a review from acelaya September 4, 2024 14:54

acelaya approved these changes Sep 4, 2024

View reviewed changes

marcospri merged commit b7f7bea into main Sep 9, 2024
9 checks passed

marcospri deleted the store-auto-grading-deep-linking branch September 9, 2024 07:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Forward auto-grading settings to DL assignments #6641

Forward auto-grading settings to DL assignments #6641

marcospri commented Sep 3, 2024 •

edited

Loading

marcospri Sep 3, 2024

marcospri Sep 4, 2024

marcospri Sep 4, 2024

acelaya left a comment

acelaya Sep 4, 2024

marcospri Sep 4, 2024

acelaya Sep 4, 2024

marcospri Sep 4, 2024

marcospri Sep 4, 2024

marcospri Sep 4, 2024 •

edited

Loading

		from tests import factories


		class TestDeepLinkingFieldsRequestSchema:

Forward auto-grading settings to DL assignments #6641

Forward auto-grading settings to DL assignments #6641

Conversation

marcospri commented Sep 3, 2024 • edited Loading

Deep linking summary:

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acelaya left a 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

marcospri Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

marcospri commented Sep 3, 2024 •

edited

Loading

marcospri Sep 4, 2024 •

edited

Loading