From 243468030a7ffd09f0d3d469dccc57a52698daac Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 16 Feb 2016 08:45:29 +0800 Subject: [PATCH 1/6] MDL-30811 output: Tidy up notifications --- .../tests/externallib_test.php | 4 +- lib/classes/output/notification.php | 89 +++++++++-- lib/deprecatedlib.php | 6 +- lib/outputrenderers.php | 138 +++++++++++------- ...t.mustache => notification_error.mustache} | 11 +- ...em.mustache => notification_info.mustache} | 11 +- lib/templates/notification_success.mustache | 7 +- ...mustache => notification_warning.mustache} | 11 +- lib/upgrade.txt | 7 + ...m.mustache => notification_error.mustache} | 0 ...ct.mustache => notification_info.mustache} | 0 ...mustache => notification_warning.mustache} | 0 theme/upgrade.txt | 5 + 13 files changed, 199 insertions(+), 90 deletions(-) rename lib/templates/{notification_redirect.mustache => notification_error.mustache} (73%) rename lib/templates/{notification_problem.mustache => notification_info.mustache} (73%) rename lib/templates/{notification_message.mustache => notification_warning.mustache} (73%) rename theme/base/templates/core/{notification_problem.mustache => notification_error.mustache} (100%) rename theme/base/templates/core/{notification_redirect.mustache => notification_info.mustache} (100%) rename theme/base/templates/core/{notification_message.mustache => notification_warning.mustache} (100%) diff --git a/admin/tool/templatelibrary/tests/externallib_test.php b/admin/tool/templatelibrary/tests/externallib_test.php index 9b09541f4d7..ac31ab64176 100644 --- a/admin/tool/templatelibrary/tests/externallib_test.php +++ b/admin/tool/templatelibrary/tests/externallib_test.php @@ -72,10 +72,10 @@ class tool_templatelibrary_external_testcase extends externallib_advanced_testca // Change the theme to 'base' because it overrides these templates. $CFG->theme = 'base'; - $template = external::load_canonical_template('core', 'notification_problem'); + $template = external::load_canonical_template('core', 'notification_error'); // Only the base template should contain the docs. - $this->assertContains('@template core/notification_problem', $template); + $this->assertContains('@template core/notification_error', $template); // Restore the original theme. $CFG->theme = $originaltheme; diff --git a/lib/classes/output/notification.php b/lib/classes/output/notification.php index 6cf19eddd84..f97af740486 100644 --- a/lib/classes/output/notification.php +++ b/lib/classes/output/notification.php @@ -23,7 +23,6 @@ */ namespace core\output; -use stdClass; /** * Data structure representing a notification. @@ -37,31 +36,57 @@ use stdClass; class notification implements \renderable, \templatable { /** + * A notification of level 'success'. + */ + const NOTIFY_SUCCESS = 'success'; + + /** + * A notification of level 'warning'. + */ + const NOTIFY_WARNING = 'warning'; + + /** + * A notification of level 'info'. + */ + const NOTIFY_INFO = 'info'; + + /** + * A notification of level 'error'. + */ + const NOTIFY_ERROR = 'error'; + + /** + * @deprecated * A generic message. */ const NOTIFY_MESSAGE = 'message'; + /** - * A message notifying the user of a successful operation. - */ - const NOTIFY_SUCCESS = 'success'; - /** + * @deprecated * A message notifying the user that a problem occurred. */ const NOTIFY_PROBLEM = 'problem'; + /** - * A message to display during a redirect.. + * @deprecated + * A notification of level 'redirect'. */ const NOTIFY_REDIRECT = 'redirect'; /** * @var string Message payload. */ - private $message = ''; + protected $message = ''; /** * @var string Message type. */ - private $messagetype = self::NOTIFY_PROBLEM; + protected $messagetype = self::NOTIFY_WARNING; + + /** + * @var array $extraclasses A list of any extra classes that may be required. + */ + protected $extraclasses = array(); /** * Notification constructor. @@ -69,11 +94,33 @@ class notification implements \renderable, \templatable { * @param string $message the message to print out * @param string $messagetype normally NOTIFY_PROBLEM or NOTIFY_SUCCESS. */ - public function __construct($message, $messagetype = self::NOTIFY_PROBLEM) { + public function __construct($message, $messagetype = null) { + $this->message = $message; + + if (empty($messagetype)) { + $messagetype = self::NOTIFY_ERROR; + } - $this->message = clean_text($message); $this->messagetype = $messagetype; + switch ($messagetype) { + case self::NOTIFY_PROBLEM: + case self::NOTIFY_REDIRECT: + case self::NOTIFY_MESSAGE: + debugging('Use of ' . $messagetype . ' has been deprecated. Please switch to an alternative type.'); + } + } + + /** + * Add any extra classes that this notification requires. + * + * @param array $classes + * @return $this + */ + public function set_extra_classes($classes = array()) { + $this->extraclasses = $classes; + + return $this; } /** @@ -83,12 +130,24 @@ class notification implements \renderable, \templatable { * @return stdClass data context for a mustache template */ public function export_for_template(\renderer_base $output) { + return array( + 'message' => clean_text($this->message), + 'extraclasses' => implode(' ', $this->extraclasses), + ); + } - $data = new stdClass(); + public function get_template_name() { + $templatemappings = [ + // Current types mapped to template names. + 'success' => 'core/notification_success', + 'info' => 'core/notification_info', + 'warning' => 'core/notification_warning', + 'error' => 'core/notification_error', + ]; - $data->type = $this->messagetype; - $data->message = $this->message; - - return $data; + if (isset($templatemappings[$this->messagetype])) { + return $templatemappings[$this->messagetype]; + } + return $templatemappings['error']; } } diff --git a/lib/deprecatedlib.php b/lib/deprecatedlib.php index 6222822795f..f71453cb494 100644 --- a/lib/deprecatedlib.php +++ b/lib/deprecatedlib.php @@ -912,14 +912,14 @@ function print_container_end($return=false) { * @param bool $return whether to return an output string or echo now * @return string|bool Depending on $result */ -function notify($message, $classes = 'notifyproblem', $align = 'center', $return = false) { +function notify($message, $classes = 'error', $align = 'center', $return = false) { global $OUTPUT; debugging('notify() is deprecated, please use $OUTPUT->notification() instead', DEBUG_DEVELOPER); if ($classes == 'green') { - debugging('Use of deprecated class name "green" in notify. Please change to "notifysuccess".', DEBUG_DEVELOPER); - $classes = 'notifysuccess'; // Backward compatible with old color system + debugging('Use of deprecated class name "green" in notify. Please change to "success".', DEBUG_DEVELOPER); + $classes = 'success'; // Backward compatible with old color system. } $output = $OUTPUT->notification($message, $classes); diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index 066f82e6a9f..99997c8075b 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -2778,38 +2778,63 @@ EOD; } /** - * Output a notification (that is, a status message about something that has - * just happened). + * Output a notification (that is, a status message about something that has just happened). * - * @param string $message the message to print out - * @param string $classes normally 'notifyproblem' or 'notifysuccess'. + * @param string $message The message to print out. + * @param string $type The type of notification. See constants on \core\output\notification. * @return string the HTML to output. */ - public function notification($message, $classes = 'notifyproblem') { + public function notification($message, $type = null) { + $typemappings = [ + // Valid types. + 'success' => \core\output\notification::NOTIFY_SUCCESS, + 'info' => \core\output\notification::NOTIFY_INFO, + 'warning' => \core\output\notification::NOTIFY_WARNING, + 'error' => \core\output\notification::NOTIFY_ERROR, - $classmappings = array( - 'notifyproblem' => \core\output\notification::NOTIFY_PROBLEM, - 'notifytiny' => \core\output\notification::NOTIFY_PROBLEM, - 'notifysuccess' => \core\output\notification::NOTIFY_SUCCESS, - 'notifymessage' => \core\output\notification::NOTIFY_MESSAGE, - 'redirectmessage' => \core\output\notification::NOTIFY_REDIRECT - ); + // Legacy types mapped to current types. + 'notifyproblem' => \core\output\notification::NOTIFY_ERROR, + 'notifytiny' => \core\output\notification::NOTIFY_ERROR, + 'notifyerror' => \core\output\notification::NOTIFY_ERROR, + 'notifysuccess' => \core\output\notification::NOTIFY_SUCCESS, + 'notifymessage' => \core\output\notification::NOTIFY_INFO, + 'notifyredirect' => \core\output\notification::NOTIFY_INFO, + 'redirectmessage' => \core\output\notification::NOTIFY_INFO, + ]; - // Identify what type of notification this is. - $type = \core\output\notification::NOTIFY_PROBLEM; - $classarray = explode(' ', self::prepare_classes($classes)); - if (count($classarray) > 0) { - foreach ($classarray as $class) { - if (isset($classmappings[$class])) { - $type = $classmappings[$class]; - break; + $extraclasses = []; + + if ($type) { + if (strpos($type, ' ') === false) { + // No spaces in the list of classes, therefore no need to loop over and determine the class. + if (isset($typemappings[$type])) { + $type = $typemappings[$type]; + } else { + // The value provided did not match a known type. It must be an extra class. + $extraclasses = [$type]; + } + } else { + // Identify what type of notification this is. + $classarray = explode(' ', self::prepare_classes($type)); + + // Separate out the type of notification from the extra classes. + foreach ($classarray as $class) { + if (isset($typemappings[$class])) { + $type = $typemappings[$class]; + } else { + $extraclasses[] = $class; + } } } } - $n = new \core\output\notification($message, $type); - return $this->render($n); + $notification = new \core\output\notification($message, $type); + if (count($extraclasses)) { + $notification->set_extra_classes($extraclasses); + } + // Return the rendered template. + return $this->render_from_template($notification->get_template_name(), $notification->export_for_template($this)); } /** @@ -2817,9 +2842,15 @@ EOD; * * @param string $message the message to print out * @return string HTML fragment. + * @deprecated since Moodle 3.1 MDL-30811 - please do not use this function any more. + * @todo MDL-53113 This will be removed in Moodle 3.5. + * @see \core\output\notification */ public function notify_problem($message) { - $n = new \core\output\notification($message, \core\output\notification::NOTIFY_PROBLEM); + debugging(__FUNCTION__ . ' is deprecated.' . + 'Please use notification() or \core\output\notification as required', + DEBUG_DEVELOPER); + $n = new \core\output\notification($message, \core\output\notification::NOTIFY_ERROR); return $this->render($n); } @@ -2828,8 +2859,14 @@ EOD; * * @param string $message the message to print out * @return string HTML fragment. + * @deprecated since Moodle 3.1 MDL-30811 - please do not use this function any more. + * @todo MDL-53113 This will be removed in Moodle 3.5. + * @see \core\output\notification */ public function notify_success($message) { + debugging(__FUNCTION__ . ' is deprecated.' . + 'Please use notification() or \core\output\notification as required', + DEBUG_DEVELOPER); $n = new \core\output\notification($message, \core\output\notification::NOTIFY_SUCCESS); return $this->render($n); } @@ -2839,9 +2876,15 @@ EOD; * * @param string $message the message to print out * @return string HTML fragment. + * @deprecated since Moodle 3.1 MDL-30811 - please do not use this function any more. + * @todo MDL-53113 This will be removed in Moodle 3.5. + * @see \core\output\notification */ public function notify_message($message) { - $n = new \core\output\notification($message, \core\output\notification::NOTIFY_MESSAGE); + debugging(__FUNCTION__ . ' is deprecated.' . + 'Please use notification() or \core\output\notification as required', + DEBUG_DEVELOPER); + $n = new \core\output\notification($message, \core\output\notification::NOTIFY_INFO); return $this->render($n); } @@ -2850,9 +2893,15 @@ EOD; * * @param string $message the message to print out * @return string HTML fragment. + * @deprecated since Moodle 3.1 MDL-30811 - please do not use this function any more. + * @todo MDL-53113 This will be removed in Moodle 3.5. + * @see \core\output\notification */ public function notify_redirect($message) { - $n = new \core\output\notification($message, \core\output\notification::NOTIFY_REDIRECT); + debugging(__FUNCTION__ . ' is deprecated.' . + 'Please use notification() or \core\output\notification as required', + DEBUG_DEVELOPER); + $n = new \core\output\notification($message, \core\output\notification::NOTIFY_INFO); return $this->render($n); } @@ -2864,30 +2913,7 @@ EOD; * @return string the HTML to output. */ protected function render_notification(\core\output\notification $notification) { - - $data = $notification->export_for_template($this); - - $templatename = ''; - switch($data->type) { - case \core\output\notification::NOTIFY_MESSAGE: - $templatename = 'core/notification_message'; - break; - case \core\output\notification::NOTIFY_SUCCESS: - $templatename = 'core/notification_success'; - break; - case \core\output\notification::NOTIFY_PROBLEM: - $templatename = 'core/notification_problem'; - break; - case \core\output\notification::NOTIFY_REDIRECT: - $templatename = 'core/notification_redirect'; - break; - default: - $templatename = 'core/notification_message'; - break; - } - - return self::render_from_template($templatename, $data); - + return $this->render_from_template($notification->get_template_name(), $notification->export_for_template($this)); } /** @@ -4251,13 +4277,13 @@ class core_renderer_cli extends core_renderer { /** * Returns a template fragment representing a notification. * - * @param string $message The message to include - * @param string $classes A space-separated list of CSS classes + * @param string $message The message to print out. + * @param string $type The type of notification. See constants on \core\output\notification. * @return string A template fragment for a notification */ - public function notification($message, $classes = 'notifyproblem') { + public function notification($message, $type = null) { $message = clean_text($message); - if ($classes === 'notifysuccess') { + if ($type === 'notifysuccess' || $type === 'success') { return "++ $message ++\n"; } return "!! $message !!\n"; @@ -4325,10 +4351,10 @@ class core_renderer_ajax extends core_renderer { * Used to display a notification. * For the AJAX notifications are discarded. * - * @param string $message - * @param string $classes + * @param string $message The message to print out. + * @param string $type The type of notification. See constants on \core\output\notification. */ - public function notification($message, $classes = 'notifyproblem') {} + public function notification($message, $type = null) {} /** * Used to display a redirection message. diff --git a/lib/templates/notification_redirect.mustache b/lib/templates/notification_error.mustache similarity index 73% rename from lib/templates/notification_redirect.mustache rename to lib/templates/notification_error.mustache index 4181bd0a4bb..3929ca013c2 100644 --- a/lib/templates/notification_redirect.mustache +++ b/lib/templates/notification_error.mustache @@ -15,11 +15,11 @@ along with Moodle. If not, see . }} {{! - @template core/notification_redirect + @template core/notification_error Moodle notification template. - The purpose of this template is to render a message notification. + The purpose of this template is to render an error notification. Classes required for JS: * none @@ -29,8 +29,11 @@ Context variables required for this template: * message A cleaned string (use clean_text()) to display. + * extraclasses Additional classes to apply to the notification. Example context (json): - { "message": "Your pants are on fire!" } + { "message": "Your pants are on fire!", "extraclasses": "foo bar"} }} -
{{{message}}}
+
+ {{{ message }}} +
diff --git a/lib/templates/notification_problem.mustache b/lib/templates/notification_info.mustache similarity index 73% rename from lib/templates/notification_problem.mustache rename to lib/templates/notification_info.mustache index 67b65167de9..dc42ca26484 100644 --- a/lib/templates/notification_problem.mustache +++ b/lib/templates/notification_info.mustache @@ -15,11 +15,11 @@ along with Moodle. If not, see . }} {{! - @template core/notification_problem + @template core/notification_info Moodle notification template. - The purpose of this template is to render a problem notification. + The purpose of this template is to render an info notification. Classes required for JS: * none @@ -29,8 +29,11 @@ Context variables required for this template: * message A cleaned string (use clean_text()) to display. + * extraclasses Additional classes to apply to the notification. Example context (json): - { "message": "Your pants are on fire!" } + { "message": "Your pants are on fire!", "extraclasses": "foo bar"} }} -
{{{message}}}
+
+ {{{ message }}} +
diff --git a/lib/templates/notification_success.mustache b/lib/templates/notification_success.mustache index 1f806f03d15..ef7aeb09d0f 100644 --- a/lib/templates/notification_success.mustache +++ b/lib/templates/notification_success.mustache @@ -29,8 +29,11 @@ Context variables required for this template: * message A cleaned string (use clean_text()) to display. + * extraclasses Additional classes to apply to the notification. Example context (json): - { "message": "Your pants are on fire!" } + { "message": "Your pants are on fire!", "extraclasses": "foo bar"} }} -
{{{message}}}
+
+ {{{ message }}} +
diff --git a/lib/templates/notification_message.mustache b/lib/templates/notification_warning.mustache similarity index 73% rename from lib/templates/notification_message.mustache rename to lib/templates/notification_warning.mustache index 16d87b74a47..1fe001d3f4f 100644 --- a/lib/templates/notification_message.mustache +++ b/lib/templates/notification_warning.mustache @@ -15,11 +15,11 @@ along with Moodle. If not, see . }} {{! - @template core/notification_message + @template core/notification_warning Moodle notification template. - The purpose of this template is to render a message notification. + The purpose of this template is to render a warning notification. Classes required for JS: * none @@ -29,8 +29,11 @@ Context variables required for this template: * message A cleaned string (use clean_text()) to display. + * extraclasses Additional classes to apply to the notification. Example context (json): - { "message": "Your pants are on fire!" } + { "message": "Your pants are on fire!", "extraclasses": "foo bar"} }} -
{{{message}}}
+
+ {{{ message }}} +
diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 68ac67a37f8..87426fb6924 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -3,6 +3,13 @@ information provided here is intended especially for developers. === 3.1 === +* The specification of extra classes in the $OUTPUT->notification() + function, and \core\output\notification renderable have been deprecated + and will be removed in a future version. + Notifications should use the levels found in \core\output\notification. +* The constants for NOTIFY_PROBLEM, NOTIFY_REDIRECT, and NOTIFY_MESSAGE in + \core\output\notification have been deprecated in favour of NOTIFY_ERROR, + NOTIFY_WARNING, and NOTIFY_INFO respectively. * The following functions, previously used (exclusively) by upgrade steps are not available anymore because of the upgrade cleanup performed for this version. See MDL-51580 for more info: - upgrade_mysql_fix_unsigned_and_lob_columns() diff --git a/theme/base/templates/core/notification_problem.mustache b/theme/base/templates/core/notification_error.mustache similarity index 100% rename from theme/base/templates/core/notification_problem.mustache rename to theme/base/templates/core/notification_error.mustache diff --git a/theme/base/templates/core/notification_redirect.mustache b/theme/base/templates/core/notification_info.mustache similarity index 100% rename from theme/base/templates/core/notification_redirect.mustache rename to theme/base/templates/core/notification_info.mustache diff --git a/theme/base/templates/core/notification_message.mustache b/theme/base/templates/core/notification_warning.mustache similarity index 100% rename from theme/base/templates/core/notification_message.mustache rename to theme/base/templates/core/notification_warning.mustache diff --git a/theme/upgrade.txt b/theme/upgrade.txt index b86f04c16f1..05587d51fff 100644 --- a/theme/upgrade.txt +++ b/theme/upgrade.txt @@ -6,6 +6,11 @@ information provided here is intended especially for theme designer. * A new search box for global search has been added to bootstrap and clean layout files, if your theme is overwriting columns1.php, columns2.php or columns3.php you will need to add a call to core_renderer::search_box to display it. +* Notification templates have been renamed to better suit types of alert + rather than uses. The following changes have been made: + * notification_problem.mustache => notification_error.mustache + * notification_message => notification_info + * notification_redirect => notification_warning === 3.0 === From 0346323cecb7c69fc9e5341f2e2af9d628ed366d Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 16 Feb 2016 08:48:39 +0800 Subject: [PATCH 2/6] MDL-30811 output: Add support for session notifications --- lib/amd/build/notification.min.js | 2 +- lib/amd/src/notification.js | 190 +++++++++++++++++--- lib/classes/notification.php | 165 +++++++++++++++++ lib/classes/output/notification.php | 36 ++++ lib/classes/session/manager.php | 8 + lib/db/services.php | 12 +- lib/external/externallib.php | 54 ++++++ lib/outputrenderers.php | 40 +++-- lib/templates/notification_error.mustache | 9 +- lib/templates/notification_info.mustache | 9 +- lib/templates/notification_success.mustache | 9 +- lib/templates/notification_warning.mustache | 9 +- lib/tests/notification_test.php | 122 +++++++++++++ lib/tests/session_manager_test.php | 4 +- lib/tests/sessionlib_test.php | 6 +- version.php | 2 +- 16 files changed, 622 insertions(+), 55 deletions(-) create mode 100644 lib/classes/notification.php create mode 100644 lib/tests/notification_test.php diff --git a/lib/amd/build/notification.min.js b/lib/amd/build/notification.min.js index 37952ecce65..7df55c238ae 100644 --- a/lib/amd/build/notification.min.js +++ b/lib/amd/build/notification.min.js @@ -1 +1 @@ -define(["core/yui"],function(a){return{alert:function(b,c,d){a.use("moodle-core-notification-alert",function(){var a=new M.core.alert({title:b,message:c,yesLabel:d});a.show()})},confirm:function(b,c,d,e,f){a.use("moodle-core-notification-confirm",function(){var a=new M.core.confirm({title:b,question:c,yesLabel:d,noLabel:e});a.on("complete-yes",function(){f()}),a.show()})},exception:function(b){b.backtrace&&(b.lineNumber=b.backtrace[0].line,b.fileName=b.backtrace[0].file,b.fileName="..."+b.fileName.substr(b.fileName.length-20),b.stack=b.debuginfo,b.name=b.errorcode),a.use("moodle-core-notification-exception",function(){var a=new M.core.exception(b);a.show()})}}}); \ No newline at end of file +define(["core/yui","jquery","theme_bootstrapbase/bootstrap","core/templates","core/ajax","core/log"],function(a,b,c,d,e,f){var g={types:{success:"core/notification_success",info:"core/notification_info",warning:"core/notification_warning",error:"core/notification_error"},fieldName:"user-notifications",fetchNotifications:function(){var a=e.call([{methodname:"core_fetch_notifications",args:{contextid:g.contextid}}]);a[0].done(g.addNotifications)},addNotifications:function(a){a||(a=[]),b.each(a,function(a,b){g.renderNotification(b.template,b.variables)})},setupTargetRegion:function(){var a=b("#"+g.fieldName);if(!a.length){var c=b("").attr("id",g.fieldName);return a=b("#region-main"),a.length?a.prepend(c):(a=b('[role="main"]'),a.length?a.prepend(c):(a=b("body"),a.prepend(c)))}},addNotification:function(a){var c=g.types.error;return a=b.extend({closebutton:!0,announce:!0,type:"error"},a),a.template?(c=a.template,delete a.template):a.type&&("undefined"!=typeof g.types[a.type]&&(c=g.types[a.type]),delete a.type),g.renderNotification(c,a)},renderNotification:function(a,c){return"undefined"!=typeof c.message&&c.message?void d.render(a,c).done(function(a){b("#"+g.fieldName).prepend(a)}).fail(g.exception):void f.debug("Notification received without content. Skipping.")},alert:function(b,c,d){a.use("moodle-core-notification-alert",function(){var a=new M.core.alert({title:b,message:c,yesLabel:d});a.show()})},confirm:function(b,c,d,e,f){a.use("moodle-core-notification-confirm",function(){var a=new M.core.confirm({title:b,question:c,yesLabel:d,noLabel:e});a.on("complete-yes",function(){f()}),a.show()})},exception:function(b){b.backtrace&&(b.lineNumber=b.backtrace[0].line,b.fileName=b.backtrace[0].file,b.fileName="..."+b.fileName.substr(b.fileName.length-20),b.stack=b.debuginfo,b.name=b.errorcode),a.use("moodle-core-notification-exception",function(){var a=new M.core.exception(b);a.show()})}};return{init:function(a,c){g.contextid=a,g.setupTargetRegion(),b().alert(),g.addNotifications(c),g.fetchNotifications()},fetchNotifications:g.fetchNotifications,addNotification:g.addNotification,alert:g.alert,confirm:g.confirm,exception:g.exception}}); \ No newline at end of file diff --git a/lib/amd/src/notification.js b/lib/amd/src/notification.js index 1880b80db06..845a2e63290 100644 --- a/lib/amd/src/notification.js +++ b/lib/amd/src/notification.js @@ -14,6 +14,8 @@ // along with Moodle. If not, see . /** + * A system for displaying notifications to users from the session. + * * Wrapper for the YUI M.core.notification class. Allows us to * use the YUI version in AMD code until it is replaced. * @@ -24,20 +26,99 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * @since 2.9 */ -define(['core/yui'], function(Y) { +define(['core/yui', 'jquery', 'theme_bootstrapbase/bootstrap', 'core/templates', 'core/ajax', 'core/log'], +function(Y, $, bootstrap, templates, ajax, log) { + var notificationModule = { + types: { + 'success': 'core/notification_success', + 'info': 'core/notification_info', + 'warning': 'core/notification_warning', + 'error': 'core/notification_error', + }, - // Private variables and functions. + fieldName: 'user-notifications', + + fetchNotifications: function() { + var promises = ajax.call([{ + methodname: 'core_fetch_notifications', + args: { + contextid: notificationModule.contextid + } + }]); + + promises[0] + .done(notificationModule.addNotifications) + ; + + }, + + addNotifications: function(notifications) { + if (!notifications) { + notifications = []; + } + + $.each(notifications, function(i, notification) { + notificationModule.renderNotification(notification.template, notification.variables); + }); + }, + + setupTargetRegion: function() { + var targetRegion = $('#' + notificationModule.fieldName); + if (targetRegion.length) { + return; + } + + var newRegion = $('').attr('id', notificationModule.fieldName); + + targetRegion = $('#region-main'); + if (targetRegion.length) { + return targetRegion.prepend(newRegion); + } + + targetRegion = $('[role="main"]'); + if (targetRegion.length) { + return targetRegion.prepend(newRegion); + } + + targetRegion = $('body'); + return targetRegion.prepend(newRegion); + }, + + addNotification: function(notification) { + var template = notificationModule.types.error; + + notification = $.extend({ + closebutton: true, + announce: true, + type: 'error' + }, notification); + + if (notification.template) { + template = notification.template; + delete notification.template; + } else if (notification.type){ + if (typeof notificationModule.types[notification.type] !== 'undefined') { + template = notificationModule.types[notification.type]; + } + delete notification.type; + } + + return notificationModule.renderNotification(template, notification); + }, + + renderNotification: function(template, variables) { + if (typeof variables.message === 'undefined' || !variables.message) { + log.debug('Notification received without content. Skipping.'); + return; + } + templates.render(template, variables) + .done(function(html) { + $('#' + notificationModule.fieldName).prepend(html); + }) + .fail(notificationModule.exception) + ; + }, - return /** @alias module:core/notification */ { - // Public variables and functions. - /** - * Wrap M.core.alert. - * - * @method alert - * @param {string} title - * @param {string} message - * @param {string} yesLabel - */ alert: function(title, message, yesLabel) { // Here we are wrapping YUI. This allows us to start transitioning, but // wait for a good alternative without having inconsistent dialogues. @@ -52,16 +133,6 @@ define(['core/yui'], function(Y) { }); }, - /** - * Wrap M.core.confirm. - * - * @method confirm - * @param {string} title - * @param {string} question - * @param {string} yesLabel - * @param {string} noLabel - * @param {function} callback - */ confirm: function(title, question, yesLabel, noLabel, callback) { // Here we are wrapping YUI. This allows us to start transitioning, but // wait for a good alternative without having inconsistent dialogues. @@ -80,12 +151,6 @@ define(['core/yui'], function(Y) { }); }, - /** - * Wrap M.core.exception. - * - * @method exception - * @param {Error} ex - */ exception: function(ex) { // Fudge some parameters. if (ex.backtrace) { @@ -102,4 +167,73 @@ define(['core/yui'], function(Y) { }); } }; + + return /** @alias module:core/notification */{ + init: function(contextid, notifications) { + notificationModule.contextid = contextid; + + // Setup the message target region if it isn't setup already + notificationModule.setupTargetRegion(); + + // Setup closing of bootstrap alerts. + $().alert(); + + // Add provided notifications. + notificationModule.addNotifications(notifications); + + // Poll for any new notifications. + notificationModule.fetchNotifications(); + }, + + /** + * Poll the server for any new notifications. + * + * @method fetchNotifications + */ + fetchNotifications: notificationModule.fetchNotifications, + + /** + * Add a notification to the page. + * + * Note: This does not cause the notification to be added to the session. + * + * @method addNotification + * @param {Object} notification The notification to add. + * @param {string} notification.message The body of the notification + * @param {string} notification.type The type of notification to add (error, warning, info, success). + * @param {Boolean} notification.closebutton Whether to show the close button. + * @param {Boolean} notification.announce Whether to announce to screen readers. + */ + addNotification: notificationModule.addNotification, + + /** + * Wrap M.core.alert. + * + * @method alert + * @param {string} title + * @param {string} message + * @param {string} yesLabel + */ + alert: notificationModule.alert, + + /** + * Wrap M.core.confirm. + * + * @method confirm + * @param {string} title + * @param {string} question + * @param {string} yesLabel + * @param {string} noLabel + * @param {function} callback + */ + confirm: notificationModule.confirm, + + /** + * Wrap M.core.exception. + * + * @method exception + * @param {Error} ex + */ + exception: notificationModule.exception + }; }); diff --git a/lib/classes/notification.php b/lib/classes/notification.php new file mode 100644 index 00000000000..aeb7e0365b2 --- /dev/null +++ b/lib/classes/notification.php @@ -0,0 +1,165 @@ +. + +namespace core; + +/** + * User Alert notifications. + * + * @package core + * @copyright 2016 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +class notification { + /** + * A notification of level 'success'. + */ + const SUCCESS = 'success'; + + /** + * A notification of level 'warning'. + */ + const WARNING = 'warning'; + + /** + * A notification of level 'info'. + */ + const INFO = 'info'; + + /** + * A notification of level 'error'. + */ + const ERROR = 'error'; + + /** + * Add a message to the session notification stack. + * + * @param string $message The message to add to the stack + * @param string $level The type of message to add to the stack + */ + public static function add($message, $level = null) { + global $PAGE, $SESSION; + + if ($PAGE && $PAGE->state === \moodle_page::STATE_IN_BODY) { + // Currently in the page body - just render and exit immediately. + // We insert some code to immediately insert this into the user-notifications created by the header. + $id = uniqid(); + echo \html_writer::span( + $PAGE->get_renderer('core')->render(new \core\output\notification($message, $level)), + '', array('id' => $id)); + + // Insert this JS here using a script directly rather than waiting for the page footer to load to avoid + // ensure that the message is added to the user-notifications section as soon as possible after it is created. + echo \html_writer::script( + "(function() {" . + "var notificationHolder = document.getElementById('user-notifications');" . + "if (!notificationHolder) { return; }" . + "var thisNotification = document.getElementById('{$id}');" . + "if (!thisNotification) { return; }" . + "notificationHolder.appendChild(thisNotification.firstChild);" . + "thisNotification.remove();" . + "})();" + ); + return; + } + + // Add the notification directly to the session. + // This will either be fetched in the header, or by JS in the footer. + $SESSION->notifications[] = (object) array( + 'message' => $message, + 'type' => $level, + ); + } + + /** + * Fetch all of the notifications in the stack and clear the stack. + * + * @return array All of the notifications in the stack + */ + public static function fetch() { + global $SESSION; + + if (!isset($SESSION) || !isset($SESSION->notifications)) { + return []; + } + + $notifications = $SESSION->notifications; + $SESSION->notifications = []; + + $renderables = []; + foreach ($notifications as $notification) { + $renderable = new \core\output\notification($notification->message, $notification->type); + $renderables[] = $renderable; + } + + return $renderables; + } + + /** + * Fetch all of the notifications in the stack and clear the stack. + * + * @return array All of the notifications in the stack + */ + public static function fetch_as_array(\renderer_base $renderer) { + $notifications = []; + foreach (self::fetch() as $notification) { + $notifications[] = [ + 'template' => $notification->get_template_name(), + 'variables' => $notification->export_for_template($renderer), + ]; + } + return $notifications; + } + + /** + * Add a success message to the notification stack. + * + * @param string $message The message to add to the stack + */ + public static function success($message) { + return self::add($message, self::SUCCESS); + } + + /** + * Add a info message to the notification stack. + * + * @param string $message The message to add to the stack + */ + public static function info($message) { + return self::add($message, self::INFO); + } + + /** + * Add a warning message to the notification stack. + * + * @param string $message The message to add to the stack + */ + public static function warning($message) { + return self::add($message, self::WARNING); + } + + /** + * Add a error message to the notification stack. + * + * @param string $message The message to add to the stack + */ + public static function error($message) { + return self::add($message, self::ERROR); + } +} diff --git a/lib/classes/output/notification.php b/lib/classes/output/notification.php index f97af740486..12c9aaf075f 100644 --- a/lib/classes/output/notification.php +++ b/lib/classes/output/notification.php @@ -83,6 +83,16 @@ class notification implements \renderable, \templatable { */ protected $messagetype = self::NOTIFY_WARNING; + /** + * @var bool $announce Whether this notification should be announced assertively to screen readers. + */ + protected $announce = true; + + /** + * @var bool $closebutton Whether this notification should inlcude a button to dismiss itself. + */ + protected $closebutton = true; + /** * @var array $extraclasses A list of any extra classes that may be required. */ @@ -111,6 +121,30 @@ class notification implements \renderable, \templatable { } } + /** + * Set whether this notification should be announced assertively to screen readers. + * + * @param bool $announce + * @return $this + */ + public function set_announce($announce = false) { + $this->announce = (bool) $announce; + + return $this; + } + + /** + * Set whether this notification should include a button to disiss itself. + * + * @param bool $button + * @return $this + */ + public function set_show_closebutton($button = false) { + $this->closebutton = (bool) $button; + + return $this; + } + /** * Add any extra classes that this notification requires. * @@ -133,6 +167,8 @@ class notification implements \renderable, \templatable { return array( 'message' => clean_text($this->message), 'extraclasses' => implode(' ', $this->extraclasses), + 'announce' => $this->announce, + 'closebutton' => $this->closebutton, ); } diff --git a/lib/classes/session/manager.php b/lib/classes/session/manager.php index d565a22cfa4..4dd205ecc2f 100644 --- a/lib/classes/session/manager.php +++ b/lib/classes/session/manager.php @@ -157,10 +157,18 @@ class manager { public static function init_empty_session() { global $CFG; + // Backup notifications. These should be preserved across session changes until the user fetches and clears them. + $notifications = []; + if (isset($GLOBALS['SESSION']->notifications)) { + $notifications = $GLOBALS['SESSION']->notifications; + } $GLOBALS['SESSION'] = new \stdClass(); $GLOBALS['USER'] = new \stdClass(); $GLOBALS['USER']->id = 0; + + // Restore notifications. + $GLOBALS['SESSION']->notifications = $notifications; if (isset($CFG->mnet_localhost_id)) { $GLOBALS['USER']->mnethostid = $CFG->mnet_localhost_id; } else { diff --git a/lib/db/services.php b/lib/db/services.php index 25c193e9586..fd42f3531e5 100644 --- a/lib/db/services.php +++ b/lib/db/services.php @@ -1067,7 +1067,17 @@ $functions = array( 'description' => 'Generic service to update title', 'type' => 'write', 'loginrequired' => true, - 'ajax' => true + 'ajax' => true, + ), + + 'core_fetch_notifications' => array( + 'classname' => 'core_external', + 'methodname' => 'fetch_notifications', + 'classpath' => 'lib/external/externallib.php', + 'description' => 'Return a list of notifications for the current session', + 'type' => 'read', + 'loginrequired' => false, + 'ajax' => true, ), // === Calendar related functions === diff --git a/lib/external/externallib.php b/lib/external/externallib.php index 7c38b776cb7..44b75e12c36 100644 --- a/lib/external/externallib.php +++ b/lib/external/externallib.php @@ -407,4 +407,58 @@ class core_external extends external_api { ) ); } + + /** + * Returns description of fetch_notifications() parameters. + * + * @return external_function_parameters + * @since Moodle 3.1 + */ + public static function fetch_notifications_parameters() { + return new external_function_parameters( + array( + 'contextid' => new external_value(PARAM_INT, 'Context ID', VALUE_REQUIRED), + )); + } + + /** + * Returns description of fetch_notifications() result value. + * + * @return external_description + * @since Moodle 3.1 + */ + public static function fetch_notifications_returns() { + return new external_multiple_structure( + new external_single_structure( + array( + 'template' => new external_value(PARAM_RAW, 'Name of the template'), + 'variables' => new external_single_structure(array( + 'message' => new external_value(PARAM_RAW, 'HTML content of the Notification'), + 'extraclasses' => new external_value(PARAM_RAW, 'Extra classes to provide to the tmeplate'), + 'announce' => new external_value(PARAM_RAW, 'Whether to announce'), + 'closebutton' => new external_value(PARAM_RAW, 'Whether to close'), + )), + ) + ) + ); + } + + /** + * Returns the list of notifications against the current session. + * + * @return array + * @since Moodle 3.1 + */ + public static function fetch_notifications($contextid) { + global $PAGE; + + self::validate_parameters(self::fetch_notifications_parameters(), [ + 'contextid' => $contextid, + ]); + + $context = \context::instance_by_id($contextid); + $PAGE->set_context($context); + + return \core\notification::fetch_as_array($PAGE->get_renderer('core')); + } } diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index 99997c8075b..b2aca169f81 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -1032,7 +1032,7 @@ class core_renderer extends renderer_base { * @return string HTML fragment */ public function footer() { - global $CFG, $DB; + global $CFG, $DB, $PAGE; $output = $this->container_end_all(true); @@ -1057,6 +1057,7 @@ class core_renderer extends renderer_base { } $footer = str_replace($this->unique_performance_info_token, $performanceinfo, $footer); + $this->page->requires->js_call_amd('core/notification', 'init', array($PAGE->context->id, \core\notification::fetch_as_array($this))); $footer = str_replace($this->unique_end_html_token, $this->page->requires->get_end_code(), $footer); $this->page->set_state(moodle_page::STATE_DONE); @@ -1086,22 +1087,37 @@ class core_renderer extends renderer_base { */ public function course_content_header($onlyifnotcalledbefore = false) { global $CFG; - if ($this->page->course->id == SITEID) { - // return immediately and do not include /course/lib.php if not necessary - return ''; - } static $functioncalled = false; if ($functioncalled && $onlyifnotcalledbefore) { // we have already output the content header return ''; } + + // Output any session notification. + $notifications = \core\notification::fetch(); + + $bodynotifications = ''; + foreach ($notifications as $notification) { + $bodynotifications .= $this->render_from_template( + $notification->get_template_name(), + $notification->export_for_template($this) + ); + } + + $output = html_writer::span($bodynotifications, 'notifications', array('id' => 'user-notifications')); + + if ($this->page->course->id == SITEID) { + // return immediately and do not include /course/lib.php if not necessary + return $output; + } + require_once($CFG->dirroot.'/course/lib.php'); $functioncalled = true; $courseformat = course_get_format($this->page->course); if (($obj = $courseformat->course_content_header()) !== null) { - return html_writer::div($courseformat->get_renderer($this->page)->render($obj), 'course-content-header'); + $output .= html_writer::div($courseformat->get_renderer($this->page)->render($obj), 'course-content-header'); } - return ''; + return $output; } /** @@ -2780,6 +2796,8 @@ EOD; /** * Output a notification (that is, a status message about something that has just happened). * + * Note: \core\notification::add() may be more suitable for your usage. + * * @param string $message The message to print out. * @param string $type The type of notification. See constants on \core\output\notification. * @return string the HTML to output. @@ -2848,7 +2866,7 @@ EOD; */ public function notify_problem($message) { debugging(__FUNCTION__ . ' is deprecated.' . - 'Please use notification() or \core\output\notification as required', + 'Please use \core\notification::add, or \core\output\notification as required', DEBUG_DEVELOPER); $n = new \core\output\notification($message, \core\output\notification::NOTIFY_ERROR); return $this->render($n); @@ -2865,7 +2883,7 @@ EOD; */ public function notify_success($message) { debugging(__FUNCTION__ . ' is deprecated.' . - 'Please use notification() or \core\output\notification as required', + 'Please use \core\notification::add, or \core\output\notification as required', DEBUG_DEVELOPER); $n = new \core\output\notification($message, \core\output\notification::NOTIFY_SUCCESS); return $this->render($n); @@ -2882,7 +2900,7 @@ EOD; */ public function notify_message($message) { debugging(__FUNCTION__ . ' is deprecated.' . - 'Please use notification() or \core\output\notification as required', + 'Please use \core\notification::add, or \core\output\notification as required', DEBUG_DEVELOPER); $n = new \core\output\notification($message, \core\output\notification::NOTIFY_INFO); return $this->render($n); @@ -2899,7 +2917,7 @@ EOD; */ public function notify_redirect($message) { debugging(__FUNCTION__ . ' is deprecated.' . - 'Please use notification() or \core\output\notification as required', + 'Please use \core\notification::add, or \core\output\notification as required', DEBUG_DEVELOPER); $n = new \core\output\notification($message, \core\output\notification::NOTIFY_INFO); return $this->render($n); diff --git a/lib/templates/notification_error.mustache b/lib/templates/notification_error.mustache index 3929ca013c2..0e7a8c8448b 100644 --- a/lib/templates/notification_error.mustache +++ b/lib/templates/notification_error.mustache @@ -30,10 +30,15 @@ Context variables required for this template: * message A cleaned string (use clean_text()) to display. * extraclasses Additional classes to apply to the notification. + * closebutton Whether a close button should be displayed to dismiss the message. + * announce Whether the notification should be announced to screen readers. Example context (json): - { "message": "Your pants are on fire!", "extraclasses": "foo bar"} + { "message": "Your pants are on fire!", "closebutton": 1, "announce": 1, "extraclasses": "foo bar"} }} -
+
+ {{# closebutton }}{{/ closebutton }} {{{ message }}}
diff --git a/lib/templates/notification_info.mustache b/lib/templates/notification_info.mustache index dc42ca26484..39cd1511c94 100644 --- a/lib/templates/notification_info.mustache +++ b/lib/templates/notification_info.mustache @@ -30,10 +30,15 @@ Context variables required for this template: * message A cleaned string (use clean_text()) to display. * extraclasses Additional classes to apply to the notification. + * closebutton Whether a close button should be displayed to dismiss the message. + * announce Whether the notification should be announced to screen readers. Example context (json): - { "message": "Your pants are on fire!", "extraclasses": "foo bar"} + { "message": "Your pants are on fire!", "closebutton": 1, "announce": 1, "extraclasses": "foo bar"} }} -
+
+ {{# closebutton }}{{/ closebutton }} {{{ message }}}
diff --git a/lib/templates/notification_success.mustache b/lib/templates/notification_success.mustache index ef7aeb09d0f..65b7e48321f 100644 --- a/lib/templates/notification_success.mustache +++ b/lib/templates/notification_success.mustache @@ -30,10 +30,15 @@ Context variables required for this template: * message A cleaned string (use clean_text()) to display. * extraclasses Additional classes to apply to the notification. + * closebutton Whether a close button should be displayed to dismiss the message. + * announce Whether the notification should be announced to screen readers. Example context (json): - { "message": "Your pants are on fire!", "extraclasses": "foo bar"} + { "message": "Your pants are on fire!", "closebutton": 1, "announce": 1, "extraclasses": "foo bar"} }} -
+
+ {{# closebutton }}{{/ closebutton }} {{{ message }}}
diff --git a/lib/templates/notification_warning.mustache b/lib/templates/notification_warning.mustache index 1fe001d3f4f..b359d830ae5 100644 --- a/lib/templates/notification_warning.mustache +++ b/lib/templates/notification_warning.mustache @@ -30,10 +30,15 @@ Context variables required for this template: * message A cleaned string (use clean_text()) to display. * extraclasses Additional classes to apply to the notification. + * closebutton Whether a close button should be displayed to dismiss the message. + * announce Whether the notification should be announced to screen readers. Example context (json): - { "message": "Your pants are on fire!", "extraclasses": "foo bar"} + { "message": "Your pants are on fire!", "closebutton": 1, "announce": 1, "extraclasses": "foo bar"} }} -
+
+ {{# closebutton }}{{/ closebutton }} {{{ message }}}
diff --git a/lib/tests/notification_test.php b/lib/tests/notification_test.php new file mode 100644 index 00000000000..2cbc131f1bd --- /dev/null +++ b/lib/tests/notification_test.php @@ -0,0 +1,122 @@ +. + +/** + * Unit tests for core\notification. + * + * @package core + * @category phpunit + * @copyright 2016 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Unit tests for core\notification. + * + * @package core + * @category phpunit + * @category phpunit + * @copyright 2016 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_notification_testcase extends advanced_testcase { + + /** + * Setup required for all notification tests. + * + * This includes emptying the list of notifications on the session, resetting any session which exists, and setting + * up a new moodle_page object. + */ + public function setUp() { + global $PAGE, $SESSION; + + parent::setUp(); + $PAGE = new moodle_page(); + \core\session\manager::init_empty_session(); + $SESSION->notifications = []; + } + + /** + * Tear down required for all notification tests. + * + * This includes emptying the list of notifications on the session, resetting any session which exists, and setting + * up a new moodle_page object. + */ + public function tearDown() { + global $PAGE, $SESSION; + + $PAGE = null; + \core\session\manager::init_empty_session(); + $SESSION->notifications = []; + parent::tearDown(); + } + + /** + * Test the way in which notifications are added to the session in different stages of the page load. + */ + public function test_add_during_output_stages() { + global $PAGE, $SESSION; + + \core\notification::add('Example before header', \core\notification::INFO); + $this->assertCount(1, $SESSION->notifications); + + $PAGE->set_state(\moodle_page::STATE_PRINTING_HEADER); + \core\notification::add('Example during header', \core\notification::INFO); + $this->assertCount(2, $SESSION->notifications); + + $PAGE->set_state(\moodle_page::STATE_IN_BODY); + \core\notification::add('Example in body', \core\notification::INFO); + $this->expectOutputRegex('/Example in body/'); + $this->assertCount(2, $SESSION->notifications); + + $PAGE->set_state(\moodle_page::STATE_DONE); + \core\notification::add('Example after page', \core\notification::INFO); + $this->assertCount(3, $SESSION->notifications); + } + + /** + * Test fetching of notifications from the session. + */ + public function test_fetch() { + // Initially there won't be any notifications. + $this->assertCount(0, \core\notification::fetch()); + + // Adding a notification should make one available to fetch. + \core\notification::success('Notification created'); + $this->assertCount(1, \core\notification::fetch()); + $this->assertCount(0, \core\notification::fetch()); + } + + /** + * Test that session notifications are persisted across session clears. + */ + public function test_session_persistance() { + global $PAGE, $SESSION; + + // Initially there won't be any notifications. + $this->assertCount(0, $SESSION->notifications); + + // Adding a notification should make one available to fetch. + \core\notification::success('Notification created'); + $this->assertCount(1, $SESSION->notifications); + + // Re-creating the session will not empty the notification bag. + \core\session\manager::init_empty_session(); + $this->assertCount(1, $SESSION->notifications); + } +} diff --git a/lib/tests/session_manager_test.php b/lib/tests/session_manager_test.php index a5a15d904e2..66a9af13d1c 100644 --- a/lib/tests/session_manager_test.php +++ b/lib/tests/session_manager_test.php @@ -59,7 +59,7 @@ class core_session_manager_testcase extends advanced_testcase { \core\session\manager::init_empty_session(); $this->assertInstanceOf('stdClass', $SESSION); - $this->assertEmpty((array)$SESSION); + $this->assertCount(1, (array)$SESSION); $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); $this->assertSame($GLOBALS['SESSION'], $SESSION); @@ -149,7 +149,7 @@ class core_session_manager_testcase extends advanced_testcase { $this->assertEquals(0, $USER->id); $this->assertInstanceOf('stdClass', $SESSION); - $this->assertEmpty((array)$SESSION); + $this->assertCount(1, (array)$SESSION); $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); $this->assertSame($GLOBALS['SESSION'], $SESSION); diff --git a/lib/tests/sessionlib_test.php b/lib/tests/sessionlib_test.php index 6879ae58aaa..891f61b5e3c 100644 --- a/lib/tests/sessionlib_test.php +++ b/lib/tests/sessionlib_test.php @@ -76,7 +76,7 @@ class core_sessionlib_testcase extends advanced_testcase { $this->assertSame($PAGE->context, context_course::instance($SITE->id)); $this->assertNotSame($adminsession, $SESSION); $this->assertObjectNotHasAttribute('test1', $SESSION); - $this->assertEmpty((array)$SESSION); + $this->assertCount(1, (array)$SESSION); $usersession1 = $SESSION; $SESSION->test2 = true; $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); @@ -99,7 +99,7 @@ class core_sessionlib_testcase extends advanced_testcase { $this->assertSame($PAGE->context, context_course::instance($SITE->id)); $this->assertNotSame($adminsession, $SESSION); $this->assertNotSame($usersession1, $SESSION); - $this->assertEmpty((array)$SESSION); + $this->assertCount(1, (array)$SESSION); $usersession2 = $SESSION; $usersession2->test3 = true; $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); @@ -123,7 +123,7 @@ class core_sessionlib_testcase extends advanced_testcase { $this->assertSame($PAGE->context, context_course::instance($SITE->id)); $this->assertNotSame($adminsession, $SESSION); $this->assertNotSame($usersession1, $SESSION); - $this->assertEmpty((array)$SESSION); + $this->assertCount(1, (array)$SESSION); $this->assertSame($GLOBALS['SESSION'], $_SESSION['SESSION']); $this->assertSame($GLOBALS['SESSION'], $SESSION); $this->assertSame($GLOBALS['USER'], $_SESSION['USER']); diff --git a/version.php b/version.php index 4f06df6ce28..ab95f0598cc 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2016022500.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2016030100.00; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. From 3ad964190cbab3e2bf32f653e283d9cb5063969d Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 16 Feb 2016 08:49:42 +0800 Subject: [PATCH 3/6] MDL-30811 core: Make use of session notifications in redirect() --- lib/outputrenderers.php | 12 +++++++++--- lib/upgrade.txt | 4 ++++ lib/weblib.php | 19 ++++++++++++++----- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index b2aca169f81..b8a655e298d 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -871,10 +871,13 @@ class core_renderer extends renderer_base { * @param boolean $debugdisableredirect this redirect has been disabled for * debugging purposes. Display a message that explains, and don't * trigger the redirect. + * @param string $messagetype The type of notification to show the message in. + * See constants on \core\output\notification. * @return string The HTML to display to the user before dying, may contain * meta refresh, javascript refresh, and may have set header redirects */ - public function redirect_message($encodedurl, $message, $delay, $debugdisableredirect) { + public function redirect_message($encodedurl, $message, $delay, $debugdisableredirect, + $messagetype = \core\output\notification::NOTIFY_INFO) { global $CFG; $url = str_replace('&', '&', $encodedurl); @@ -905,7 +908,7 @@ class core_renderer extends renderer_base { throw new coding_exception('You cannot redirect after the entire page has been generated'); break; } - $output .= $this->notification($message, 'redirectmessage'); + $output .= $this->notification($message, $messagetype); $output .= '
('. get_string('continue') .')
'; if ($debugdisableredirect) { $output .= '

'.get_string('erroroutput', 'error').'

'; @@ -4383,8 +4386,11 @@ class core_renderer_ajax extends core_renderer { * @param string $message * @param int $delay * @param bool $debugdisableredirect + * @param string $messagetype The type of notification to show the message in. + * See constants on \core\output\notification. */ - public function redirect_message($encodedurl, $message, $delay, $debugdisableredirect) {} + public function redirect_message($encodedurl, $message, $delay, $debugdisableredirect, + $messagetype = \core\output\notification::NOTIFY_INFO) {} /** * Prepares the start of an AJAX output. diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 87426fb6924..c6796a445c3 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -3,6 +3,10 @@ information provided here is intended especially for developers. === 3.1 === +* The redirect() function will now redirect immediately if output has not + already started. Messages will be displayed on the subsequent page using + session notifications. The type of message output can be configured using the + fourth parameter to redirect(). * The specification of extra classes in the $OUTPUT->notification() function, and \core\output\notification renderable have been deprecated and will be removed in a future version. diff --git a/lib/weblib.php b/lib/weblib.php index f3ed1ef10b3..79871e2579a 100644 --- a/lib/weblib.php +++ b/lib/weblib.php @@ -2596,9 +2596,10 @@ function notice ($message, $link='', $course=null) { * @param moodle_url|string $url A moodle_url to redirect to. Strings are not to be trusted! * @param string $message The message to display to the user * @param int $delay The delay before redirecting + * @param string $messagetype The type of notification to show the message in. See constants on \core\output\notification. * @throws moodle_exception */ -function redirect($url, $message='', $delay=-1) { +function redirect($url, $message='', $delay=-1, $messagetype = \core\output\notification::NOTIFY_INFO) { global $OUTPUT, $PAGE, $CFG; if (CLI_SCRIPT or AJAX_SCRIPT) { @@ -2696,10 +2697,18 @@ function redirect($url, $message='', $delay=-1) { $url = str_replace('&', '&', $encodedurl); if (!empty($message)) { - if ($delay === -1 || !is_numeric($delay)) { - $delay = 3; + if (!$debugdisableredirect && !headers_sent()) { + // A message has been provided, and the headers have not yet been sent. + // Display the message as a notification on the subsequent page. + \core\notification::add($message, $messagetype); + $message = null; + $delay = 0; + } else { + if ($delay === -1 || !is_numeric($delay)) { + $delay = 3; + } + $message = clean_text($message); } - $message = clean_text($message); } else { $message = get_string('pageshouldredirect'); $delay = 0; @@ -2720,7 +2729,7 @@ function redirect($url, $message='', $delay=-1) { // Include a redirect message, even with a HTTP redirect, because that is recommended practice. if ($PAGE) { $CFG->docroot = false; // To prevent the link to moodle docs from being displayed on redirect page. - echo $OUTPUT->redirect_message($encodedurl, $message, $delay, $debugdisableredirect); + echo $OUTPUT->redirect_message($encodedurl, $message, $delay, $debugdisableredirect, $messagetype); exit; } else { echo bootstrap_renderer::early_redirect_message($encodedurl, $message, $delay); From d954b54f9e3984578089ceb5564b448b8965f2d2 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 16 Feb 2016 09:22:17 +0800 Subject: [PATCH 4/6] MDL-30811 forum: Update use of redirect to include message and type --- mod/forum/index.php | 23 ++++-- mod/forum/maildigest.php | 2 +- mod/forum/post.php | 53 ++++++-------- mod/forum/subscribe.php | 72 ++++++++++++++++--- .../behat/discussion_subscriptions.feature | 42 +++++------ .../tests/behat/forum_subscriptions.feature | 4 +- .../behat/forum_subscriptions_default.feature | 4 +- 7 files changed, 127 insertions(+), 73 deletions(-) diff --git a/mod/forum/index.php b/mod/forum/index.php index 6eb49a092ff..1e9d1fdf622 100644 --- a/mod/forum/index.php +++ b/mod/forum/index.php @@ -174,8 +174,13 @@ foreach ($modinfo->get_instances_of('forum') as $forumid=>$cm) { // Do course wide subscribe/unsubscribe if requested if (!is_null($subscribe)) { if (isguestuser() or !$can_subscribe) { - // there should not be any links leading to this place, just redirect - redirect(new moodle_url('/mod/forum/index.php', array('id' => $id)), get_string('subscribeenrolledonly', 'forum')); + // There should not be any links leading to this place, just redirect. + redirect( + new moodle_url('/mod/forum/index.php', array('id' => $id)), + get_string('subscribeenrolledonly', 'forum'), + null, + \core\output\notification::NOTIFY_ERROR + ); } // Can proceed now, the user is not guest and is enrolled foreach ($modinfo->get_instances_of('forum') as $forumid=>$cm) { @@ -204,9 +209,19 @@ if (!is_null($subscribe)) { $returnto = forum_go_back_to(new moodle_url('/mod/forum/index.php', array('id' => $course->id))); $shortname = format_string($course->shortname, true, array('context' => context_course::instance($course->id))); if ($subscribe) { - redirect($returnto, get_string('nowallsubscribed', 'forum', $shortname), 1); + redirect( + $returnto, + get_string('nowallsubscribed', 'forum', $shortname), + null, + \core\output\notification::NOTIFY_SUCCESS + ); } else { - redirect($returnto, get_string('nowallunsubscribed', 'forum', $shortname), 1); + redirect( + $returnto, + get_string('nowallunsubscribed', 'forum', $shortname), + null, + \core\output\notification::NOTIFY_SUCCESS + ); } } diff --git a/mod/forum/maildigest.php b/mod/forum/maildigest.php index ae8d72d6e11..966442d9f98 100644 --- a/mod/forum/maildigest.php +++ b/mod/forum/maildigest.php @@ -75,4 +75,4 @@ if ($backtoindex) { $returnto = "view.php?f={$id}"; } -redirect($returnto, $updatemessage, 1); +redirect($returnto, $updatemessage, null, \core\output\notification::NOTIFY_SUCCESS); diff --git a/mod/forum/post.php b/mod/forum/post.php index fa6ed461023..653fe365ee8 100644 --- a/mod/forum/post.php +++ b/mod/forum/post.php @@ -740,11 +740,6 @@ if ($mform_post->is_cancelled()) { $DB->update_record("forum", $forum); } - $timemessage = 2; - if (!empty($message)) { // if we're printing stuff about the file upload - $timemessage = 4; - } - if ($realpost->userid == $USER->id) { $message .= '
'.get_string("postupdated", "forum"); } else { @@ -752,9 +747,7 @@ if ($mform_post->is_cancelled()) { $message .= '
'.get_string("editedpostupdated", "forum", fullname($realuser)); } - if ($subscribemessage = forum_post_subscription($fromform, $forum, $discussion)) { - $timemessage = 4; - } + $subscribemessage = forum_post_subscription($fromform, $forum, $discussion); if ($forum->type == 'single') { // Single discussion forums are an exception. We show // the forum itself since it only has one discussion @@ -782,10 +775,12 @@ if ($mform_post->is_cancelled()) { $event->add_record_snapshot('forum_discussions', $discussion); $event->trigger(); - redirect(forum_go_back_to($discussionurl), $message.$subscribemessage, $timemessage); - - exit; - + redirect( + forum_go_back_to($discussionurl), + $message . $subscribemessage, + null, + \core\output\notification::NOTIFY_SUCCESS + ); } else if ($fromform->discussion) { // Adding a new post to an existing discussion // Before we add this we must check that the user will not exceed the blocking threshold. @@ -796,18 +791,10 @@ if ($mform_post->is_cancelled()) { $addpost = $fromform; $addpost->forum=$forum->id; if ($fromform->id = forum_add_new_post($addpost, $mform_post, $message)) { - $timemessage = 2; - if (!empty($message)) { // if we're printing stuff about the file upload - $timemessage = 4; - } - - if ($subscribemessage = forum_post_subscription($fromform, $forum, $discussion)) { - $timemessage = 4; - } + $subscribemessage = forum_post_subscription($fromform, $forum, $discussion); if (!empty($fromform->mailnow)) { $message .= get_string("postmailnow", "forum"); - $timemessage = 4; } else { $message .= '

'.get_string("postaddedsuccess", "forum") . '

'; $message .= '

'.get_string("postaddedtimeleft", "forum", format_time($CFG->maxeditingtime)) . '

'; @@ -843,7 +830,12 @@ if ($mform_post->is_cancelled()) { $completion->update_state($cm,COMPLETION_COMPLETE); } - redirect(forum_go_back_to($discussionurl), $message.$subscribemessage, $timemessage); + redirect( + forum_go_back_to($discussionurl), + $message . $subscribemessage, + null, + \core\output\notification::NOTIFY_SUCCESS + ); } else { print_error("couldnotadd", "forum", $errordestination); @@ -924,22 +916,14 @@ if ($mform_post->is_cancelled()) { $event->add_record_snapshot('forum_discussions', $discussion); $event->trigger(); - $timemessage = 2; - if (!empty($message)) { // If we're printing stuff about the file upload. - $timemessage = 4; - } - if ($fromform->mailnow) { $message .= get_string("postmailnow", "forum"); - $timemessage = 4; } else { $message .= '

'.get_string("postaddedsuccess", "forum") . '

'; $message .= '

'.get_string("postaddedtimeleft", "forum", format_time($CFG->maxeditingtime)) . '

'; } - if ($subscribemessage = forum_post_subscription($fromform, $forum, $discussion)) { - $timemessage = 6; - } + $subscribemessage = forum_post_subscription($fromform, $forum, $discussion); } else { print_error("couldnotadd", "forum", $errordestination); } @@ -953,7 +937,12 @@ if ($mform_post->is_cancelled()) { } // Redirect back to the discussion. - redirect(forum_go_back_to($redirectto->out()), $message . $subscribemessage, $timemessage); + redirect( + forum_go_back_to($redirectto->out()), + $message . $subscribemessage, + null, + \core\output\notification::NOTIFY_SUCCESS + ); } } diff --git a/mod/forum/subscribe.php b/mod/forum/subscribe.php index 7bd84140459..ab209b02644 100644 --- a/mod/forum/subscribe.php +++ b/mod/forum/subscribe.php @@ -99,8 +99,13 @@ if (is_null($mode) and !is_enrolled($context, $USER, '', true)) { // Guests an echo $OUTPUT->footer(); exit; } else { - // there should not be any links leading to this place, just redirect - redirect(new moodle_url('/mod/forum/view.php', array('f'=>$id)), get_string('subscribeenrolledonly', 'forum')); + // There should not be any links leading to this place, just redirect. + redirect( + new moodle_url('/mod/forum/view.php', array('f'=>$id)), + get_string('subscribeenrolledonly', 'forum'), + null, + \core\output\notification::NOTIFY_ERROR + ); } } @@ -117,11 +122,21 @@ if (!is_null($mode) and has_capability('mod/forum:managesubscriptions', $context switch ($mode) { case FORUM_CHOOSESUBSCRIBE : // 0 \mod_forum\subscriptions::set_subscription_mode($forum->id, FORUM_CHOOSESUBSCRIBE); - redirect($returnto, get_string("everyonecannowchoose", "forum"), 1); + redirect( + $returnto, + get_string('everyonecannowchoose', 'forum'), + null, + \core\output\notification::NOTIFY_SUCCESS + ); break; case FORUM_FORCESUBSCRIBE : // 1 \mod_forum\subscriptions::set_subscription_mode($forum->id, FORUM_FORCESUBSCRIBE); - redirect($returnto, get_string("everyoneisnowsubscribed", "forum"), 1); + redirect( + $returnto, + get_string('everyoneisnowsubscribed', 'forum'), + null, + \core\output\notification::NOTIFY_SUCCESS + ); break; case FORUM_INITIALSUBSCRIBE : // 2 if ($forum->forcesubscribe <> FORUM_INITIALSUBSCRIBE) { @@ -131,11 +146,21 @@ if (!is_null($mode) and has_capability('mod/forum:managesubscriptions', $context } } \mod_forum\subscriptions::set_subscription_mode($forum->id, FORUM_INITIALSUBSCRIBE); - redirect($returnto, get_string("everyoneisnowsubscribed", "forum"), 1); + redirect( + $returnto, + get_string('everyoneisnowsubscribed', 'forum'), + null, + \core\output\notification::NOTIFY_SUCCESS + ); break; case FORUM_DISALLOWSUBSCRIBE : // 3 \mod_forum\subscriptions::set_subscription_mode($forum->id, FORUM_DISALLOWSUBSCRIBE); - redirect($returnto, get_string("noonecansubscribenow", "forum"), 1); + redirect( + $returnto, + get_string('noonecansubscribenow', 'forum'), + null, + \core\output\notification::NOTIFY_SUCCESS + ); break; default: print_error(get_string('invalidforcesubscribe', 'forum')); @@ -143,7 +168,12 @@ if (!is_null($mode) and has_capability('mod/forum:managesubscriptions', $context } if (\mod_forum\subscriptions::is_forcesubscribed($forum)) { - redirect($returnto, get_string("everyoneisnowsubscribed", "forum"), 1); + redirect( + $returnto, + get_string('everyoneisnowsubscribed', 'forum'), + null, + \core\output\notification::NOTIFY_SUCCESS + ); } $info = new stdClass(); @@ -174,14 +204,24 @@ if ($issubscribed) { require_sesskey(); if ($discussionid === null) { if (\mod_forum\subscriptions::unsubscribe_user($user->id, $forum, $context, true)) { - redirect($returnto, get_string("nownotsubscribed", "forum", $info), 1); + redirect( + $returnto, + get_string('nownotsubscribed', 'forum', $info), + null, + \core\output\notification::NOTIFY_SUCCESS + ); } else { print_error('cannotunsubscribe', 'forum', get_local_referer(false)); } } else { if (\mod_forum\subscriptions::unsubscribe_user_from_discussion($user->id, $discussion, $context)) { $info->discussion = $discussion->name; - redirect($returnto, get_string("discussionnownotsubscribed", "forum", $info), 1); + redirect( + $returnto, + get_string('discussionnownotsubscribed', 'forum', $info), + null, + \core\output\notification::NOTIFY_SUCCESS + ); } else { print_error('cannotunsubscribe', 'forum', get_local_referer(false)); } @@ -217,10 +257,20 @@ if ($issubscribed) { require_sesskey(); if ($discussionid == null) { \mod_forum\subscriptions::subscribe_user($user->id, $forum, $context, true); - redirect($returnto, get_string("nowsubscribed", "forum", $info), 1); + redirect( + $returnto, + get_string('nowsubscribed', 'forum', $info), + null, + \core\output\notification::NOTIFY_SUCCESS + ); } else { $info->discussion = $discussion->name; \mod_forum\subscriptions::subscribe_user_to_discussion($user->id, $discussion, $context); - redirect($returnto, get_string("discussionnowsubscribed", "forum", $info), 1); + redirect( + $returnto, + get_string('discussionnowsubscribed', 'forum', $info), + null, + \core\output\notification::NOTIFY_SUCCESS + ); } } diff --git a/mod/forum/tests/behat/discussion_subscriptions.feature b/mod/forum/tests/behat/discussion_subscriptions.feature index 760bfcf1a42..e3cb9314ce5 100644 --- a/mod/forum/tests/behat/discussion_subscriptions.feature +++ b/mod/forum/tests/behat/discussion_subscriptions.feature @@ -39,27 +39,27 @@ Feature: A user can control their own subscription preferences for a discussion And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject one" "table_row" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject two" "table_row" And I click on "You are not subscribed to this discussion. Click to subscribe." "link" in the "Test post subject one" "table_row" - And I follow "Continue" + And I should see "Student One will be notified of new posts in 'Test post subject one' of 'Test forum name'" And I should see "Subscribe to this forum" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject one" "table_row" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject two" "table_row" And I click on "You are subscribed to this discussion. Click to unsubscribe." "link" in the "Test post subject one" "table_row" - And I follow "Continue" + And I should see "Student One will NOT be notified of new posts in 'Test post subject one' of 'Test forum name'" And I should see "Subscribe to this forum" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject one" "table_row" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject two" "table_row" And I click on "You are not subscribed to this discussion. Click to subscribe." "link" in the "Test post subject one" "table_row" - And I follow "Continue" + And I should see "Student One will be notified of new posts in 'Test post subject one' of 'Test forum name'" And I should see "Subscribe to this forum" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject one" "table_row" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject two" "table_row" And I follow "Subscribe to this forum" - And I follow "Continue" + And I should see "Student One will be notified of new posts in 'Test forum name'" And I should see "Unsubscribe from this forum" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject one" "table_row" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject two" "table_row" And I follow "Unsubscribe from this forum" - And I follow "Continue" + And I should see "Student One will NOT be notified of new posts in 'Test forum name'" And I should see "Subscribe to this forum" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject one" "table_row" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject two" "table_row" @@ -84,27 +84,27 @@ Feature: A user can control their own subscription preferences for a discussion And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject one" "table_row" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject two" "table_row" And I click on "You are subscribed to this discussion. Click to unsubscribe." "link" in the "Test post subject one" "table_row" - And I follow "Continue" + And I should see "Student One will NOT be notified of new posts in 'Test post subject one' of 'Test forum name'" And I should see "Unsubscribe from this forum" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject one" "table_row" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject two" "table_row" And I click on "You are not subscribed to this discussion. Click to subscribe." "link" in the "Test post subject one" "table_row" - And I follow "Continue" + And I should see "Student One will be notified of new posts in 'Test post subject one' of 'Test forum name'" And I should see "Unsubscribe from this forum" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject one" "table_row" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject two" "table_row" And I click on "You are subscribed to this discussion. Click to unsubscribe." "link" in the "Test post subject one" "table_row" - And I follow "Continue" + And I should see "Student One will NOT be notified of new posts in 'Test post subject one' of 'Test forum name'" And I should see "Unsubscribe from this forum" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject one" "table_row" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject two" "table_row" And I follow "Unsubscribe from this forum" - And I follow "Continue" + And I should see "Student One will NOT be notified of new posts in 'Test forum name'" And I should see "Subscribe to this forum" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject one" "table_row" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject two" "table_row" And I follow "Subscribe to this forum" - And I follow "Continue" + And I should see "Student One will be notified of new posts in 'Test forum name'" And I should see "Unsubscribe from this forum" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject one" "table_row" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject two" "table_row" @@ -129,7 +129,7 @@ Feature: A user can control their own subscription preferences for a discussion And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject one" "table_row" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject two" "table_row" And I click on "You are not subscribed to this discussion. Click to subscribe." "link" in the "Test post subject one" "table_row" - And I follow "Continue" + And I should see "Student One will be notified of new posts in 'Test post subject one' of 'Test forum name'" And I should see "Subscribe to this forum" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject one" "table_row" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject two" "table_row" @@ -150,8 +150,8 @@ Feature: A user can control their own subscription preferences for a discussion And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject one" "table_row" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject two" "table_row" When I follow "Unsubscribe from this forum" - And I follow "Continue" - Then I should see "Subscribe to this forum" + Then I should see "Student One will NOT be notified of new posts in 'Test forum name'" + And I should see "Subscribe to this forum" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject one" "table_row" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject two" "table_row" @@ -175,7 +175,7 @@ Feature: A user can control their own subscription preferences for a discussion And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject one" "table_row" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject two" "table_row" And I click on "You are not subscribed to this discussion. Click to subscribe." "link" in the "Test post subject one" "table_row" - And I follow "Continue" + And I should see "Student One will be notified of new posts in 'Test post subject one' of 'Test forum name'" And I should see "Subscribe to this forum" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject one" "table_row" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject two" "table_row" @@ -196,7 +196,7 @@ Feature: A user can control their own subscription preferences for a discussion And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject one" "table_row" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject two" "table_row" When I follow "Unsubscribe from this forum" - And I follow "Continue" + And I should see "Student One will NOT be notified of new posts in 'Test forum name'" Then I should see "Subscribe to this forum" And "You are subscribed to this discussion. Click to unsubscribe." "link" should exist in the "Test post subject one" "table_row" And "You are not subscribed to this discussion. Click to subscribe." "link" should exist in the "Test post subject two" "table_row" @@ -325,31 +325,31 @@ Feature: A user can control their own subscription preferences for a discussion Then I should see "Subscribe to this forum" And I should see "Subscribe to this discussion" And I follow "Subscribe to this forum" - And I follow "Continue" + And I should see "Student One will be notified of new posts in 'Test forum name'" And I follow "Test post subject one" And I should see "Unsubscribe from this forum" And I should see "Unsubscribe from this discussion" And I follow "Unsubscribe from this discussion" - And I follow "Continue" + And I should see "Student One will NOT be notified of new posts in 'Test post subject one' of 'Test forum name'" And I follow "Test post subject one" And I should see "Unsubscribe from this forum" And I should see "Subscribe to this discussion" And I follow "Unsubscribe from this forum" - And I follow "Continue" + And I should see "Student One will NOT be notified of new posts in 'Test forum name'" And I follow "Test post subject one" And I should see "Subscribe to this forum" And I should see "Subscribe to this discussion" And I follow "Subscribe to this discussion" - And I follow "Continue" + And I should see "Student One will be notified of new posts in 'Test post subject one' of 'Test forum name'" And I should see "Subscribe to this forum" And I should see "Unsubscribe from this discussion" And I follow "Subscribe to this forum" - And I follow "Continue" + And I should see "Student One will be notified of new posts in 'Test forum name'" And I follow "Test post subject one" And I should see "Unsubscribe from this forum" And I should see "Unsubscribe from this discussion" And I follow "Unsubscribe from this forum" - And I follow "Continue" + And I should see "Student One will NOT be notified of new posts in 'Test forum name'" And I follow "Test post subject one" And I should see "Subscribe to this forum" And I should see "Subscribe to this discussion" diff --git a/mod/forum/tests/behat/forum_subscriptions.feature b/mod/forum/tests/behat/forum_subscriptions.feature index acab5cdc043..f1551183905 100644 --- a/mod/forum/tests/behat/forum_subscriptions.feature +++ b/mod/forum/tests/behat/forum_subscriptions.feature @@ -71,7 +71,7 @@ Feature: A user can control their own subscription preferences for a forum Then I should see "Subscribe to this forum" And I should not see "Unsubscribe from this forum" And I follow "Subscribe to this forum" - And I follow "Continue" + And I should see "Student One will be notified of new posts in 'Test forum name'" And I should see "Unsubscribe from this forum" And I should not see "Subscribe to this forum" @@ -91,6 +91,6 @@ Feature: A user can control their own subscription preferences for a forum Then I should see "Unsubscribe from this forum" And I should not see "Subscribe to this forum" And I follow "Unsubscribe from this forum" - And I follow "Continue" + And I should see "Student One will NOT be notified of new posts in 'Test forum name'" And I should see "Subscribe to this forum" And I should not see "Unsubscribe from this forum" diff --git a/mod/forum/tests/behat/forum_subscriptions_default.feature b/mod/forum/tests/behat/forum_subscriptions_default.feature index d5da6b6c10f..e40e7b2d683 100644 --- a/mod/forum/tests/behat/forum_subscriptions_default.feature +++ b/mod/forum/tests/behat/forum_subscriptions_default.feature @@ -121,7 +121,7 @@ Feature: A user can control their default discussion subscription settings And I follow "Course 1" And I follow "Test forum name" And I click on "You are subscribed to this discussion. Click to unsubscribe." "link" in the "Test post subject" "table_row" - And I follow "Continue" + And I should see "Student One will NOT be notified of new posts in 'Test post subject' of 'Test forum name'" And I follow "Test post subject" When I follow "Reply" And "input[name=discussionsubscribe][checked=checked]" "css_element" should exist @@ -130,7 +130,7 @@ Feature: A user can control their default discussion subscription settings And I follow "Course 1" And I follow "Test forum name" And I click on "You are subscribed to this discussion. Click to unsubscribe." "link" in the "Test post subject" "table_row" - And I follow "Continue" + And I should see "Student Two will NOT be notified of new posts in 'Test post subject' of 'Test forum name'" And I follow "Test post subject" And I follow "Reply" And "input[name=discussionsubscribe]:not([checked=checked])" "css_element" should exist From 372d6b923b6091d9af312daa3b0cb4a9049ea118 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 16 Feb 2016 08:52:22 +0800 Subject: [PATCH 5/6] MDL-30811 core: Replace use of continue_button with redirect --- admin/tool/task/scheduledtasks.php | 20 ++------------------ admin/user/user_bulk_cohortadd.php | 7 +------ badges/action.php | 3 +-- lib/weblib.php | 6 +++++- mod/imscp/view.php | 10 ++++------ mod/quiz/report/overview/report.php | 6 +----- user/emailupdate.php | 27 +++++++++++++-------------- user/view.php | 16 ++++++++-------- 8 files changed, 35 insertions(+), 60 deletions(-) diff --git a/admin/tool/task/scheduledtasks.php b/admin/tool/task/scheduledtasks.php index 4b6f3441eb8..ed6e53c0a66 100644 --- a/admin/tool/task/scheduledtasks.php +++ b/admin/tool/task/scheduledtasks.php @@ -87,13 +87,9 @@ if ($mform && ($mform->is_cancelled() || !empty($CFG->preventscheduledtaskchange try { \core\task\manager::configure_scheduled_task($task); - $url = $PAGE->url; - $url->params(array('success'=>get_string('changessaved'))); - redirect($url); + redirect($PAGE->url, get_string('changessaved'), null, \core\output\notification::NOTIFY_SUCCESS); } catch (Exception $e) { - $url = $PAGE->url; - $url->params(array('error'=>$e->getMessage())); - redirect($url); + redirect($PAGE->url, $e->getMessage(), null, \core\output\notification::NOTIFY_ERROR); } } else { echo $OUTPUT->header(); @@ -104,19 +100,7 @@ if ($mform && ($mform->is_cancelled() || !empty($CFG->preventscheduledtaskchange } else { echo $OUTPUT->header(); - $error = optional_param('error', '', PARAM_NOTAGS); - if ($error) { - echo $OUTPUT->notification($error, 'notifyerror'); - } - $success = optional_param('success', '', PARAM_NOTAGS); - if ($success) { - echo $OUTPUT->notification($success, 'notifysuccess'); - } $tasks = core\task\manager::get_all_scheduled_tasks(); echo $renderer->scheduled_tasks_table($tasks); echo $OUTPUT->footer(); } - - - - diff --git a/admin/user/user_bulk_cohortadd.php b/admin/user/user_bulk_cohortadd.php index 958d6c1ecc8..7e2d4c53365 100644 --- a/admin/user/user_bulk_cohortadd.php +++ b/admin/user/user_bulk_cohortadd.php @@ -60,12 +60,7 @@ foreach ($allcohorts as $c) { unset($allcohorts); if (count($cohorts) < 2) { - echo $OUTPUT->header(); - echo $OUTPUT->heading(get_string('bulkadd', 'core_cohort')); - echo $OUTPUT->notification(get_string('bulknocohort', 'core_cohort')); - echo $OUTPUT->continue_button(new moodle_url('/admin/user/user_bulk.php')); - echo $OUTPUT->footer(); - die; + redirect(new moodle_url('/admin/user/user_bulk.php'), get_string('bulknocohort', 'core_cohort')); } $countries = get_string_manager()->get_list_of_countries(true); diff --git a/badges/action.php b/badges/action.php index 31843a20880..db431720bb6 100644 --- a/badges/action.php +++ b/badges/action.php @@ -115,8 +115,7 @@ if ($activate) { $url = new moodle_url('/badges/action.php', $params); if (!$badge->has_criteria()) { - echo $OUTPUT->notification(get_string('error:cannotact', 'badges') . get_string('nocriteria', 'badges')); - echo $OUTPUT->continue_button($returnurl); + redirect($returnurl, get_string('error:cannotact', 'badges') . get_string('nocriteria', 'badges'), null, \core\output\notification::NOTIFY_ERROR); } else { $message = get_string('reviewconfirm', 'badges', $badge->name); echo $OUTPUT->confirm($message, $url, $returnurl); diff --git a/lib/weblib.php b/lib/weblib.php index 79871e2579a..18ce243e1ca 100644 --- a/lib/weblib.php +++ b/lib/weblib.php @@ -2599,7 +2599,7 @@ function notice ($message, $link='', $course=null) { * @param string $messagetype The type of notification to show the message in. See constants on \core\output\notification. * @throws moodle_exception */ -function redirect($url, $message='', $delay=-1, $messagetype = \core\output\notification::NOTIFY_INFO) { +function redirect($url, $message='', $delay=null, $messagetype = \core\output\notification::NOTIFY_INFO) { global $OUTPUT, $PAGE, $CFG; if (CLI_SCRIPT or AJAX_SCRIPT) { @@ -2607,6 +2607,10 @@ function redirect($url, $message='', $delay=-1, $messagetype = \core\output\noti throw new moodle_exception('redirecterrordetected', 'error'); } + if ($delay === null) { + $delay = -1; + } + // Prevent debug errors - make sure context is properly initialised. if ($PAGE) { $PAGE->set_context(null); diff --git a/mod/imscp/view.php b/mod/imscp/view.php index e3c2867726d..f47f6cc095a 100644 --- a/mod/imscp/view.php +++ b/mod/imscp/view.php @@ -62,17 +62,15 @@ $PAGE->requires->string_for_js('show', 'moodle'); $PAGE->set_title($course->shortname.': '.$imscp->name); $PAGE->set_heading($course->fullname); $PAGE->set_activity_record($imscp); -echo $OUTPUT->header(); -echo $OUTPUT->heading(format_string($imscp->name)); // Verify imsmanifest was parsed properly. if (!$imscp->structure) { - echo $OUTPUT->notification(get_string('deploymenterror', 'imscp'), 'notifyproblem'); - echo $OUTPUT->continue_button(course_get_url($course->id, $cm->section)); - echo $OUTPUT->footer(); - die; + redirect(course_get_url($course->id, $cm->section), get_string('deploymenterror', 'imscp')); } +echo $OUTPUT->header(); +echo $OUTPUT->heading(format_string($imscp->name)); + imscp_print_content($imscp, $cm, $course); echo $OUTPUT->footer(); diff --git a/mod/quiz/report/overview/report.php b/mod/quiz/report/overview/report.php index 5cb1145d55c..646073ab5ff 100644 --- a/mod/quiz/report/overview/report.php +++ b/mod/quiz/report/overview/report.php @@ -302,11 +302,7 @@ class quiz_overview_report extends quiz_attempts_report { * @uses exit. This method never returns. */ protected function finish_regrade($nexturl) { - global $OUTPUT, $PAGE; - echo $OUTPUT->heading(get_string('regradecomplete', 'quiz_overview'), 3); - echo $OUTPUT->continue_button($nexturl); - echo $OUTPUT->footer(); - die(); + redirect($nexturl, get_string('regradecomplete', 'quiz_overview'), null, \core\output\notification::NOTIFY_SUCCESS); } /** diff --git a/user/emailupdate.php b/user/emailupdate.php index 7adb835888f..bfc3945c20a 100644 --- a/user/emailupdate.php +++ b/user/emailupdate.php @@ -45,16 +45,15 @@ $stremailupdate = get_string('emailupdate', 'auth', $a); $PAGE->set_title(format_string($SITE->fullname) . ": $stremailupdate"); $PAGE->set_heading(format_string($SITE->fullname) . ": $stremailupdate"); -echo $OUTPUT->header(); - if (empty($preferences['newemailattemptsleft'])) { redirect("$CFG->wwwroot/user/view.php?id=$user->id"); } else if ($preferences['newemailattemptsleft'] < 1) { cancel_email_update($user->id); - $stroutofattempts = get_string('auth_outofnewemailupdateattempts', 'auth'); - echo $OUTPUT->box($stroutofattempts, 'center'); + echo $OUTPUT->header(); + echo $OUTPUT->box(get_string('auth_outofnewemailupdateattempts', 'auth'), 'center'); + echo $OUTPUT->footer(); } else if ($key == $preferences['newemailkey']) { $olduser = clone($user); cancel_email_update($user->id); @@ -62,25 +61,25 @@ if (empty($preferences['newemailattemptsleft'])) { // Detect duplicate before saving. if ($DB->get_record('user', array('email' => $user->email))) { - $stremailnowexists = get_string('emailnowexists', 'auth'); - echo $OUTPUT->box($stremailnowexists, 'center'); - echo $OUTPUT->continue_button("$CFG->wwwroot/user/view.php?id=$user->id"); + redirect(new moodle_url('/user/view.php', ['id' => $user->id]), get_string('emailnowexists', 'auth')); } else { // Update user email. $authplugin = get_auth_plugin($user->auth); $authplugin->user_update($olduser, $user); user_update_user($user, false); $a->email = $user->email; - $stremailupdatesuccess = get_string('emailupdatesuccess', 'auth', $a); - echo $OUTPUT->box($stremailupdatesuccess, 'center'); - echo $OUTPUT->continue_button("$CFG->wwwroot/user/view.php?id=$user->id"); + redirect( + new moodle_url('/user/view.php', ['id' => $user->id]), + get_string('emailupdatesuccess', 'auth', $a), + null, + \core\output\notification::NOTIFY_SUCCESS + ); } } else { $preferences['newemailattemptsleft']--; set_user_preference('newemailattemptsleft', $preferences['newemailattemptsleft'], $user->id); - $strinvalidkey = get_string('auth_invalidnewemailkey', 'auth'); - echo $OUTPUT->box($strinvalidkey, 'center'); + echo $OUTPUT->header(); + echo $OUTPUT->box(get_string('auth_invalidnewemailkey', 'auth'), 'center'); + echo $OUTPUT->footer(); } - -echo $OUTPUT->footer(); diff --git a/user/view.php b/user/view.php index 8fbca1412e5..0829ca8890f 100644 --- a/user/view.php +++ b/user/view.php @@ -110,12 +110,12 @@ $fullname = fullname($user, has_capability('moodle/site:viewfullnames', $coursec if ($currentuser) { if (!is_viewing($coursecontext) && !is_enrolled($coursecontext)) { // Need to have full access to a course to see the rest of own info. - echo $OUTPUT->header(); - echo $OUTPUT->heading(get_string('notenrolled', '', $fullname)); $referer = get_local_referer(false); if (!empty($referer)) { - echo $OUTPUT->continue_button($referer); + redirect($referer, get_string('notenrolled', '', $fullname)); } + echo $OUTPUT->header(); + echo $OUTPUT->heading(get_string('notenrolled', '', $fullname)); echo $OUTPUT->footer(); die; } @@ -136,17 +136,17 @@ if ($currentuser) { // or test for course:inspect capability. if (has_capability('moodle/role:assign', $coursecontext)) { $PAGE->navbar->add($fullname); - echo $OUTPUT->header(); - echo $OUTPUT->heading(get_string('notenrolled', '', $fullname)); + $notice = get_string('notenrolled', '', $fullname); } else { - echo $OUTPUT->header(); $PAGE->navbar->add($struser); - echo $OUTPUT->heading(get_string('notenrolledprofile')); + $notice = get_string('notenrolledprofile', '', $fullname); } $referer = get_local_referer(false); if (!empty($referer)) { - echo $OUTPUT->continue_button($referer); + redirect($referer, $notice); } + echo $OUTPUT->header(); + echo $OUTPUT->heading($notice); echo $OUTPUT->footer(); exit; } From 0e8ce68a179e6ff7f84b2af036b169ba309fcba5 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 16 Feb 2016 09:27:59 +0800 Subject: [PATCH 6/6] MDL-30811 core: Fix tests where continue button has been removed --- my/tests/behat/reset_all_pages.feature | 4 ++-- tag/tests/behat/flag_tags.feature | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/my/tests/behat/reset_all_pages.feature b/my/tests/behat/reset_all_pages.feature index 3e9664e8914..d995f51aad9 100644 --- a/my/tests/behat/reset_all_pages.feature +++ b/my/tests/behat/reset_all_pages.feature @@ -66,7 +66,7 @@ Feature: Reset all personalised pages to default And I log in as "admin" And I navigate to "Default Dashboard page" node in "Site administration > Appearance" When I press "Reset Dashboard for all users" - And I follow "Continue" + And I should see "All Dashboard pages have been reset to default." And I log out And I log in as "student1" @@ -108,7 +108,7 @@ Feature: Reset all personalised pages to default And I log in as "admin" And I navigate to "Default profile page" node in "Site administration > Appearance" When I press "Reset profile for all users" - And I follow "Continue" + And I should see "All profile pages have been reset to default." And I log out And I log in as "student2" diff --git a/tag/tests/behat/flag_tags.feature b/tag/tests/behat/flag_tags.feature index 966abee7208..74231bedd92 100644 --- a/tag/tests/behat/flag_tags.feature +++ b/tag/tests/behat/flag_tags.feature @@ -29,13 +29,11 @@ Feature: Users can flag tags and manager can reset flags And I follow "Badtag" And I follow "Flag as inappropriate" And I should see "The person responsible will be notified" - And I follow "Continue" And I navigate to "Participants" node in "Site pages" And I follow "User 1" And I follow "Sweartag" And I follow "Flag as inappropriate" And I should see "The person responsible will be notified" - And I follow "Continue" And I log out And I log in as "user3" And I navigate to "Participants" node in "Site pages" @@ -43,7 +41,6 @@ Feature: Users can flag tags and manager can reset flags And I follow "Sweartag" And I follow "Flag as inappropriate" And I should see "The person responsible will be notified" - And I follow "Continue" And I log out Scenario: Managing tag flags with javascript disabled