Merge remote-tracking branch 'origin/main' into PM-47393-embedded-res…

…ourcelink

.github/workflows/moodle-ci.yml

@@ -144,6 +144,9 @@ jobs:
       - name: PHPUnit tests
         if: ${{ always() }}
         run: moodle-plugin-ci phpunit --fail-on-warning
+        # Somehow the PHPunit job of the moodle-plugin-ci currently fails with an external error,
+        # so we allow this to fail for now until we know how to fix it.
+        continue-on-error: ${{matrix.moodle-branch == 'main'}}
 
   # Automatically releases tags when the CI tests passed.
   release:

.php-cs-fixer.php

@@ -5,7 +5,8 @@
     ->ignoreUnreadableDirs()
     ->in(__DIR__)
     ->exclude('development')
-    ->exclude('node_modules');
+    ->exclude('node_modules')
+    ->exclude('vendor');
 
 $config = new PhpCsFixer\Config();
 

CHANGES.md

@@ -1,3 +1,11 @@
+### v1.3.3 (Build - 2025012401)
+
+* Bugfixes: grade syncing issues ("LMS configuration issue")
+
+### v1.3.2 (Build - 2025012201)
+
+* Bugfixes
+
 ### v1.3.0 (Build - 2024101601)
 
 * Added Grading

classes/grading/grading_service.php

