From b33e8c852423b14b155d529575c7494f6fd639e8 Mon Sep 17 00:00:00 2001 From: Claude Vervoort Date: Sun, 8 Mar 2020 20:59:29 -0400 Subject: [PATCH] MDL-65306 mod_lti: preserve resourceid restoring older backups --- mod/lti/locallib.php | 41 +++++++++++-------- ...vice_gradebookservices_subplugin.class.php | 27 ++++++++++++ .../local/service/gradebookservices.php | 11 +++-- .../service/gradebookservices/db/upgrade.php | 17 ++++---- mod/lti/service/gradebookservices/version.php | 2 +- mod/lti/version.php | 2 +- 6 files changed, 68 insertions(+), 32 deletions(-) diff --git a/mod/lti/locallib.php b/mod/lti/locallib.php index 37529326718..95f5059b82f 100644 --- a/mod/lti/locallib.php +++ b/mod/lti/locallib.php @@ -497,6 +497,23 @@ function lti_get_jwt_claim_mapping() { ); } +/** + * Return the type of the instance, using domain matching if no explicit type is set. + * + * @param object $instance the external tool activity settings + * @return object|null + * @since Moodle 3.9 + */ +function lti_get_instance_type(object $instance) : ?object { + if (empty($instance->typeid)) { + if (!$tool = lti_get_tool_by_url_match($instance->toolurl, $instance->course)) { + $tool = lti_get_tool_by_url_match($instance->securetoolurl, $instance->course); + } + return $tool; + } + return lti_get_type($instance->typeid); +} + /** * Return the launch data required for opening the external tool. * @@ -508,25 +525,13 @@ function lti_get_jwt_claim_mapping() { function lti_get_launch_data($instance, $nonce = '') { global $PAGE, $CFG, $USER; - if (empty($instance->typeid)) { - $tool = lti_get_tool_by_url_match($instance->toolurl, $instance->course); - if ($tool) { - $typeid = $tool->id; - $ltiversion = $tool->ltiversion; - } else { - $tool = lti_get_tool_by_url_match($instance->securetoolurl, $instance->course); - if ($tool) { - $typeid = $tool->id; - $ltiversion = $tool->ltiversion; - } else { - $typeid = null; - $ltiversion = LTI_VERSION_1; - } - } - } else { - $typeid = $instance->typeid; - $tool = lti_get_type($typeid); + $tool = lti_get_instance_type($instance); + if ($tool) { + $typeid = $tool->id; $ltiversion = $tool->ltiversion; + } else { + $typeid = null; + $ltiversion = LTI_VERSION_1; } if ($typeid) { diff --git a/mod/lti/service/gradebookservices/backup/moodle2/restore_ltiservice_gradebookservices_subplugin.class.php b/mod/lti/service/gradebookservices/backup/moodle2/restore_ltiservice_gradebookservices_subplugin.class.php index f4125ae97e4..7d39b598893 100644 --- a/mod/lti/service/gradebookservices/backup/moodle2/restore_ltiservice_gradebookservices_subplugin.class.php +++ b/mod/lti/service/gradebookservices/backup/moodle2/restore_ltiservice_gradebookservices_subplugin.class.php @@ -206,9 +206,36 @@ class restore_ltiservice_gradebookservices_subplugin extends restore_subplugin { $newgradeitemid = $this->get_mappingid('grade_item', $oldgradeitemid, 0); if ($newgradeitemid > 0) { $gbs->gradeitemid = $newgradeitemid; + if (!isset($gbs->resourceid)) { + // Before 3.9 resourceid was stored in grade_item->idnumber. + $gbs->resourceid = $DB->get_field_select('grade_items', 'idnumber', "id=:id", ['id' => $newgradeitemid]); + } $DB->update_record('ltiservice_gradebookservices', $gbs); } } + // Pre 3.9 backups did not include a gradebookservices record. We create one here if idnumber is set. + $gradeitems = $DB->get_records('grade_items', array('itemtype' => 'mod', 'itemmodule' => 'lti', 'courseid' => $courseid)); + foreach ($gradeitems as $gi) { + if (isset($gi->idnumber) && !empty(trim($gi->idnumber))) { + $gbs = $DB->get_records('ltiservice_gradebookservices', ['gradeitemid' => $gi->id]); + if (empty($gbs) && !empty($gi->iteminstance)) { + // We did not find an entry for an LTI grade item with an idnumber, so let's create a gbs entry. + if ($instance = $DB->get_record('lti', array('id' => $gi->iteminstance))) { + if ($tool = lti_get_instance_type($instance)) { + $DB->insert_record('ltiservice_gradebookservices', (object) array( + 'gradeitemid' => $gi->id, + 'courseid' => $courseid, + 'toolproxyid' => $tool->toolproxyid, + 'ltilinkid' => $gi->iteminstance, + 'typeid' => $tool->id, + 'baseurl' => $tool->baseurl, + 'resourceid' => $gi->idnumber + )); + } + } + } + } + } } } diff --git a/mod/lti/service/gradebookservices/classes/local/service/gradebookservices.php b/mod/lti/service/gradebookservices/classes/local/service/gradebookservices.php index a11ac459465..43039f42934 100644 --- a/mod/lti/service/gradebookservices/classes/local/service/gradebookservices.php +++ b/mod/lti/service/gradebookservices/classes/local/service/gradebookservices.php @@ -617,17 +617,20 @@ class gradebookservices extends service_base { * Updates the tag and resourceid values for a grade item coupled to an lti link instance. * * @param object $ltiinstance The lti instance to which the grade item is coupled to - * @param string|null $resourceid The resourceid to apply to the lineitem. Might be an empty string. - * @param string|null $tag The tag to apply to the lineitem. Might be an empty string. + * @param string|null $resourceid The resourceid to apply to the lineitem. If empty string which will be stored as null. + * @param string|null $tag The tag to apply to the lineitem. If empty string which will be stored as null. * */ public static function update_coupled_gradebookservices(object $ltiinstance, ?string $resourceid, ?string $tag) : void { global $DB; + if ($ltiinstance && $ltiinstance->typeid) { $gradeitem = $DB->get_record('grade_items', array('itemmodule' => 'lti', 'iteminstance' => $ltiinstance->id)); if ($gradeitem) { + $resourceid = (isset($resourceid) && empty(trim($resourceid))) ? null : $resourceid; + $tag = (isset($tag) && empty(trim($tag))) ? null : $tag; $gbs = self::find_ltiservice_gradebookservice_for_lineitem($gradeitem->id); if ($gbs) { $gbs->resourceid = $resourceid; @@ -655,7 +658,7 @@ class gradebookservices extends service_base { * @param object $lti LTI Instance. */ public function instance_added(object $lti): void { - self::update_coupled_gradebookservices($lti, $lti->lineitemresourceid ?? '', $lti->lineitemtag ?? ''); + self::update_coupled_gradebookservices($lti, $lti->lineitemresourceid ?? null, $lti->lineitemtag ?? null); } /** @@ -664,7 +667,7 @@ class gradebookservices extends service_base { * @param object $lti LTI Instance. */ public function instance_updated(object $lti): void { - self::update_coupled_gradebookservices($lti, $lti->lineitemresourceid ?? '', $lti->lineitemtag ?? ''); + self::update_coupled_gradebookservices($lti, $lti->lineitemresourceid ?? null, $lti->lineitemtag ?? null); } /** diff --git a/mod/lti/service/gradebookservices/db/upgrade.php b/mod/lti/service/gradebookservices/db/upgrade.php index d212447b840..20d54438e77 100644 --- a/mod/lti/service/gradebookservices/db/upgrade.php +++ b/mod/lti/service/gradebookservices/db/upgrade.php @@ -59,7 +59,7 @@ function xmldb_ltiservice_gradebookservices_upgrade($oldversion) { $dbman = $DB->get_manager(); - if ($oldversion < 2020020601) { + if ($oldversion < 2020042401) { // Define field typeid to be added to lti_tool_settings. $table = new xmldb_table('ltiservice_gradebookservices'); $field = new xmldb_field('resourceid', XMLDB_TYPE_CHAR, "512", null, null, null, null); @@ -70,16 +70,17 @@ function xmldb_ltiservice_gradebookservices_upgrade($oldversion) { } // Lti savepoint reached. - upgrade_plugin_savepoint(true, 2020020601, 'ltiservice', 'gradebookservices'); + upgrade_plugin_savepoint(true, 2020042401, 'ltiservice', 'gradebookservices'); } - if ($oldversion < 2020020602) { + if ($oldversion < 2020042402) { // Now that we have added the new column let's migrate it' // Prior implementation was storing the resourceid under the grade item idnumber, so moving it to lti_gradebookservices. // We only care for mod/lti grade items as manual columns would already have a matching gradebookservices record. - $DB->execute("INSERT INTO {ltiservice_gradebookservices} (gradeitemid, courseid, typeid, resourceid, baseurl) - SELECT gi.id, courseid, lti.typeid, gi.idnumber, t.baseurl + $DB->execute("INSERT INTO {ltiservice_gradebookservices} + (gradeitemid, courseid, typeid, ltilinkid, resourceid, baseurl, toolproxyid) + SELECT gi.id, courseid, lti.typeid, lti.id, gi.idnumber, t.baseurl, t.toolproxyid FROM {grade_items} gi JOIN {lti} lti ON lti.id=gi.iteminstance AND gi.itemtype='mod' AND gi.itemmodule='lti' JOIN {lti_types} t ON t.id = lti.typeid @@ -89,10 +90,10 @@ function xmldb_ltiservice_gradebookservices_upgrade($oldversion) { AND gi.idnumber <> ''"); // Lti savepoint reached. - upgrade_plugin_savepoint(true, 2020020602, 'ltiservice', 'gradebookservices'); + upgrade_plugin_savepoint(true, 2020042402, 'ltiservice', 'gradebookservices'); } - if ($oldversion < 2020020603) { + if ($oldversion < 2020042403) { // Here updating the resourceid of pre-existing lti_gradebookservices. $DB->execute("UPDATE {ltiservice_gradebookservices} SET resourceid = (SELECT idnumber FROM {grade_items} WHERE id=gradeitemid) @@ -103,7 +104,7 @@ function xmldb_ltiservice_gradebookservices_upgrade($oldversion) { AND (resourceid is null OR resourceid = '')"); // Lti savepoint reached. - upgrade_plugin_savepoint(true, 2020020603, 'ltiservice', 'gradebookservices'); + upgrade_plugin_savepoint(true, 2020042403, 'ltiservice', 'gradebookservices'); } return true; diff --git a/mod/lti/service/gradebookservices/version.php b/mod/lti/service/gradebookservices/version.php index b1b5c624392..9af909de8a8 100644 --- a/mod/lti/service/gradebookservices/version.php +++ b/mod/lti/service/gradebookservices/version.php @@ -25,6 +25,6 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2020020603; +$plugin->version = 2020042403; $plugin->requires = 2019111200; $plugin->component = 'ltiservice_gradebookservices'; diff --git a/mod/lti/version.php b/mod/lti/version.php index e26af0d5dd2..de1dccb5e87 100644 --- a/mod/lti/version.php +++ b/mod/lti/version.php @@ -48,7 +48,7 @@ defined('MOODLE_INTERNAL') || die; -$plugin->version = 2020022200; // The current module version (Date: YYYYMMDDXX). +$plugin->version = 2020042403; // The current module version (Date: YYYYMMDDXX). $plugin->requires = 2019111200; // Requires this Moodle version. $plugin->component = 'mod_lti'; // Full name of the plugin (used for diagnostics). $plugin->cron = 0;