diff --git a/enrol/lti/classes/local/ltiadvantage/table/published_resources_table.php b/enrol/lti/classes/local/ltiadvantage/table/published_resources_table.php index d375d9163c5..8367b634da2 100644 --- a/enrol/lti/classes/local/ltiadvantage/table/published_resources_table.php +++ b/enrol/lti/classes/local/ltiadvantage/table/published_resources_table.php @@ -92,7 +92,8 @@ class published_resources_table extends \table_sql { * @return string */ public function col_name($tool) { - $name = helper::get_name($tool); + $toolcontext = \context::instance_by_id($tool->contextid, IGNORE_MISSING); + $name = $toolcontext ? helper::get_name($tool) : $this->get_deleted_activity_name_html($tool); return $this->get_display_text($tool, $name); } @@ -215,9 +216,39 @@ class published_resources_table extends \table_sql { */ protected function get_display_text($tool, $text) { if ($tool->status != ENROL_INSTANCE_ENABLED) { - return \html_writer::tag('span', $text, array('class' => 'dimmed_text')); + return \html_writer::tag('div', $text, array('class' => 'dimmed_text')); } return $text; } + + /** + * Get a warning icon, with tooltip, describing enrolment instances sharing activities which have been deleted. + * + * @param \stdClass $tool the tool instance record. + * @return string the HTML for the name column. + */ + protected function get_deleted_activity_name_html(\stdClass $tool): string { + global $OUTPUT; + $icon = \html_writer::tag( + 'a', + $OUTPUT->pix_icon('enrolinstancewarning', get_string('deletedactivityalt' , 'enrol_lti'), 'enrol_lti'), [ + "class" => "btn btn-link p-0", + "role" => "button", + "data-container" => "body", + "data-toggle" => "popover", + "data-placement" => right_to_left() ? "left" : "right", + "data-content" => get_string('deletedactivitydescription', 'enrol_lti'), + "data-html" => "true", + "tabindex" => "0", + "data-trigger" => "focus" + ] + ); + $name = \html_writer::span($icon . get_string('deletedactivity', 'enrol_lti')); + if ($tool->name) { + $name .= \html_writer::empty_tag('br') . \html_writer::empty_tag('br') . $tool->name; + } + + return $name; + } } diff --git a/enrol/lti/classes/manage_table.php b/enrol/lti/classes/manage_table.php index bd9ccc208da..262010103ba 100644 --- a/enrol/lti/classes/manage_table.php +++ b/enrol/lti/classes/manage_table.php @@ -101,7 +101,8 @@ class manage_table extends \table_sql { * @return string */ public function col_name($tool) { - $name = helper::get_name($tool); + $toolcontext = \context::instance_by_id($tool->contextid, IGNORE_MISSING); + $name = $toolcontext ? helper::get_name($tool) : $this->get_deleted_activity_name_html($tool); return $this->get_display_text($tool, $name); } @@ -247,9 +248,39 @@ class manage_table extends \table_sql { */ protected function get_display_text($tool, $text) { if ($tool->status != ENROL_INSTANCE_ENABLED) { - return \html_writer::tag('span', $text, array('class' => 'dimmed_text')); + return \html_writer::tag('div', $text, array('class' => 'dimmed_text')); } return $text; } + + /** + * Get a warning icon, with tooltip, describing enrolment instances sharing activities which have been deleted. + * + * @param \stdClass $tool the tool instance record. + * @return string the HTML for the name column. + */ + protected function get_deleted_activity_name_html(\stdClass $tool): string { + global $OUTPUT; + $icon = \html_writer::tag( + 'a', + $OUTPUT->pix_icon('enrolinstancewarning', get_string('deletedactivityalt' , 'enrol_lti'), 'enrol_lti'), [ + "class" => "btn btn-link p-0", + "role" => "button", + "data-container" => "body", + "data-toggle" => "popover", + "data-placement" => right_to_left() ? "left" : "right", + "data-content" => get_string('deletedactivitydescription', 'enrol_lti'), + "data-html" => "true", + "tabindex" => "0", + "data-trigger" => "focus" + ] + ); + $name = \html_writer::span($icon . get_string('deletedactivity', 'enrol_lti')); + if ($tool->name) { + $name .= \html_writer::empty_tag('br') . \html_writer::empty_tag('br') . $tool->name; + } + + return $name; + } } diff --git a/enrol/lti/db/upgrade.php b/enrol/lti/db/upgrade.php index 92da6cab9b9..5cf89474132 100644 --- a/enrol/lti/db/upgrade.php +++ b/enrol/lti/db/upgrade.php @@ -450,5 +450,17 @@ function xmldb_enrol_lti_upgrade($oldversion) { // Automatically generated Moodle v4.0.0 release upgrade line. // Put any upgrade step following this. + if ($oldversion < 2022061500) { + // Disable all orphaned enrolment method instances. + $sql = "id IN (SELECT t.enrolid + FROM {enrol_lti_tools} t + LEFT JOIN {context} c ON (t.contextid = c.id) + WHERE c.id IS NULL)"; + $DB->set_field_select('enrol', 'status', 1, $sql); + + // Lti savepoint reached. + upgrade_plugin_savepoint(true, 2022061500, 'enrol', 'lti'); + } + return true; } diff --git a/enrol/lti/lang/en/enrol_lti.php b/enrol/lti/lang/en/enrol_lti.php index 80f89612bcf..5808a0334d5 100644 --- a/enrol/lti/lang/en/enrol_lti.php +++ b/enrol/lti/lang/en/enrol_lti.php @@ -36,6 +36,9 @@ $string['copytoclipboard'] = 'Copy to clipboard'; $string['couldnotestablishproxy'] = 'Could not establish proxy with consumer.'; $string['customproperties'] = 'Custom properties'; $string['deeplinkingurl'] = 'Deep linking URL'; +$string['deletedactivity'] = 'Deleted activity'; +$string['deletedactivityalt'] = 'The activity shared by this instance has been deleted'; +$string['deletedactivitydescription'] = 'The activity shared by this instance has been deleted. You can select another activity to share by editing the instance or, if no longer required, you can simply delete the instance. Deleting the instance will remove any associated user enrolments.'; $string['deploymentadd'] = 'Add a deployment'; $string['deploymentaddnotice'] = 'Deployment added'; $string['deploymentdelete'] = 'Delete deployment'; diff --git a/enrol/lti/lib.php b/enrol/lti/lib.php index 7c8f78e3251..f01522cb9e1 100644 --- a/enrol/lti/lib.php +++ b/enrol/lti/lib.php @@ -475,6 +475,22 @@ function enrol_lti_extend_navigation_course($navigation, $course, $context) { function enrol_lti_get_fontawesome_icon_map() { return [ 'enrol_lti:managedeployments' => 'fa-sitemap', - 'enrol_lti:platformdetails' => 'fa-pencil-square-o' + 'enrol_lti:platformdetails' => 'fa-pencil-square-o', + 'enrol_lti:enrolinstancewarning' => 'fa-exclamation-circle text-danger', ]; } + +/** + * Pre-delete course module hook which disables any methods referring to the deleted module, preventing launches and allowing remap. + * + * @param stdClass $cm The deleted course module record. + */ +function enrol_lti_pre_course_module_delete(stdClass $cm) { + global $DB; + $sql = "id IN (SELECT t.enrolid + FROM {enrol_lti_tools} t + JOIN {context} c ON (t.contextid = c.id) + WHERE c.contextlevel = :contextlevel + AND c.instanceid = :cmid)"; + $DB->set_field_select('enrol', 'status', ENROL_INSTANCE_DISABLED, $sql, ['contextlevel' => CONTEXT_MODULE, 'cmid' => $cm->id]); +} diff --git a/enrol/lti/pix/enrolinstancewarning.png b/enrol/lti/pix/enrolinstancewarning.png new file mode 100644 index 00000000000..edb89e473ea Binary files /dev/null and b/enrol/lti/pix/enrolinstancewarning.png differ diff --git a/enrol/lti/pix/enrolinstancewarning.svg b/enrol/lti/pix/enrolinstancewarning.svg new file mode 100644 index 00000000000..73a13a90cac --- /dev/null +++ b/enrol/lti/pix/enrolinstancewarning.svg @@ -0,0 +1,2 @@ + + \ No newline at end of file diff --git a/enrol/lti/tests/lib_test.php b/enrol/lti/tests/lib_test.php index d11e0f82699..9698d5e818b 100644 --- a/enrol/lti/tests/lib_test.php +++ b/enrol/lti/tests/lib_test.php @@ -216,4 +216,36 @@ class lib_test extends \lti_advantage_testcase { // LTI enrolment has 1 enrol actions for active users -- unenrol. $this->assertCount(1, $actions); } + + /** + * Test the behaviour of an enrolment method when the activity to which it provides access is deleted. + * + * @covers \enrol_lti_pre_course_module_delete + */ + public function test_course_module_deletion() { + // Create two modules and publish them. + $course = $this->getDataGenerator()->create_course(); + $mod = $this->getDataGenerator()->create_module('assign', ['course' => $course->id]); + $mod2 = $this->getDataGenerator()->create_module('assign', ['course' => $course->id]); + $tooldata = [ + 'cmid' => $mod->cmid, + 'courseid' => $course->id, + ]; + $tool = $this->getDataGenerator()->create_lti_tool((object)$tooldata); + $tooldata['cmid'] = $mod2->cmid; + $tool2 = $this->getDataGenerator()->create_lti_tool((object)$tooldata); + + // Verify the instances are both enabled. + $modinstance = helper::get_lti_tool($tool->id); + $mod2instance = helper::get_lti_tool($tool2->id); + $this->assertEquals(ENROL_INSTANCE_ENABLED, $modinstance->status); + $this->assertEquals(ENROL_INSTANCE_ENABLED, $mod2instance->status); + + // Delete a module and verify the associated instance is disabled. + course_delete_module($mod->cmid); + $modinstance = helper::get_lti_tool($tool->id); + $mod2instance = helper::get_lti_tool($tool2->id); + $this->assertEquals(ENROL_INSTANCE_DISABLED, $modinstance->status); + $this->assertEquals(ENROL_INSTANCE_ENABLED, $mod2instance->status); + } } diff --git a/enrol/lti/tests/local/ltiadvantage/task/sync_grades_test.php b/enrol/lti/tests/local/ltiadvantage/task/sync_grades_test.php index e8680a5f238..88e01c9dfd3 100644 --- a/enrol/lti/tests/local/ltiadvantage/task/sync_grades_test.php +++ b/enrol/lti/tests/local/ltiadvantage/task/sync_grades_test.php @@ -412,6 +412,35 @@ class sync_grades_test extends \lti_advantage_testcase { $task->execute(); } + /** + * Test syncing grades when the enrolment instance is disabled. + * + * @covers ::execute + */ + public function test_sync_grades_disabled_instance() { + $this->resetAfterTest(); + global $DB; + + [$course, $resource, $resource2, $resource3] = $this->create_test_environment(); + + // Disable resource 1. + $enrol = (object) ['id' => $resource->enrolid, 'status' => ENROL_INSTANCE_DISABLED]; + $DB->update_record('enrol', $enrol); + + // Delete the activity being shared by resource 2, leaving resource 2 disabled as a result. + $modcontext = \context::instance_by_id($resource2->contextid); + course_delete_module($modcontext->instanceid); + + // Only the enabled resource 3 should sync grades. + $task = $this->get_task_with_mocked_grade_service(); + $this->expectOutputRegex( + "/^Starting - LTI Advantage grade sync for shared resource '$resource3->id' in course '$course->id'.\n". + "Completed - Synced grades for tool '$resource3->id' in the course '$course->id'. Processed 0 users; ". + "sent 0 grades.\n$/" + ); + $task->execute(); + } + /** * Test the grade sync when the context has been deleted in between launch and when the grade sync task is run. * @@ -419,6 +448,8 @@ class sync_grades_test extends \lti_advantage_testcase { */ public function test_sync_grades_deleted_context() { $this->resetAfterTest(); + global $DB; + [$course, $resource] = $this->create_test_environment(); $launchservice = $this->get_tool_launch_service(); @@ -427,10 +458,11 @@ class sync_grades_test extends \lti_advantage_testcase { $instructoruser = $this->getDataGenerator()->create_user(); [$userid] = $launchservice->user_launches_tool($instructoruser, $teachermocklaunch); - global $CFG; - require_once($CFG->dirroot . '/course/lib.php'); + // Delete the activity, then enable the enrolment method (it is disabled during activity deletion). $modcontext = \context::instance_by_id($resource->contextid); course_delete_module($modcontext->instanceid); + $enrol = (object) ['id' => $resource->enrolid, 'status' => ENROL_INSTANCE_ENABLED]; + $DB->update_record('enrol', $enrol); $task = $this->get_task_with_mocked_grade_service(); $this->expectOutputRegex( diff --git a/enrol/lti/tests/local/ltiadvantage/task/sync_members_test.php b/enrol/lti/tests/local/ltiadvantage/task/sync_members_test.php index 7a2c3d3044c..ec265de1b91 100644 --- a/enrol/lti/tests/local/ltiadvantage/task/sync_members_test.php +++ b/enrol/lti/tests/local/ltiadvantage/task/sync_members_test.php @@ -609,6 +609,38 @@ class sync_members_test extends \lti_advantage_testcase { $this->assertCount(1, $userrepo->find_by_resource($resource->id)); } + /** + * Test syncing members when the enrolment instance is disabled. + * + * @covers ::execute + */ + public function test_sync_members_disabled_instance() { + $this->resetAfterTest(); + global $DB; + + [$course, $resource, $resource2, $resource3] = $this->create_test_environment(); + $userrepo = new user_repository(); + + // Disable resource 1. + $enrol = (object) ['id' => $resource->enrolid, 'status' => ENROL_INSTANCE_DISABLED]; + $DB->update_record('enrol', $enrol); + + // Delete the activity being shared by resource2, leaving resource 2 disabled as a result. + $modcontext = \context::instance_by_id($resource2->contextid); + course_delete_module($modcontext->instanceid); + + // Only the enabled resource 3 should sync members. + $task = $this->get_mock_task_with_users($this->get_mock_members_with_ids(range(1, 1))); + $task->execute(); + + $this->expectOutputRegex( + "/^Starting - Member sync for published resource '$resource3->id' for course '$course->id'.\n". + "Completed - Synced members for tool '$resource3->id' in the course '$course->id'. Processed 0 users; ". + "enrolled 0 members; unenrolled 0 members.\n$/" + ); + $this->assertCount(0, $userrepo->find_by_resource($resource->id)); + } + /** * Test syncing members for a membersync-enabled resource when the launch omits the NRPS service endpoints. * diff --git a/enrol/lti/version.php b/enrol/lti/version.php index 87b900db4a1..5aa032eedb6 100644 --- a/enrol/lti/version.php +++ b/enrol/lti/version.php @@ -24,7 +24,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2022041900; // The current plugin version (Date: YYYYMMDDXX). +$plugin->version = 2022061500; // The current plugin version (Date: YYYYMMDDXX). $plugin->requires = 2022041200; // Requires this Moodle version. $plugin->component = 'enrol_lti'; // Full name of the plugin (used for diagnostics). $plugin->dependencies = [