mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-19 07:25:30 +02:00
Code Issues Projects Releases Wiki Activity

MDL-78597 mod_lti: remove the ability to add manual instances of tools

Browse Source 
In summary:
- Remove the 'External tool' item from get_course_content_items()
hook, which results in its removal from the activity chooser.
- Remove the 'External tool' item from get_all_content_items() hook,
which results in its removal from the admin activity chooser
recommendations page.
- Prevent use of the edit_form for creation of new manual instances.
- Retain the ability to edit existing manual instances.
- Fix tests expecting external tool.
This commit is contained in:
Jake Dallimore 2023-07-26 14:14:12 +08:00
parent 36900136a8
commit ca10084639
No known key found for this signature in database
7 changed files with 58 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
mod/lti/lang/en/lti.php
View File

@ -289,6 +289,7 @@ $string['launchoptions'] = 'Launch options';
$string['lti'] = 'LTI';
$string['lti:addcoursetool'] = 'Add course-specific tool configurations';
$string['lti:addmanualinstance'] = 'Add a manually-configured tool';
$string['lti:addmanualinstanceprohibitederror'] = 'The manual creation of tools without a course tool definition is no longer supported. Please create a course tool first and then use that to create activity instances.';
$string['lti:addinstance'] = 'Add a new external tool';
$string['lti:addpreconfiguredinstance'] = 'Add a preconfigured tool';
$string['lti:grade'] = 'View grades returned by the external tool';

35
mod/lti/lib.php
View File

