From 036f61da49d988b6a05380acfaccd26278c8cdd8 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Fri, 9 Aug 2024 15:09:54 +0800 Subject: [PATCH 1/2] MDL-75947 ltiservice_gradebookservices: test sequential score posting This tests highlights a problem with the score handling logic and will currently fail. On score post, the current time is used for grade->timemodified instead of the score->timestamp provided by the tool in the POST JSON. --- .../gradebookservices/tests/lineitem_test.php | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/mod/lti/service/gradebookservices/tests/lineitem_test.php b/mod/lti/service/gradebookservices/tests/lineitem_test.php index ad5f8be601d..32e9237a3f8 100644 --- a/mod/lti/service/gradebookservices/tests/lineitem_test.php +++ b/mod/lti/service/gradebookservices/tests/lineitem_test.php @@ -17,6 +17,8 @@ namespace ltiservice_gradebookservices; use ltiservice_gradebookservices\local\resources\lineitem; +use ltiservice_gradebookservices\local\resources\results; +use ltiservice_gradebookservices\local\resources\scores; use ltiservice_gradebookservices\local\service\gradebookservices; /** @@ -174,6 +176,97 @@ final class lineitem_test extends \advanced_testcase { $this->assertFalse(isset($responseitem->submissionReview->custom)); } + /** + * Test running a series of score updates, highlighting problems with the score posting logic. + * + * @covers ::execute + * + * @return void + */ + public function test_sequential_score_posts(): void { + global $CFG; + require_once($CFG->dirroot . '/mod/lti/locallib.php'); + $this->resetAfterTest(); + $resourceid = 'test-resource-id'; + $tag = 'tag'; + $course = $this->getDataGenerator()->create_course(); + $typeid = $this->create_type(); + $user = $this->getDataGenerator()->create_and_enrol($course); + + // Create mod instance with line item - nothing pushed via services yet. + $gbservice = new gradebookservices(); + $gbservice->set_type(lti_get_type($typeid)); + $modinstance = $this->create_graded_lti($typeid, $course, $resourceid, $tag); + $gradeitems = $gbservice->get_lineitems($course->id, null, null, null, null, null, $typeid); + $this->assertEquals(1, $gradeitems[0]); // The 1st item in the array is the items count. + + // Post a score so that there's at least one grade present at the time of lineitem update. + $score = new scores($gbservice); + $_SERVER['REQUEST_METHOD'] = \mod_lti\local\ltiservice\resource_base::HTTP_POST; + $_SERVER['PATH_INFO'] = "/$course->id/lineitems/{$gradeitems[1][0]->id}/lineitem/scores?type_id=$typeid"; + $token = lti_new_access_token($typeid, ['https://purl.imsglobal.org/spec/lti-ags/scope/score']); + $_SERVER['HTTP_Authorization'] = 'Bearer '.$token->token; + $_GET['type_id'] = (string)$typeid; + $score->scoreGiven = "8.0"; + $score->scoreMaximum = "10.0"; + $score->activityProgress = "Completed"; + $score->timestamp = "2024-08-07T18:54:36.736+00:00"; + $score->gradingProgress = "FullyGraded"; + $score->userId = $user->id; + $response = new \mod_lti\local\ltiservice\response(); + $response->set_content_type('application/vnd.ims.lis.v1.score+json'); + $response->set_request_data(json_encode($score)); + $score->execute($response); + + // The grade in Moodle should reflect the score->timestamp, not the time of the score post. + $grades = grade_get_grades($course->id, 'mod', 'lti', $modinstance->id, $user->id); + $studentgrade = array_shift($grades->items[0]->grades); + $this->assertEquals(strtotime($score->timestamp), $studentgrade->dategraded); + + // Read the results via the service. This should also return the correct dategraded time (timemodified in LTI terms). + $_SERVER['REQUEST_METHOD'] = \mod_lti\local\ltiservice\resource_base::HTTP_GET; + $_SERVER['PATH_INFO'] = "/$course->id/lineitems/{$gradeitems[1][0]->id}/lineitem/results?type_id=$typeid"; + $token = lti_new_access_token($typeid, ['https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly']); + $_SERVER['HTTP_Authorization'] = 'Bearer '.$token->token; + $_GET['type_id'] = (string)$typeid; + $result = new results($gbservice); + $response = new \mod_lti\local\ltiservice\response(); + $response->set_content_type('application/vnd.ims.lis.v2.resultcontainer+json'); + $result->execute($response); + $body = json_decode($response->get_body()); + $result = array_shift($body); + $this->assertEquals(strtotime($score->timestamp), strtotime($result->timestamp)); + + // Now, try to post a newer score using a timestamp that is greater than the one originally sent, but less than the time at + // which the score was last posted. This should be valid since the timestamp is greater than the original posted score. + $_SERVER['REQUEST_METHOD'] = \mod_lti\local\ltiservice\resource_base::HTTP_POST; + $_SERVER['PATH_INFO'] = "/$course->id/lineitems/{$gradeitems[1][0]->id}/lineitem/scores?type_id=$typeid"; + $token = lti_new_access_token($typeid, ['https://purl.imsglobal.org/spec/lti-ags/scope/score']); + $_SERVER['HTTP_Authorization'] = 'Bearer '.$token->token; + $_GET['type_id'] = (string)$typeid; + $score->scoreGiven = "14"; + $score->timestamp = "2024-08-08T18:54:36.736+00:00"; + $response = new \mod_lti\local\ltiservice\response(); + $response->set_content_type('application/vnd.ims.lis.v1.score+json'); + $response->set_request_data(json_encode($score)); + $score->execute($response); + $this->assertEquals(200, json_decode($response->get_code())); + + // Finally, post a score that's just been updated (i.e. score->timestamp = now). + $_SERVER['REQUEST_METHOD'] = \mod_lti\local\ltiservice\resource_base::HTTP_POST; + $_SERVER['PATH_INFO'] = "/$course->id/lineitems/{$gradeitems[1][0]->id}/lineitem/scores?type_id=$typeid"; + $token = lti_new_access_token($typeid, ['https://purl.imsglobal.org/spec/lti-ags/scope/score']); + $_SERVER['HTTP_Authorization'] = 'Bearer '.$token->token; + $_GET['type_id'] = (string)$typeid; + $score->scoreGiven = "15"; + $score->timestamp = date('c', time()); + $response = new \mod_lti\local\ltiservice\response(); + $response->set_content_type('application/vnd.ims.lis.v1.score+json'); + $response->set_request_data(json_encode($score)); + $score->execute($response); + $this->assertEquals(200, json_decode($response->get_code())); + } + /** * Inserts a graded lti instance, which should create a grade_item and gradebookservices record. * From 1bf62c2e468ea088fb7011e70bc35f05b3e69545 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Fri, 9 Aug 2024 13:55:43 +0800 Subject: [PATCH 2/2] MDL-75947 ltiservice_gradebookservices: score->timestamp as dategraded If this is not passed in to grade_update(), then it's not passed on to update_raw_grade(), resulting in time() being used instead, which isn't correct. The grade timemodified value should be set to the timestamp provided by the tool in the score post JSON, not the time of the score post itself. Co-authored-by: Jayce Birrell --- lib/grade/grade_grade.php | 6 ++++++ .../classes/local/service/gradebookservices.php | 1 + 2 files changed, 7 insertions(+) diff --git a/lib/grade/grade_grade.php b/lib/grade/grade_grade.php index 46adfbfc972..b053df86094 100644 --- a/lib/grade/grade_grade.php +++ b/lib/grade/grade_grade.php @@ -218,6 +218,12 @@ class grade_grade extends grade_object { */ public $label; + /** + * Date when this grade was last graded. + * @var int $dategraded + */ + public $dategraded = null; + /** * Returns array of grades for given grade_item+users * diff --git a/mod/lti/service/gradebookservices/classes/local/service/gradebookservices.php b/mod/lti/service/gradebookservices/classes/local/service/gradebookservices.php index 47383c42dc4..d020a198708 100644 --- a/mod/lti/service/gradebookservices/classes/local/service/gradebookservices.php +++ b/mod/lti/service/gradebookservices/classes/local/service/gradebookservices.php @@ -541,6 +541,7 @@ class gradebookservices extends service_base { $grade->feedbackformat = $feedbackformat; $grade->feedback = $feedback; $grade->rawgrade = $finalgrade; + $grade->dategraded = $timemodified; $status = grade_update($source, $gradeitem->courseid, $gradeitem->itemtype, $gradeitem->itemmodule, $gradeitem->iteminstance, $gradeitem->itemnumber, $grade);