@@ -52,7 +52,7 @@ class grading_service {
     public static function get_line_item(int $courseid, int $cmid, string $resourcelinkid): line_item {
         $module = get_coursemodule_from_id('kialo', $cmid, $courseid, false, MUST_EXIST);
 
-        $gradeitem = grade_item::fetch(['iteminstance' => $module->instance, 'itemtype' => 'mod']);
+        $gradeitem = grade_item::fetch(['iteminstance' => $module->instance, 'itemtype' => 'mod', 'itemmodule' => 'kialo']);
         if (!$gradeitem) {
             $maxscore = 100;
         } else {

classes/lti_flow.php

@@ -179,7 +179,7 @@ public static function parse_resource_link_id(string $resourcelinkid): int {
      *
      * @param int $courseid
      * @param int $coursemoduleid
-     * @param string $deploymentid
+     * @param string $deploymentid Usually KIALO_LTI_DEPLOYMENT_ID
      * @param string $moodleuserid
      * @param string $discussionurl
      * @param string|null $groupid
@@ -242,7 +242,7 @@ public static function init_resource_link(
      */
     public static function validate_deep_linking_response(
         ServerRequestInterface $request,
-        string $deploymentid
+        string $deploymentid = KIALO_LTI_DEPLOYMENT_ID
     ): deep_linking_result {
         $kialoconfig = kialo_config::get_instance();
         $registration = $kialoconfig->create_registration($deploymentid);
@@ -313,7 +313,7 @@ private static function create_platform_jwt_token(
      * @throws LtiExceptionInterface
      * @throws \dml_exception
      */
-    public static function init_deep_link(int $courseid, string $moodleuserid, string $deploymentid) {
+    public static function init_deep_link(int $courseid, string $moodleuserid, string $deploymentid = KIALO_LTI_DEPLOYMENT_ID) {
         $kialoconfig = kialo_config::get_instance();
 
         $deeplinkingreturnurl = (new \moodle_url('/mod/kialo/lti_select.php'))->out(false);

constants.php

@@ -47,3 +47,11 @@
  * Value used to indicate that the Kialo app should be displayed in a new window.
  */
 const MOD_KIALO_DISPLAY_IN_NEW_WINDOW = 'new-window';
+
+/**
+ * In LTI the deployment ID identifies an LTI tool definition or installation. Moodle sends
+ * the same ID for all activities that are based on the same LTI external tool definition.
+ * We always send 1 for the plugin because there can only be one Kialo plugin installed which
+ * always has the same configuration.
+ */
+const KIALO_LTI_DEPLOYMENT_ID = "1";

lang/en/kialo.php

@@ -36,7 +36,6 @@
 $string['acceptterms_desc'] = 'To enable the Kialo plugin you have to accept Kialo Edu’s <a href="{$a->terms}" target="_blank">Terms of Service</a> on behalf of all users of this Moodle instance. Here is a link to our <a href="{$a->privacy}" target="_blank">Privacy Policy</a> and to our <a href="{$a->data_security}" target="_blank">Data Security and Privacy Plan.</a>';
 $string['cachedef_nonces'] = "Used to store temporary nonces to secure LTI requests.";
 $string['close_prompt'] = 'You can close this window now.';
-$string['deploymentid'] = "Discussion Linked";
 $string['discussion_title'] = 'Discussion';
 $string['display'] = 'Display';
 $string['display_embed'] = 'Display in embed';
@@ -47,7 +46,6 @@
 $string['errors:invalidrequest'] = "Invalid request";
 $string['errors:ltiauth'] = "Authentication failed due to an unexpected error. Please try again.";
 $string['errors:missingcourseid'] = "Missing course id";
-$string['errors:missingdeploymentid'] = "Missing deployment id";
 $string['errors:missingidtoken'] = "Missing id token";
 $string['errors:missingsessiondata'] = "Missing session data";
 $string['errors:noguestaccess'] = "Guests cannot access this activity. Please log in.";

lti_select.php

@@ -39,20 +39,22 @@
 
 $courseid = optional_param('courseid', 0, PARAM_INT);
 $idtoken = optional_param("JWT", "", PARAM_TEXT);
-$deploymentid = optional_param("deploymentid", "", PARAM_TEXT);
 
-if ($courseid && $deploymentid) {
+if ($courseid) {
     // Called by our activity creation form in Moodle to start the deeplinking flow.
+    $context = context_course::instance($courseid);
+    $PAGE->set_context($context);
     require_login($courseid, false);
-    require_capability('mod/kialo:addinstance', context_course::instance($courseid));
-    $_SESSION["kialo_deployment_id"] = $deploymentid;
+    require_capability('mod/kialo:addinstance', $context);
+
+    $PAGE->set_url('/mod/kialo/lti_select.php', ['courseid' => $courseid]);
+    $PAGE->set_title(get_string("redirect_title", "mod_kialo"));
 
     // This will throw an exception and result in a generic error page, if the deep linking response is invalid.
     try {
         $deeplinkmsg = lti_flow::init_deep_link(
             $courseid,
             $USER->id,
-            $deploymentid,
         );
     } catch (Throwable $e) {
         // Show Moodle's default error page including some debug info.
@@ -65,13 +67,14 @@
         get_string("redirect_loading", "mod_kialo"),
         $deeplinkmsg->toHtmlRedirectForm()
     ));
-} else if ($idtoken && isset($_SESSION["kialo_deployment_id"])) {
+} else if ($idtoken) {
     // Received LtiDeepLinkingResponse from Kialo.
-    $deploymentid = $_SESSION["kialo_deployment_id"];
-    unset($_SESSION["kialo_deployment_id"]);
+    $PAGE->set_context(context_system::instance());
+    $PAGE->set_url('/mod/kialo/lti_select.php', ['idtoken' => $idtoken]);
+    $PAGE->set_title(get_string('close_prompt', 'mod_kialo'));
 
     try {
-        $link = lti_flow::validate_deep_linking_response(ServerRequest::fromGlobals(), $deploymentid);
+        $link = lti_flow::validate_deep_linking_response(ServerRequest::fromGlobals());
     } catch (Throwable $e) {
         // Show Moodle's default error page including some debug info.
         throw new \moodle_exception('errors:deeplinking', 'kialo', '', null, $e->getMessage());
@@ -80,7 +83,6 @@
     // Inform the activity form about the successful selection. When acknowledged by the form, close the window.
     $message = json_encode([
             "type" => "kialo_discussion_selected",
-            "deploymentid" => $link->deploymentid,
             "discussionurl" => $link->discussionurl,
             "discussiontitle" => $link->discussiontitle,
     ]);
@@ -92,16 +94,16 @@
     // The user should basically not see this, or just very briefly.
     echo "<br><br><br><br><center>" . get_string('close_prompt', 'mod_kialo') . "</center>";
 } else {
+    $PAGE->set_context(context_system::instance());
+    $PAGE->set_url('/mod/kialo/lti_select.php', ['idtoken' => $idtoken, 'courseid' => $courseid]);
+
     $error = "errors:invalidrequest";
-    if (empty($_SESSION['kialo_deployment_id'])) {
-        $error = "errors:missingsessiondata";
-    } else if (empty($deploymentid)) {
-        $error = "errors:missingdeploymentid";
-    } else if (empty($courseid)) {
+    if (empty($courseid)) {
         $error = "errors:missingcourseid";
     } else if (empty($idtoken)) {
         $error = "errors:missingidtoken";
     }
+    $PAGE->set_title(get_string($error, 'mod_kialo'));
 
     // Should not happen (but could if someone intentionally calls this page with wrong params). Display moodle error page.
     throw new \moodle_exception('errors:deeplinking', 'kialo', "", null, get_string($error, 'mod_kialo'));

mod_form.php

@@ -41,17 +41,6 @@
  */
 class mod_kialo_mod_form extends moodleform_mod {
 
-    /**
-     * In LTI the deployment ID identifies an LTI tool definition or installation. Moodle sends
-     * the same ID for all activities that are based on the same LTI external tool definition.
-     * We always send 1 for the plugin because there can only be one Kialo plugin installed which
-     * always has the same configuration.
-     * @return string
-     */
-    private function get_deployment_id(): string {
-        return "1";
-    }
-
     /**
      * Defines forms elements
      */
@@ -91,14 +80,8 @@ public function definition() {
         $mform->addElement("hidden", "discussion_url", "");
         $mform->setType("discussion_url", PARAM_RAW);
 
-        // Deployment ID, filled when selecting the discussion.
-        $deploymentid = $this->get_deployment_id();
-        $mform->addElement("hidden", "deployment_id", $deploymentid);
-        $mform->setType("deployment_id", PARAM_RAW);
-
         // Deep Linking Button, allowing the user to select a discussion on Kialo.
         $deeplinkurl = (new moodle_url('/mod/kialo/lti_select.php', [
-            "deploymentid" => $deploymentid,
             "courseid" => $COURSE->id,
         ]))->out(false);
         $mform->addElement("button", "kialo_select_discussion", get_string("select_discussion", "mod_kialo"));
@@ -116,7 +99,6 @@ public function definition() {
 
                   // Fill in the deep-linked details.
                   document.querySelector('input[name=discussion_url]').value = event.data.discussionurl;
-                  document.querySelector('input[name=deployment_id]').value = event.data.deploymentid;
                   document.querySelector('input[name=discussion_title]').value = event.data.discussiontitle;
 
                   // Prefill activity name based on discussion title if user hasn't entered one yet.

tests/grading/grading_service_test.php

@@ -124,6 +124,41 @@ public function test_get_line_item(): void {
         $this->assertEquals($resourcelinkid, $lineitem->resourceLinkId);
     }
 
+    /**
+     * Tests getting the line item for a course module when we have same courseid and iteminstance for moodle's build-in
+     * lti client and the plugin.
+     * This is used by Kialo to get the max. grade configured in the LMS,
+     * as well as the endpoint to send grades to.
+     *
+     * @return void
+     * @throws \OAT\Library\Lti1p3Core\Exception\LtiException
+     * @throws \coding_exception
+     * @throws \moodle_exception
+     * @covers \mod_kialo\grading\grading_service::get_line_item
+     */
+    public function test_get_line_item_conflict_between_moodle_lti_client_and_plugin(): void {
+        $maxgrade = 123;
+        $course = $this->getDataGenerator()->create_course();
+        $kialo = $this->getDataGenerator()->create_module('kialo', ['course' => $course->id, 'grade' => $maxgrade]);
+        // Create a grade item for the moodle's build-in lti client with same iteminstance and courseid.
+        $this->getDataGenerator()->create_grade_item(['iteminstance' => $kialo->id, 'courseid' => $course->id,
+            'itemmodule' => 'lti', 'itemtype' => 'mod']);
+        $coursemodule = get_coursemodule_from_instance("kialo", $kialo->id);
+
+        $courseid = $course->id;
+        $coursemoduleid = $coursemodule->id;
+        $resourcelinkid = lti_flow::resource_link_id($coursemoduleid);
+
+        $endpoint = "/mod/kialo/lti_lineitem.php?course_id={$courseid}&cmid={$coursemoduleid}&resource_link_id={$resourcelinkid}";
+        $_SERVER['REQUEST_URI'] = $endpoint;
+
+        $lineitem = grading_service::get_line_item($courseid, $coursemoduleid, $resourcelinkid);
+        $this->assertEquals("https://www.example.com/moodle" . $endpoint, $lineitem->id);
+        $this->assertEquals($coursemodule->name, $lineitem->label);
+        $this->assertEquals($maxgrade, $lineitem->scoreMaximum);
+        $this->assertEquals($resourcelinkid, $lineitem->resourceLinkId);
+    }
+
     /**
      * Tests getting the line item for a course module. This is used by Kialo to get the max. grade configured in the LMS,
      * as well as the endpoint to send grades to.

version.php

@@ -28,7 +28,7 @@
 $plugin->component = 'mod_kialo';
 
 // See https://moodledev.io/docs/apis/commonfiles/version.php.
-$plugin->version = 2025011501;  // Must be incremented for each new release!
+$plugin->version = 2025012402;  // Must be incremented for each new release!
 $plugin->release = '1.4.0';  // Semantic version.
 
 // Officially we require PHP 7.4. The first Moodle version that requires this as a minimum is Moodle 4.1.

view.php

@@ -68,7 +68,7 @@
     $message = lti_flow::init_resource_link(
         $course->id,
         $cm->id,
-        $moduleinstance->deployment_id,
+        KIALO_LTI_DEPLOYMENT_ID,
         $USER->id,
         $moduleinstance->discussion_url,
         $groupinfo->groupid,