From 2d3130532c94c256ee80381fde4291367d0b9a8d Mon Sep 17 00:00:00 2001
From: Jun Pataleta <jun@moodle.com>
Date: Mon, 3 Oct 2016 15:31:46 +0800
Subject: [PATCH] MDL-56232 mod_lti: Fix logic for disabling mod_form fields

* Disable tool "Select content" button if selected preconfigured tool
  does not support Content-Item, and if the selected option is set to
  "Automatic, based on launch URL".
* Disable tool configuration form fields if selected preconfigured tool
  does not support Content-Item, but enable these fields if the selected
  option is set to "Automatic, based on launch URL".
* Behat tests also updated to check disabling/enabling of launch url field.
---
 mod/lti/mod_form.php                    |  8 +++---
 mod/lti/tests/behat/contentitem.feature | 35 ++++++++++++++++++++-----
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/mod/lti/mod_form.php b/mod/lti/mod_form.php
index eea82d98b27..3d425f82bdf 100644
--- a/mod/lti/mod_form.php
+++ b/mod/lti/mod_form.php
@@ -107,7 +107,7 @@ class mod_lti_mod_form extends moodleform_mod {
         $toolproxy = array();
 
         // Array of tool type IDs that don't support ContentItemSelectionRequest.
-        $noncontentitemtypes = ['0'];
+        $noncontentitemtypes = [];
 
         foreach (lti_get_types_for_add_instance() as $id => $type) {
             if (!empty($type->toolproxyid)) {
@@ -152,7 +152,10 @@ class mod_lti_mod_form extends moodleform_mod {
         if ($update) {
             $mform->disabledIf('selectcontent', 'typeid', 'neq', 0);
         } else {
-            $mform->disabledIf('selectcontent', 'typeid', 'in', $noncontentitemtypes);
+            // Disable select content button if the selected tool doesn't support content item or it's set to Automatic.
+            $allnoncontentitemtypes = $noncontentitemtypes;
+            $allnoncontentitemtypes[] = '0'; // Add option value for "Automatic, based on launch URL".
+            $mform->disabledIf('selectcontent', 'typeid', 'in', $allnoncontentitemtypes);
         }
 
         $mform->addElement('text', 'toolurl', get_string('launch_url', 'lti'), array('size' => '64'));
@@ -193,7 +196,6 @@ class mod_lti_mod_form extends moodleform_mod {
         $mform->setType('resourcekey', PARAM_TEXT);
         $mform->setAdvanced('resourcekey');
         $mform->addHelpButton('resourcekey', 'resourcekey', 'lti');
-        $mform->disabledIf('resourcekey', 'typeid', 'neq', '0');
         if ($update) {
             $mform->disabledIf('resourcekey', 'typeid', 'neq', 0);
         } else {
diff --git a/mod/lti/tests/behat/contentitem.feature b/mod/lti/tests/behat/contentitem.feature
index 07ee81859f5..2e4f41a56cf 100644
--- a/mod/lti/tests/behat/contentitem.feature
+++ b/mod/lti/tests/behat/contentitem.feature
@@ -16,6 +16,7 @@ Feature: Content-Item support
       | teacher1 | C1 | editingteacher |
     And I log in as "admin"
     And I navigate to "Manage tools" node in "Site administration > Plugins > Activity modules > External tool"
+    # Create tool type that supports content-item.
     And I follow "configure a tool manually"
     And I set the field "Tool name" to "Teaching Tool 1"
     And I set the field "Tool base URL/cartridge URL" to local url "/mod/lti/tests/fixtures/tool_provider.php"
@@ -34,7 +35,7 @@ Feature: Content-Item support
     Then the "Select content" "button" should be enabled
 
   @javascript
-  Scenario: Adding a preconfigured tool that does not support Content-Item.
+  Scenario: Editing a tool's settings that was configured from a preconfigured tool that supports Content-Item.
     When I log in as "teacher1"
     And I follow "Course 1"
     And I turn editing mode on
@@ -51,15 +52,37 @@ Feature: Content-Item support
     And the "Select content" "button" should be disabled
 
   @javascript
-  Scenario: Selecting a preconfigured tool that supports Content-Item
+  Scenario: Changing preconfigured tool selection
+    Given I log in as "admin"
+    And I navigate to "Manage tools" node in "Site administration > Plugins > Activity modules > External tool"
+    And I follow "configure a tool manually"
+    And I set the field "Tool name" to "Teaching Tool 2"
+    And I set the field "Tool base URL/cartridge 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 log out
     When I log in as "teacher1"
     And I follow "Course 1"
     And I turn editing mode on
     And I add a "External tool" to section "1"
+    # On load with no preconfigured tool selected: Select content button - disabled, Launch/cartridge URL - enabled.
     And the field "Preconfigured tool" matches value "Automatic, based on launch URL"
-    And the "Select content" "button" should be disabled
     And I set the field "Activity name" to "Test tool activity 1"
-    And I set the field "Preconfigured tool" to "Teaching Tool 1"
-    Then the "Select content" "button" should be enabled
-    And I set the field "Preconfigured tool" to "Automatic, based on launch URL"
     And the "Select content" "button" should be disabled
+    And the "Launch/cartridge URL" "field" should be enabled
+    # Selecting a tool that supports content-item: Select content button - enabled, Launch/cartridge URL - enabled.
+    And I set the field "Preconfigured tool" to "Teaching Tool 1"
+    And I set the field "Activity name" to "Test tool activity 1"
+    Then the "Select content" "button" should be enabled
+    And the "Launch/cartridge URL" "field" should be enabled
+    # Selecting a tool that does not support content-item: Select content button - disabled, Launch/cartridge URL - disabled.
+    And I set the field "Preconfigured tool" to "Teaching Tool 2"
+    And I set the field "Activity name" to "Test tool activity 1"
+    And the "Select content" "button" should be disabled
+    And the "Launch/cartridge URL" "field" should be disabled
+    # Not selecting any tool: Select content button - disabled, Launch/cartridge URL - enabled.
+    And I set the field "Preconfigured tool" to "Automatic, based on launch URL"
+    And I set the field "Activity name" to "Test tool activity 1"
+    And the "Select content" "button" should be disabled
+    And the "Launch/cartridge URL" "field" should be enabled