From 0450f5e0819fab1539d58bdabb7c9450917fe176 Mon Sep 17 00:00:00 2001
From: John Okely <john@moodle.com>
Date: Thu, 19 May 2016 17:03:47 +0800
Subject: [PATCH] MDL-53988 mod_lti: Improve error handling and user interface

Only show one, more helpful error instead of two non-helpful errors.
Don't show the 'no configured tools' message if there is a proxy there.
Return to the Tool configure page when editing a proxy if you came
from it.
---
 mod/lti/amd/src/tool_type.js         |  4 ++--
 mod/lti/lang/en/lti.php              |  2 +-
 mod/lti/registersettings.php         | 11 +++++++++++
 mod/lti/templates/tool_list.mustache |  4 ++--
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/mod/lti/amd/src/tool_type.js b/mod/lti/amd/src/tool_type.js
index 15efac6d738..729f8c3a99f 100644
--- a/mod/lti/amd/src/tool_type.js
+++ b/mod/lti/amd/src/tool_type.js
@@ -53,6 +53,8 @@ define(['core/ajax', 'core/notification'], function(ajax, notification) {
         /**
          * Create a tool type in Moodle.
          *
+         * The promise will fail if the URL is not a cartridge, so you must handle the fail result.
+         *
          * See also:
          * mod/lti/classes/external.php create_tool_type_parameters()
          *
@@ -69,8 +71,6 @@ define(['core/ajax', 'core/notification'], function(ajax, notification) {
 
             var promise = ajax.call([request])[0];
 
-            promise.fail(notification.exception);
-
             return promise;
         },
 
diff --git a/mod/lti/lang/en/lti.php b/mod/lti/lang/en/lti.php
index e0d57ad2bdf..324931eaa21 100644
--- a/mod/lti/lang/en/lti.php
+++ b/mod/lti/lang/en/lti.php
@@ -191,7 +191,7 @@ Three icons are available after the preconfigured tool dropdown list:
 * **Delete** - Remove the selected course level tool.';
 $string['external_tool_types'] = 'Preconfigured tools';
 $string['failedtoconnect'] = 'Moodle was unable to communicate with the "{$a}" system';
-$string['failedtocreatetooltype'] = 'Failed to create new tool.';
+$string['failedtocreatetooltype'] = 'Failed to create new tool. Please check the url and try again.';
 $string['failedtodeletetoolproxy'] = 'Failed to delete tool registration. You may need to visit "Manage external tool registrations" and delete it manually.';
 $string['filter_basiclti_configlink'] = 'Configure your preferred sites and their passwords';
 $string['filter_basiclti_password'] = 'Password is mandatory';
diff --git a/mod/lti/registersettings.php b/mod/lti/registersettings.php
index c58bf0e80ae..797f2ada248 100644
--- a/mod/lti/registersettings.php
+++ b/mod/lti/registersettings.php
@@ -34,6 +34,11 @@ require_once($CFG->dirroot.'/mod/lti/locallib.php');
 $action       = optional_param('action', null, PARAM_ALPHANUMEXT);
 $id           = optional_param('id', null, PARAM_INT);
 $tab          = optional_param('tab', '', PARAM_ALPHAEXT);
+$returnto     = optional_param('returnto', '', PARAM_ALPHA);
+
+if ($returnto == 'toolconfigure') {
+    $returnurl = new moodle_url($CFG->wwwroot . '/mod/lti/toolconfigure.php');
+}
 
 // No guest autologin.
 require_login(0, false);
@@ -43,12 +48,18 @@ $pageurl = new moodle_url('/mod/lti/registersettings.php');
 if ($isupdate) {
     $pageurl->param('id', $id);
 }
+if (!empty($returnto)) {
+    $pageurl->param('returnto', $returnto);
+}
 $PAGE->set_url($pageurl);
 
 admin_externalpage_setup('ltitoolproxies');
 
 $redirect = new moodle_url('/mod/lti/toolproxies.php', array('tab' => $tab));
 $redirect = $redirect->out();
+if (!empty($returnurl)) {
+    $redirect = $returnurl;
+}
 
 require_sesskey();
 
diff --git a/mod/lti/templates/tool_list.mustache b/mod/lti/templates/tool_list.mustache
index 70f99a75b93..b23338fb72b 100644
--- a/mod/lti/templates/tool_list.mustache
+++ b/mod/lti/templates/tool_list.mustache
@@ -37,9 +37,9 @@
 <div id="tool-list-loader-container">
     {{> mod_lti/loader }}
 </div>
-{{^tools}}
+{{^proxies}}{{^tools}}
 <div id="tool-notools-text" class="container-fluid">{{#str}} no_lti_tools, mod_lti {{/str}}</div>
-{{/tools}}
+{{/tools}}{{/proxies}}
 <div id="tool-card-container" class="container-fluid">
     {{#proxies}}
         {{> mod_lti/tool_proxy_card }}