MDL-78597 mod_lti: deprecate lti_get_lti_types_by_course()

This method includes broken legacy behaviour in which the inclusion of course and site tools could be controlled independently, based on the capabilities 'mod/lti:addmanualinstance' (to include course tools) and 'mod/lti:addpreconfiguredinstance' (to include site tools). This behaviour is deprecated in 4.3, so this method is also deprecated. See the replacement method types_helper::get_lti_types_by_course.
2023-08-29 13:14:15 +08:00
parent 7ca26f1460
commit d8e2067b66
3 changed files with 136 additions and 8 deletions
@@ -2310,12 +2310,16 @@ function lti_filter_tool_types(array $tools, $state) {
 /**
 * Returns all lti types visible in this course
 *
+ * @deprecated since Moodle 4.3
 * @param int $courseid The id of the course to retieve types for
 * @param array $coursevisible options for 'coursevisible' field,
 *        default [LTI_COURSEVISIBLE_PRECONFIGURED, LTI_COURSEVISIBLE_ACTIVITYCHOOSER]
 * @return stdClass[] All the lti types visible in the given course
 */
 function lti_get_lti_types_by_course($courseid, $coursevisible = null) {
+    debugging(__FUNCTION__ . '() is deprecated. Please use \mod_lti\local\types_helper::get_lti_types_by_course() instead.',
+        DEBUG_DEVELOPER);
+
    global $DB, $SITE;

    if ($coursevisible === null) {
@@ -2359,15 +2363,15 @@ function lti_get_lti_types_by_course($courseid, $coursevisible = null) {
 * @return array Array of lti types
 */
 function lti_get_types_for_add_instance() {
-    global $COURSE;
-    $admintypes = lti_get_lti_types_by_course($COURSE->id);
+    global $COURSE, $USER;
+    $preconfiguredtypes = \mod_lti\local\types_helper::get_lti_types_by_course($COURSE->id, $USER->id);

-    $types = array();
+    $types = [];
    if (has_capability('mod/lti:addmanualinstance', context_course::instance($COURSE->id))) {
        $types[0] = (object)array('name' => get_string('automatic', 'lti'), 'course' => 0, 'toolproxyid' => null);
    }

-    foreach ($admintypes as $type) {
+    foreach ($preconfiguredtypes as $type) {
        $types[$type->id] = $type;
    }

@@ -2382,11 +2386,12 @@ function lti_get_types_for_add_instance() {
 * @return array Array of lti types. Each element is object with properties: name, title, icon, help, helplink, link
 */
 function lti_get_configured_types($courseid, $sectionreturn = 0) {
-    global $OUTPUT;
-    $types = array();
-    $admintypes = lti_get_lti_types_by_course($courseid, [LTI_COURSEVISIBLE_ACTIVITYCHOOSER]);
+    global $OUTPUT, $USER;
+    $types = [];
+    $preconfiguredtypes = \mod_lti\local\types_helper::get_lti_types_by_course($courseid, $USER->id,
+        [LTI_COURSEVISIBLE_ACTIVITYCHOOSER]);

-    foreach ($admintypes as $ltitype) {
+    foreach ($preconfiguredtypes as $ltitype) {
        $type           = new stdClass();
        $type->id       = $ltitype->id;
        $type->modclass = MOD_CLASS_ACTIVITY;
@@ -2248,4 +2248,126 @@ MwIDAQAB

        return ['proxies' => $proxies, 'types' => $types];
    }
+
+    /**
+     * Test for lti_get_lti_types_by_course.
+     *
+     * Note: This includes verification of the broken legacy behaviour in which the inclusion of course and site tools could be
+     * controlled independently, based on the capabilities 'mod/lti:addmanualinstance' (to include course tools) and
+     * 'mod/lti:addpreconfiguredinstance' (to include site tools). This behaviour is deprecated in 4.3 and all preconfigured tools
+     * are controlled by the single capability 'mod/lti:addpreconfiguredinstance'.
+     *
+     * @covers ::lti_get_lti_types_by_course()
+     * @return void
+     */
+    public function test_lti_get_lti_types_by_course(): void {
+        $this->resetAfterTest();
+
+        global $DB;
+        $coursecat1 = $this->getDataGenerator()->create_category();
+        $coursecat2 = $this->getDataGenerator()->create_category();
+        $course = $this->getDataGenerator()->create_course(['category' => $coursecat1->id]);
+        $course2 = $this->getDataGenerator()->create_course(['category' => $coursecat2->id]);
+        $teacher = $this->getDataGenerator()->create_and_enrol($course, 'editingteacher');
+        $teacher2 = $this->getDataGenerator()->create_and_enrol($course2, 'editingteacher');
+
+        // Create the following tool types for testing:
+        // - Site tool configured as "Do not show" (LTI_COURSEVISIBLE_NO).
+        // - Site tool configured as "Show as a preconfigured tool only" (LTI_COURSEVISIBLE_PRECONFIGURED).
+        // - Site tool configured as "Show as a preconfigured tool and in the activity chooser" (LTI_COURSEVISIBLE_ACTIVITYCHOOSER).
+        // - Course tool which, by default, is configured as LTI_COURSEVISIBLE_ACTIVITYCHOOSER).
+        // - Site tool configured to "Show as a preconfigured tool and in the activity chooser" but restricted to a category.
+
+        /** @var \mod_lti_generator $ltigenerator */
+        $ltigenerator = $this->getDataGenerator()->get_plugin_generator('mod_lti');
+        $ltigenerator->create_tool_types([
+            'name' => 'site tool do not show',
+            'baseurl' => 'http://example.com/tool/1',
+            'coursevisible' => LTI_COURSEVISIBLE_NO,
+            'state' => LTI_TOOL_STATE_CONFIGURED
+        ]);
+        $ltigenerator->create_tool_types([
+            'name' => 'site tool preconfigured only',
+            'baseurl' => 'http://example.com/tool/2',
+            'coursevisible' => LTI_COURSEVISIBLE_PRECONFIGURED,
+            'state' => LTI_TOOL_STATE_CONFIGURED
+        ]);
+        $ltigenerator->create_tool_types([
+            'name' => 'site tool preconfigured and activity chooser',
+            'baseurl' => 'http://example.com/tool/3',
+            'coursevisible' => LTI_COURSEVISIBLE_ACTIVITYCHOOSER,
+            'state' => LTI_TOOL_STATE_CONFIGURED
+        ]);
+        $ltigenerator->create_course_tool_types([
+            'name' => 'course tool preconfigured and activity chooser',
+            'baseurl' => 'http://example.com/tool/4',
+            'course' => $course->id
+        ]);
+        $ltigenerator->create_tool_types([
+            'name' => 'site tool preconfigured and activity chooser, restricted to category 2',
+            'baseurl' => 'http://example.com/tool/5',
+            'coursevisible' => LTI_COURSEVISIBLE_ACTIVITYCHOOSER,
+            'state' => LTI_TOOL_STATE_CONFIGURED,
+            'lti_coursecategories' => $coursecat2->id
+        ]);
+
+        $this->setUser($teacher); // Important: this deprecated method depends on the global user for cap checks.
+
+        // Request using the default 'coursevisible' param will include all tools except the one configured as "Do not show".
+        $coursetooltypes = lti_get_lti_types_by_course($course->id);
+        $this->assertDebuggingCalled();
+        $this->assertCount(3, $coursetooltypes);
+        $this->assertEmpty(array_diff(
+            ['http://example.com/tool/2', 'http://example.com/tool/3', 'http://example.com/tool/4'],
+            array_column($coursetooltypes, 'baseurl')
+        ));
+
+        // Request for only those tools configured to show in the activity chooser for the teacher.
+        $coursetooltypes = lti_get_lti_types_by_course($course->id, [LTI_COURSEVISIBLE_ACTIVITYCHOOSER]);
+        $this->assertDebuggingCalled();
+        $this->assertCount(2, $coursetooltypes);
+        $this->assertEmpty(array_diff(
+            ['http://example.com/tool/3', 'http://example.com/tool/4'],
+            array_column($coursetooltypes, 'baseurl')
+        ));
+
+        // Request for only those tools configured to show as a preconfigured tool for the teacher.
+        $coursetooltypes = lti_get_lti_types_by_course($course->id, [LTI_COURSEVISIBLE_PRECONFIGURED]);
+        $this->assertDebuggingCalled();
+        $this->assertCount(1, $coursetooltypes);
+        $this->assertEmpty(array_diff(
+            ['http://example.com/tool/2'],
+            array_column($coursetooltypes, 'baseurl')
+        ));
+
+        // Request for teacher2 in course2 (course category 2).
+        $this->setUser($teacher2);
+        $coursetooltypes = lti_get_lti_types_by_course($course2->id);
+        $this->assertDebuggingCalled();
+        $this->assertCount(3, $coursetooltypes);
+        $this->assertEmpty(array_diff(
+            ['http://example.com/tool/2', 'http://example.com/tool/3', 'http://example.com/tool/5'],
+            array_column($coursetooltypes, 'baseurl')
+        ));
+
+        // Request for a teacher who cannot use preconfigured tools in the course.
+        // No tools are available.
+        $this->setUser($teacher);
+        $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'));
+        assign_capability('mod/lti:addpreconfiguredinstance', CAP_PROHIBIT, $teacherrole->id,
+            \core\context\course::instance($course->id));
+        $coursetooltypes = lti_get_lti_types_by_course($course->id);
+        $this->assertDebuggingCalled();
+        $this->assertCount(1, $coursetooltypes);
+        $this->unassignUserCapability('mod/lti:addpreconfiguredinstance', (\core\context\course::instance($course->id))->id,
+            $teacherrole->id);
+
+        // Request for a teacher who cannot use manually configured tools in the course.
+        // Only return site tools, which is broken legacy behaviour.
+        assign_capability('mod/lti:addmanualinstance', CAP_PROHIBIT, $teacherrole->id,
+            \context_course::instance($course->id));
+        $coursetooltypes = lti_get_lti_types_by_course($course->id);
+        $this->assertDebuggingCalled();
+        $this->assertCount(2, $coursetooltypes);
+    }
 }
@@ -5,6 +5,7 @@ This files describes API changes in the lti code.
 * The `lti_libxml_disable_entity_loader` method is deprecated, as it is no longer required from PHP 8.0
 * The `mod_lti_mod_form` constructor will now throw an exception if called without passing a typeid as manual configuration of
  instances is now unsupported.
+* The `lti_get_lti_types_by_course` method is deprecated. Please use mod_lti\local\types_helper::get_lti_types_by_course instead.

 === 4.2 ===