@ -240,22 +240,6 @@ function lti_get_course_content_items(\core_course\local\entity\content_item $de
$types = [];
// The 'External tool' entry (the main module content item), should always take the id of 1.
if (has_capability('mod/lti:addmanualinstance', context_course::instance($course->id), $user)) {
$types = [new \core_course\local\entity\content_item(
1,
$defaultmodulecontentitem->get_name(),
$defaultmodulecontentitem->get_title(),
$defaultmodulecontentitem->get_link(),
$defaultmodulecontentitem->get_icon(),
$defaultmodulecontentitem->get_help(),
$defaultmodulecontentitem->get_archetype(),
$defaultmodulecontentitem->get_component_name(),
$defaultmodulecontentitem->get_purpose()
)];
}
// Other, preconfigured tools take their own id + 1, so we'll never clash with the module's entry.
$preconfiguredtools = lti_get_configured_types($course->id, $defaultmodulecontentitem->get_link()->param('sr'));
foreach ($preconfiguredtools as $preconfiguredtool) {
@ -270,6 +254,9 @@ function lti_get_course_content_items(\core_course\local\entity\content_item $de
$preconfiguredtool->help = '';
}
// Preconfigured tools take their own id + 1. This logic exists because, previously, the entry permitting manual instance
// creation (the $defaultmodulecontentitem, or 'External tool' item) was included and had the id 1. This logic prevented id
// collisions.
$types[] = new \core_course\local\entity\content_item(
$preconfiguredtool->id + 1,
$preconfiguredtool->name,
@ -295,18 +282,7 @@ function mod_lti_get_all_content_items(\core_course\local\entity\content_item $d
global $OUTPUT, $CFG;
require_once($CFG->dirroot . '/mod/lti/locallib.php'); // For access to constants.
// The 'External tool' entry (the main module content item), should always take the id of 1.
$types = [new \core_course\local\entity\content_item(
1,
$defaultmodulecontentitem->get_name(),
$defaultmodulecontentitem->get_title(),
$defaultmodulecontentitem->get_link(),
$defaultmodulecontentitem->get_icon(),
$defaultmodulecontentitem->get_help(),
$defaultmodulecontentitem->get_archetype(),
$defaultmodulecontentitem->get_component_name(),
$defaultmodulecontentitem->get_purpose()
)];
$types = [];
foreach (lti_get_lti_types() as $ltitype) {
if ($ltitype->coursevisible != LTI_COURSEVISIBLE_ACTIVITYCHOOSER) {
@ -332,6 +308,9 @@ function mod_lti_get_all_content_items(\core_course\local\entity\content_item $d
}
$type->link = new moodle_url('/course/modedit.php', array('add' => 'lti', 'return' => 0, 'typeid' => $ltitype->id));
// Preconfigured tools take their own id + 1. This logic exists because, previously, the entry permitting manual instance
// creation (the $defaultmodulecontentitem, or 'External tool' item) was included and had the id 1. This logic prevented id
// collisions.
$types[] = new \core_course\local\entity\content_item(
$type->id + 1,
$type->name,

12
mod/lti/mod_form.php
View File

@ -62,10 +62,13 @@ class mod_lti_mod_form extends moodleform_mod {
/**
* Constructor.
*
* Throws an exception if trying to init the form for a new manual instance of a tool, which is not supported in 4.3 onward.
*
* @param \stdClass $current the current form data.
* @param string $section the section number.
* @param \stdClass $cm the course module object.
* @param \stdClass $course the course object.
* @throws moodle_exception if trying to init the form for the creation of a manual instance, which is no longer supported.
*/
public function __construct($current, $section, $cm, $course) {
@ -74,6 +77,15 @@ class mod_lti_mod_form extends moodleform_mod {
$this->typeid = optional_param('typeid', null, PARAM_INT);
$this->type = optional_param('type', null, PARAM_ALPHA);
// Only permit construction if the form deals with editing an existing instance (current->id not empty), or creating an
// instance from a preconfigured tool type ($this->typeid not empty).
global $PAGE;
if ($PAGE->has_set_url() && str_contains($PAGE->url, '/course/modedit.php')) {
if (empty($this->typeid) && empty($current->id)) {
throw new moodle_exception('lti:addmanualinstanceprohibitederror', 'mod_lti');
}
}
parent::__construct($current, $section, $cm, $course);
}

57
mod/lti/tests/behat/backup_restore.feature
View File

@ -15,30 +15,20 @@ Feature: Restoring Moodle 2 backup restores LTI configuration
| teacher1 | C2 | editingteacher |
Scenario: Backup and restore course with preconfigured site LTI tool on the same site
When I log in as "admin"
And I navigate to "Plugins > Activity modules > External tool > Manage tools" in site administration
And I follow "Manage preconfigured tools"
And I follow "Add preconfigured tool"
And I set the following fields to these values:
| Tool name | My site tool |
| Tool URL | https://www.moodle.org |
| lti_coursevisible | 1 |
And I press "Save changes"
And I navigate to "Plugins > Activity modules > External tool > Manage tools" in site administration
And "This tool has not yet been used" "text" should exist in the "//div[contains(@id,'tool-card-container') and contains(., 'My site tool')]" "xpath_element"
And I am on site homepage
And I am on "Course 1" course homepage
And I turn editing mode on
And I add a "External tool" to section "1" and I fill the form with:
| Activity name | My LTI module |
| Preconfigured tool | My site tool |
| Launch container | Embed |
Given the following "mod_lti > tool types" exist:
| name | description | baseurl | coursevisible | state |
| My site tool | Site tool description | https://www.moodle.org | 2 | 1 |
And the following "mod_lti > tool instances" exist:
| name | tool | course |
| My LTI module | My site tool | C1 |
And I am on the "Course 1" course page logged in as admin
And I should see "My LTI module"
And I backup "Course 1" course using this options:
When I backup "Course 1" course using this options:
| Confirmation | Filename | test_backup.mbz |
And I restore "test_backup.mbz" backup into a new course using this options:
And I am on site homepage
And I follow "Course 1 copy 1"
And I turn editing mode on
And I open "My LTI module" actions menu
And I choose "Edit settings" in the open action menu
Then the field "Preconfigured tool" matches value "My site tool"
@ -47,29 +37,20 @@ Feature: Restoring Moodle 2 backup restores LTI configuration
@javascript @_switch_window
Scenario: Backup and restore course with preconfigured course LTI tool on the same site
When I log in as "teacher1"
And I am on "Course 1" course homepage with editing mode on
# In the first course create an LTI module that uses a course preconfigured toolю
And I add a "External tool" to section "1"
And I set the following fields to these values:
| Activity name | Test tool activity 2 |
And I follow "Add preconfigured tool"
And I switch to "add_tool" window
And I set the field "Tool name" to "My course tool"
And I set the field "Tool URL" to "http://www.example.com/lti/provider.php"
And I set the field "Consumer key" to "my key"
And I set the field "Shared secret" to "my secret"
And I set the field "Default launch container" to "Existing window"
And I press "Save changes"
And I switch to the main window
And I press "Save and return to course"
Given the following "mod_lti > course tools" exist:
| name | description | baseurl | course | lti_resourcekey | lti_password | lti_launchcontainer |
| My course tool | Example description | http://www.example.com/lti/provider.php | C1 | my key | my secret | 5 |
# In the first course create an LTI module that uses a course preconfigured tool
And the following "mod_lti > tool instances" exist:
| name | tool | course |
| Test tool activity 2 | My course tool | C1 |
And I am on the "Course 1" course page logged in as admin
# Backup course and restore into another course
And I backup "Course 1" course using this options:
| Confirmation | Filename | test_backup.mbz |
And I restore "test_backup.mbz" backup into "Course 2" course using this options:
And I am on site homepage
And I follow "Course 2"
# Make sure the copy of the preconfigured tool was created in the second course with both encrtypted and non-encrypted properties.
And I am on "Course 2" course homepage with editing mode on
# Make sure the copy of the preconfigured tool was created in the second course with both encrypted and non-encrypted properties.
And I open "Test tool activity 2" actions menu
And I choose "Edit settings" in the open action menu
Then the field "Preconfigured tool" matches value "My course tool"

36
mod/lti/tests/behat/contentitem.feature
View File

@ -14,18 +14,9 @@ Feature: Content-Item support
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
And I log in as "admin"
And I navigate to "Plugins > Activity modules > External tool > Manage tools" in site administration
# Create tool type that supports deep linking.
And I follow "configure a tool manually"
And I set the field "Tool name" to "Teaching Tool 1"
And I set the field "Tool URL" to local url "/mod/lti/tests/fixtures/tool_provider.php"
And I set the field "Tool configuration usage" to "Show in activity chooser and as a preconfigured tool"
And I expand all fieldsets
And I set the field "Supports Deep Linking (Content-Item Message)" to "1"
And I press "Save changes"
And I should see "Teaching Tool 1"
And I log out
And the following "mod_lti > tool types" exist:
| name | description | baseurl | coursevisible | state | lti_contentitem |
| Teaching Tool 1 | Tool 1 description | /mod/lti/tests/fixtures/tool_provider.php | 2 | 1 | 1 |
@javascript
Scenario: Tool that supports Deep Linking should be able to configure a tool via the Select content button
@ -51,19 +42,14 @@ Feature: Content-Item support
@javascript
Scenario: Changing preconfigured tool selection
Given I log in as "admin"
And I navigate to "Plugins > Activity modules > External tool > Manage tools" in site administration
And I follow "configure a tool manually"
And I set the field "Tool name" to "Teaching Tool 2"
And I set the field "Tool URL" to local url "/mod/lti/tests/fixtures/tool_provider.php"
And I set the field "Tool configuration usage" to "Show in activity chooser and as a preconfigured tool"
And I expand all fieldsets
And I press "Save changes"
And I should see "Teaching Tool 2"
And I log out
When I log in as "teacher1"
And I am on "Course 1" course homepage with editing mode on
And I add a "External tool" to section "1"
Given the following "mod_lti > tool types" exist:
| name | description | baseurl | coursevisible | state |
| Teaching Tool 2 | Another description | /mod/lti/tests/fixtures/tool_provider.php | 2 | 1 |
# Create a manually configured instance using the generator (this isn't possible via the UI any more).
And the following "activities" exist:
| activity | course | name | typeid | toolurl |
| lti | C1 | Test tool activity 1 | 0 | /mod/lti/tests/fixtures/tool_provider.php |
When I am on the "Test tool activity 1" "lti activity editing" page logged in as teacher1
# On load with no preconfigured tool selected: Select content button - disabled, Tool URL - enabled.
And the field "Preconfigured tool" matches value "Automatic, based on tool URL"
And I set the field "Activity name" to "Test tool activity 1"

18
mod/lti/tests/lib_test.php
View File

@ -412,43 +412,37 @@ class lib_test extends \advanced_testcase {
// The lti_get_lti_types_by_course method (used by the callbacks) assumes the global user.
$this->setUser($teacher);
// Teacher in course1 should be able to see the default module item ('external tool'),
// the site preconfigured tool and the tool created in course1.
// Teacher in course1 should be able to see the site preconfigured tool and the tool created in course1.
$courseitems = lti_get_course_content_items($defaultmodulecontentitem, $teacher, $course);
$this->assertCount(3, $courseitems);
$this->assertCount(2, $courseitems);
$ids = [];
foreach ($courseitems as $item) {
$ids[] = $item->get_id();
}
$this->assertContains(1, $ids);
$this->assertContains($sitetoolrecord->id + 1, $ids);
$this->assertContains($course1toolrecord->id + 1, $ids);
$this->assertNotContains($sitetoolrecordnonchooser->id + 1, $ids);
// The content items for teacher2 in course2 include the default module content item ('external tool'),
// the site preconfigured tool and the tool created in course2.
// The content items for teacher2 in course2 include the site preconfigured tool and the tool created in course2.
$this->setUser($teacher2);
$course2items = lti_get_course_content_items($defaultmodulecontentitem, $teacher2, $course2);
$this->assertCount(3, $course2items);
$this->assertCount(2, $course2items);
$ids = [];
foreach ($course2items as $item) {
$ids[] = $item->get_id();
}
$this->assertContains(1, $ids);
$this->assertContains($sitetoolrecord->id + 1, $ids);
$this->assertContains($course2toolrecord->id + 1, $ids);
$this->assertNotContains($sitetoolrecordnonchooser->id + 1, $ids);
// When fetching all content items, we expect to see all items available in activity choosers (in any course),
// plus the default module content item ('external tool').
// When fetching all content items, we expect to see all items available in activity choosers (in any course).
$this->setAdminUser();
$allitems = mod_lti_get_all_content_items($defaultmodulecontentitem);
$this->assertCount(4, $allitems);
$this->assertCount(3, $allitems);
$ids = [];
foreach ($allitems as $item) {
$ids[] = $item->get_id();
}
$this->assertContains(1, $ids);
$this->assertContains($sitetoolrecord->id + 1, $ids);
$this->assertContains($course1toolrecord->id + 1, $ids);
$this->assertContains($course2toolrecord->id + 1, $ids);

2
mod/lti/upgrade.txt
View File

@ -3,6 +3,8 @@ This files describes API changes in the lti code.
=== 4.3 ===
* 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.
=== 4.2 ===