MDL-30344 lti - Fix issue with privacy settings not working for a course level tool if no Launch URL is specified.

mod/lti/ajax.php

@@ -39,14 +39,23 @@ $response = new stdClass();
 switch ($action) {
     case 'find_tool_config':
         $toolurl = required_param('toolurl', PARAM_RAW);
+        $toolid = optional_param('toolid', 0, PARAM_INT);
 
-        $tool = lti_get_tool_by_url_match($toolurl, $courseid);
-
-        if (!empty($tool)) {
-            $response->toolid = $tool->id;
-            $response->toolname = htmlspecialchars($tool->name);
-            $response->tooldomain = htmlspecialchars($tool->tooldomain);
+        if(empty($toolid) && !empty($toolurl)){
+            $tool = lti_get_tool_by_url_match($toolurl, $courseid);
             
+            if(!empty($tool)){
+                $toolid = $tool->id;
+            
+                $response->toolid = $tool->id;
+                $response->toolname = htmlspecialchars($tool->name);
+                $response->tooldomain = htmlspecialchars($tool->tooldomain);
+            }
+        } else {
+            $response->toolid = $toolid;
+        }
+        
+        if (!empty($toolid)) {
             //Look up privacy settings
             $query = 
             '
@@ -57,7 +66,7 @@ switch ($action) {
                 AND name IN (\'sendname\', \'sendemailaddr\', \'acceptgrades\', \'allowroster\')
             ';
                         
-            $privacyconfigs = $DB->get_records_sql($query, array('typeid' => $tool->id));
+            $privacyconfigs = $DB->get_records_sql($query, array('typeid' => $toolid));
             foreach($privacyconfigs as $config){
                 $configname = $config->name;
                 $response->$configname = $config->value;

mod/lti/mod_form.js

@@ -40,6 +40,7 @@
             this.settings = Y.JSON.parse(settings);
 
             this.urlCache = {};
+            this.toolTypeCache = {};
 
             this.addOptGroups();
 
@@ -81,6 +82,7 @@
 
         clearToolCache: function(){
             this.urlCache = {};
+            this.toolTypeCache = {};
         },
 
         updateAutomaticToolMatch: function(field){
@@ -110,7 +112,7 @@
                 automatchToolDisplay.setStyle('display', '');
             }
 
-            var selectedToolType = typeSelector.get('value');
+            var selectedToolType = parseInt(typeSelector.get('value'));
             var selectedOption = typeSelector.one('option[value="' + selectedToolType + '"]');
 
             //A specific tool type is selected (not "auto")
@@ -141,7 +143,7 @@
 
                 if(toolInfo.toolname){
                     automatchToolDisplay.set('innerHTML',  '<img style="vertical-align:text-bottom" src="' + self.settings.green_check_icon_url + '" />' + M.str.lti.using_tool_configuration + toolInfo.toolname);
-                } else {
+                } else if(!selectedToolType) {
                     //Inform them custom configuration is in use
                     if(key.get('value') === '' || secret.get('value') === ''){
                         automatchToolDisplay.set('innerHTML', '<img style="vertical-align:text-bottom" src="' + self.settings.warning_icon_url + '" />' + M.str.lti.tool_config_not_found);
@@ -150,13 +152,24 @@
             };
 
             //Cache urls which have already been checked to increase performance
-            if(self.urlCache[url]){
-                continuation(self.urlCache[url]);
+            //Don't use URL cache if tool type manually selected
+            if(selectedToolType && self.toolTypeCache[selectedToolType]){
+                return continuation(self.toolTypeCache[selectedToolType]);
+            } else if(self.urlCache[url] && !selectedToolType){
+                return continuation(self.urlCache[url]);
+            } else if(!selectedToolType && !url) {
+                //No tool type or url set
+                return continuation({});
             } else {
-                self.findToolByUrl(url, function(toolInfo){
+                self.findToolByUrl(url, selectedToolType, function(toolInfo){
                     if(toolInfo){
-                        self.urlCache[url] = toolInfo;
-
+                        //Cache the result based on whether the URL or tool type was used to look up the tool
+                        if(!selectedToolType){
+                            self.urlCache[url] = toolInfo;
+                        } else {
+                            self.toolTypeCache[selectedToolType] = toolInfo;
+                        }
+                        
                         continuation(toolInfo);
                     }
                 });
@@ -167,7 +180,7 @@
          * Updates display of privacy settings to show course / site tool configuration settings.
          */
         updatePrivacySettings: function(toolInfo){
-            if(!toolInfo || !toolInfo.toolname){
+            if(!toolInfo || !toolInfo.toolid){
                 toolInfo = {
                     sendname: M.mod_lti.LTI_SETTING_DELEGATE,
                     sendemailaddr: M.mod_lti.LTI_SETTING_DELEGATE,
@@ -395,17 +408,14 @@
             });
         },
 
-        findToolByUrl: function(url, callback){
+        findToolByUrl: function(url, toolId, callback){
             var self = this;
             
-            if(!url || url === ''){
-                return callback();
-            }
-
             Y.io(self.settings.ajax_url, {
                 data: {action: 'find_tool_config',
                         course: self.settings.courseId,
-                        toolurl: url
+                        toolurl: url,
+                        toolid: toolId || 0
                 },
 
                 on: {

mod/lti/styles.css

@@ -31,5 +31,9 @@
 /* Styles for admin */
 .path-admin-mod-lti .mform .fitem .fitemtitle { min-width:18em;padding-right:1em } /* Prevent setting titles from wrapping */
 
+/* Styles for instructor editing an external tool */
+
+.path-mod-lti .mform .fitem .fitemtitle { min-width:14em;padding-right:1em } /* Prevent setting titles from wrapping */
+
 /* Styles for instructor_edit_tool_type.php */
 #page-mod-lti-instructor_edit_tool_type .mform .fitem .fitemtitle { min-width:18em;padding-right:1em } /* Prevent setting titles from wrapping */