From d89aab87a7f0c54e4386bf5e1cf880c860ab09b4 Mon Sep 17 00:00:00 2001 From: Marcos Prieto Date: Wed, 4 Sep 2024 17:53:12 +0200 Subject: [PATCH] Update auto grading config based on DL configuration --- lms/models/__init__.py | 2 +- lms/product/canvas/_plugin/misc.py | 22 ++++++++-- lms/product/plugin/misc.py | 4 +- lms/services/assignment.py | 32 +++++++++++++- .../lms/product/canvas/_plugin/misc_test.py | 31 ++++++++++---- tests/unit/lms/services/assignment_test.py | 42 ++++++++++++++++++- 6 files changed, 116 insertions(+), 17 deletions(-) diff --git a/lms/models/__init__.py b/lms/models/__init__.py index bd67960c53..04996cd473 100644 --- a/lms/models/__init__.py +++ b/lms/models/__init__.py @@ -1,6 +1,6 @@ from lms.models._mixins import CreatedUpdatedMixin from lms.models.application_instance import ApplicationInstance, ApplicationSettings -from lms.models.assignment import Assignment +from lms.models.assignment import Assignment, AutoGradingConfig from lms.models.assignment_grouping import AssignmentGrouping from lms.models.assignment_membership import AssignmentMembership from lms.models.course_groups_exported_from_h import CourseGroupsExportedFromH diff --git a/lms/product/canvas/_plugin/misc.py b/lms/product/canvas/_plugin/misc.py index 68a1cd2e30..7ea36aabc8 100644 --- a/lms/product/canvas/_plugin/misc.py +++ b/lms/product/canvas/_plugin/misc.py @@ -1,7 +1,10 @@ +import json import re from functools import lru_cache +from typing import cast from urllib.parse import unquote, urlencode, urlparse +from lms.js_config_types import AutoGradingConfig from lms.models import Assignment from lms.product.plugin.misc import AssignmentConfig, MiscPlugin from lms.services.vitalsource import VSBookLocation @@ -46,12 +49,22 @@ def get_assignment_configuration( ) -> AssignmentConfig: document_url = self._get_document_url(request) - return { - "document_url": document_url, + assignment_config = AssignmentConfig( + document_url=document_url, # For canvas we add parameter to the launch URL as we don't store the # assignment during deep linking. - "group_set_id": request.params.get("group_set"), - } + group_set_id=request.params.get("group_set"), + ) + + if auto_grading_config := self.get_deep_linked_assignment_configuration( + request + ).get("auto_grading_config"): + # Auto grading is a complex structure, deserialize it before hand + assignment_config["auto_grading_config"] = cast( + AutoGradingConfig, json.loads(auto_grading_config) + ) + + return assignment_config @lru_cache(1) def _get_document_url(self, request) -> str | None: @@ -133,6 +146,7 @@ def get_deep_linked_assignment_configuration(self, request) -> dict: possible_parameters = [ "group_set", + "auto_grading_config", # VS, legacy method "vitalsource_book", "book_id", diff --git a/lms/product/plugin/misc.py b/lms/product/plugin/misc.py index 721e562af6..a7e0f1458c 100644 --- a/lms/product/plugin/misc.py +++ b/lms/product/plugin/misc.py @@ -1,7 +1,8 @@ -from typing import TypedDict +from typing import NotRequired, TypedDict from pyramid.request import Request +from lms.js_config_types import AutoGradingConfig from lms.models import Assignment, LTIParams, LTIRegistration from lms.services.html_service import strip_html_tags @@ -9,6 +10,7 @@ class AssignmentConfig(TypedDict): document_url: str | None group_set_id: str | None + auto_grading_config: NotRequired[AutoGradingConfig | None] class MiscPlugin: diff --git a/lms/services/assignment.py b/lms/services/assignment.py index 08e5aeaa65..9dbe11fcf1 100644 --- a/lms/services/assignment.py +++ b/lms/services/assignment.py @@ -9,6 +9,7 @@ Assignment, AssignmentGrouping, AssignmentMembership, + AutoGradingConfig, Course, Grouping, LTIRole, @@ -53,13 +54,14 @@ def create_assignment(self, tool_consumer_instance_guid, resource_link_id): return assignment - def update_assignment( # noqa: PLR0913 + def update_assignment( # noqa: PLR0913, PLR0917 self, request, assignment: Assignment, document_url: str, group_set_id, course: Course, + auto_grading_config: dict | None = None, ): """Update an existing assignment.""" if self._misc_plugin.is_speed_grader_launch(request): @@ -84,6 +86,15 @@ def update_assignment( # noqa: PLR0913 ) assignment.course_id = course.id + if auto_grading_config: + if not assignment.auto_grading_config: + assignment.auto_grading_config = AutoGradingConfig() + self._db.add(assignment.auto_grading_config) + + assignment.auto_grading_config = self._update_auto_grading_config( + assignment.auto_grading_config, auto_grading_config + ) + return assignment def _get_copied_from_assignment(self, lti_params) -> Assignment | None: @@ -137,6 +148,7 @@ def get_assignment_for_launch(self, request, course: Course) -> Assignment | Non ) document_url = assignment_config.get("document_url") group_set_id = assignment_config.get("group_set_id") + auto_grading_config = assignment_config.get("auto_grading_config") if not document_url: # We can't find a document_url, we shouldn't try to create an @@ -177,7 +189,7 @@ def get_assignment_for_launch(self, request, course: Course) -> Assignment | Non # It often will be the same one while launching the assignment again but # it might for example be an updated deep linked URL or similar. return self.update_assignment( - request, assignment, document_url, group_set_id, course + request, assignment, document_url, group_set_id, course, auto_grading_config ) def upsert_assignment_membership( @@ -316,6 +328,22 @@ def get_courses_assignments_count(self, **kwargs) -> dict[int, int]: return {x.course_id: x.count for x in self._db.execute(query)} # type: ignore + def _update_auto_grading_config( + self, auto_grading_model: AutoGradingConfig, auto_grading_config: dict + ) -> AutoGradingConfig: + auto_grading_model.activity_calculation = auto_grading_config.get( + "activity_calculation" + ) + auto_grading_model.grading_type = auto_grading_config.get("grading_type") + auto_grading_model.required_annotations = auto_grading_config[ + "required_annotations" + ] + auto_grading_model.required_replies = auto_grading_config.get( + "required_replies" + ) + + return auto_grading_model + def factory(_context, request): return AssignmentService(db=request.db, misc_plugin=request.product.plugin.misc) diff --git a/tests/unit/lms/product/canvas/_plugin/misc_test.py b/tests/unit/lms/product/canvas/_plugin/misc_test.py index 38a836fbb6..169d26f7b7 100644 --- a/tests/unit/lms/product/canvas/_plugin/misc_test.py +++ b/tests/unit/lms/product/canvas/_plugin/misc_test.py @@ -48,6 +48,20 @@ def test_post_launch_assignment_hook( else: js_config.set_focused_user.assert_not_called() + def test_get_assignment_configuration_with_auto_grading_config( + self, plugin, pyramid_request + ): + pyramid_request.params["auto_grading_config"] = '{"some":"value"}' + config = plugin.get_assignment_configuration( + pyramid_request, sentinel.assignment, sentinel.historical_assignment + ) + + assert config == { + "document_url": None, + "group_set_id": None, + "auto_grading_config": {"some": "value"}, + } + def test_get_assignment_configuration(self, plugin, pyramid_request): config = plugin.get_assignment_configuration( pyramid_request, sentinel.assignment, sentinel.historical_assignment @@ -164,22 +178,23 @@ def test_get_deeplinking_launch_url(self, plugin, pyramid_request): == "http://example.com/lti_launches?param=value" ) - @pytest.mark.parametrize("url_param", (None, sentinel.from_url)) + @pytest.mark.parametrize("parameter", ["group_set", "auto_grading_config", "url"]) + @pytest.mark.parametrize("request_param", (None, sentinel.from_url)) @pytest.mark.parametrize("custom_param", (None, sentinel.from_custom)) def test_get_deep_linked_assignment_configuration( - self, plugin, pyramid_request, url_param, custom_param + self, plugin, pyramid_request, request_param, custom_param, parameter ): - pyramid_request.params["url"] = url_param - pyramid_request.lti_params["custom_url"] = custom_param + pyramid_request.params[parameter] = request_param + pyramid_request.lti_params[f"custom_{parameter}"] = custom_param result = plugin.get_deep_linked_assignment_configuration(pyramid_request) - if url_param: - assert result["url"] == sentinel.from_url + if request_param: + assert result[parameter] == sentinel.from_url elif custom_param: - assert result["url"] == sentinel.from_custom + assert result[parameter] == sentinel.from_custom else: - assert "url" not in result + assert parameter not in result @pytest.mark.parametrize( "get,expected", [({}, False), ({"learner_canvas_user_id": "ID"}, True)] diff --git a/tests/unit/lms/services/assignment_test.py b/tests/unit/lms/services/assignment_test.py index ab57135492..c4959e1214 100644 --- a/tests/unit/lms/services/assignment_test.py +++ b/tests/unit/lms/services/assignment_test.py @@ -4,7 +4,13 @@ import pytest from h_matchers import Any -from lms.models import AssignmentGrouping, AssignmentMembership, RoleScope, RoleType +from lms.models import ( + AssignmentGrouping, + AssignmentMembership, + AutoGradingConfig, + RoleScope, + RoleType, +) from lms.services.assignment import AssignmentService, factory from tests import factories @@ -60,6 +66,40 @@ def test_update_assignment( assert assignment.title == title assert assignment.course_id == course.id + def test_update_assignment_updates_auto_grading_config( + self, + svc, + pyramid_request, + course, + ): + assignment = factories.Assignment( + auto_grading_config=AutoGradingConfig( + activity_calculation="separate", + grading_type="scaled", + required_annotations=1, + required_replies=1, + ) + ) + + assignment = svc.update_assignment( + pyramid_request, + assignment, + sentinel.document_url, + sentinel.group_set_id, + course, + auto_grading_config={ + "activity_calculation": "combined", + "grading_type": "all_or_nothing", + "required_annotations": 10, + "required_replies": 10, + }, + ) + + assert assignment.auto_grading_config.activity_calculation == "combined" + assert assignment.auto_grading_config.grading_type == "all_or_nothing" + assert assignment.auto_grading_config.required_annotations == 10 + assert assignment.auto_grading_config.required_replies == 10 + @pytest.mark.parametrize( "param", (