From c4701daa75c9867d0e3afa16fd5d245a1480d6ee Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Sun, 26 Oct 2014 22:52:08 -0700 Subject: [PATCH] MDL-47758 tool_monitor: prevent DB error when revisiting a delete URL --- .../classes/output/managerules/renderable.php | 7 ++--- .../monitor/classes/output/managesubs/subs.php | 9 +++---- admin/tool/monitor/index.php | 31 +++++++++++++++++++--- admin/tool/monitor/managerules.php | 30 +++++++++++++++++---- 4 files changed, 57 insertions(+), 20 deletions(-) diff --git a/admin/tool/monitor/classes/output/managerules/renderable.php b/admin/tool/monitor/classes/output/managerules/renderable.php index c42f83724f7..0d405e1cd6e 100644 --- a/admin/tool/monitor/classes/output/managerules/renderable.php +++ b/admin/tool/monitor/classes/output/managerules/renderable.php @@ -171,11 +171,8 @@ class renderable extends \table_sql implements \renderable { $icon = $OUTPUT->render(new \pix_icon('t/copy', get_string('duplicaterule', 'tool_monitor'))); $manage .= \html_writer::link($copyurl, $icon, array('class' => 'action-icon')); - $a = $rule->get_name($this->context); - $action = new \component_action('click', 'M.util.show_confirm_dialog', array('message' => get_string('ruleareyousure', - 'tool_monitor', $a))); - $icon = $OUTPUT->action_link($deleteurl, new \pix_icon('t/delete', get_string('deleterule', 'tool_monitor')), $action); - $manage .= $icon; + $icon = $OUTPUT->render(new \pix_icon('t/delete', get_string('deleterule', 'tool_monitor'))); + $manage .= \html_writer::link($deleteurl, $icon, array('class' => 'action-icon')); } else { $manage = get_string('nopermission', 'tool_monitor'); } diff --git a/admin/tool/monitor/classes/output/managesubs/subs.php b/admin/tool/monitor/classes/output/managesubs/subs.php index 8ba616d52d4..11c123137e7 100644 --- a/admin/tool/monitor/classes/output/managesubs/subs.php +++ b/admin/tool/monitor/classes/output/managesubs/subs.php @@ -135,14 +135,11 @@ class subs extends \table_sql implements \renderable { public function col_unsubscribe(\tool_monitor\subscription $sub) { global $OUTPUT, $CFG; - $a = $sub->get_name($this->context); $deleteurl = new \moodle_url($CFG->wwwroot. '/admin/tool/monitor/index.php', array('subscriptionid' => $sub->id, 'action' => 'unsubscribe', 'courseid' => $this->courseid, 'sesskey' => sesskey())); - $action = new \component_action('click', 'M.util.show_confirm_dialog', array('message' => get_string('subareyousure', - 'tool_monitor', $a))); - $icon = $OUTPUT->action_link($deleteurl, - new \pix_icon('t/delete', get_string('deletesubscription', 'tool_monitor')), $action); - return $icon; + $icon = $OUTPUT->render(new \pix_icon('t/delete', get_string('deletesubscription', 'tool_monitor'))); + + return \html_writer::link($deleteurl, $icon, array('class' => 'action-icon')); } /** diff --git a/admin/tool/monitor/index.php b/admin/tool/monitor/index.php index 3a02a62c49e..2983eade182 100644 --- a/admin/tool/monitor/index.php +++ b/admin/tool/monitor/index.php @@ -30,6 +30,7 @@ $action = optional_param('action', '', PARAM_ALPHA); $cmid = optional_param('cmid', 0, PARAM_INT); $ruleid = optional_param('ruleid', 0, PARAM_INT); $subscriptionid = optional_param('subscriptionid', 0, PARAM_INT); +$confirm = optional_param('confirm', false, PARAM_BOOL); // Validate course id. if (empty($courseid)) { @@ -61,8 +62,6 @@ $PAGE->set_pagelayout('report'); $PAGE->set_title($title); $PAGE->set_heading($title); -echo $OUTPUT->header(); - // Create/delete subscription if needed. if (!empty($action)) { require_sesskey(); @@ -70,14 +69,38 @@ if (!empty($action)) { case 'subscribe' : $rule = \tool_monitor\rule_manager::get_rule($ruleid); $rule->subscribe_user($courseid, $cmid); + echo $OUTPUT->header(); echo $OUTPUT->notification(get_string('subcreatesuccess', 'tool_monitor'), 'notifysuccess'); break; case 'unsubscribe' : - \tool_monitor\subscription_manager::delete_subscription($subscriptionid); - echo $OUTPUT->notification(get_string('subdeletesuccess', 'tool_monitor'), 'notifysuccess'); + // If the subscription does not exist, then redirect back as the subscription must have already been deleted. + if (!$subscription = $DB->record_exists('tool_monitor_subscriptions', array('id' => $subscriptionid))) { + redirect(new moodle_url('/admin/tool/monitor/index.php', array('courseid' => $courseid))); + } + + // Set the URLs. + $confirmurl = new moodle_url('/admin/tool/monitor/index.php', array('subscriptionid' => $subscriptionid, + 'courseid' => $courseid, 'action' => 'unsubscribe', 'confirm' => true, + 'sesskey' => sesskey())); + $cancelurl = new moodle_url('/admin/tool/monitor/index.php', array('subscriptionid' => $subscriptionid, + 'courseid' => $courseid, 'sesskey' => sesskey())); + if ($confirm) { + \tool_monitor\subscription_manager::delete_subscription($subscriptionid); + echo $OUTPUT->header(); + echo $OUTPUT->notification(get_string('subdeletesuccess', 'tool_monitor'), 'notifysuccess'); + } else { + $subscription = \tool_monitor\subscription_manager::get_subscription($subscriptionid); + echo $OUTPUT->header(); + echo $OUTPUT->confirm(get_string('subareyousure', 'tool_monitor', $subscription->get_name($context)), + $confirmurl, $cancelurl); + echo $OUTPUT->footer(); + exit(); + } break; default: } +} else { + echo $OUTPUT->header(); } // Render the current subscriptions list. diff --git a/admin/tool/monitor/managerules.php b/admin/tool/monitor/managerules.php index bdafd03da76..d86a308e0f6 100644 --- a/admin/tool/monitor/managerules.php +++ b/admin/tool/monitor/managerules.php @@ -28,6 +28,7 @@ require_once($CFG->libdir.'/adminlib.php'); $courseid = optional_param('courseid', 0, PARAM_INT); $ruleid = optional_param('ruleid', 0, PARAM_INT); $action = optional_param('action', '', PARAM_ALPHA); +$confirm = optional_param('confirm', false, PARAM_BOOL); // Validate course id. if (empty($courseid)) { @@ -62,12 +63,17 @@ if (empty($courseid)) { admin_externalpage_setup('toolmonitorrules', '', null, '', array('pagelayout' => 'report')); } -echo $OUTPUT->header(); - // Copy/delete rule if needed. if (!empty($action) && $ruleid) { require_sesskey(); - $rule = \tool_monitor\rule_manager::get_rule($ruleid); + + // If the rule does not exist, then redirect back as the rule must have already been deleted. + if (!$rule = $DB->get_record('tool_monitor_rules', array('id' => $ruleid), '*', IGNORE_MISSING)) { + redirect(new moodle_url('/admin/tool/monitor/managerules.php', array('courseid' => $courseid))); + } + + echo $OUTPUT->header(); + $rule = \tool_monitor\rule_manager::get_rule($rule); if ($rule->can_manage_rule()) { switch ($action) { case 'copy' : @@ -75,8 +81,20 @@ if (!empty($action) && $ruleid) { echo $OUTPUT->notification(get_string('rulecopysuccess', 'tool_monitor'), 'notifysuccess'); break; case 'delete' : - $rule->delete_rule(); - echo $OUTPUT->notification(get_string('ruledeletesuccess', 'tool_monitor'), 'notifysuccess'); + $confirmurl = new moodle_url($CFG->wwwroot. '/admin/tool/monitor/managerules.php', + array('ruleid' => $ruleid, 'courseid' => $courseid, 'action' => 'delete', + 'confirm' => true, 'sesskey' => sesskey())); + $cancelurl = new moodle_url($CFG->wwwroot. '/admin/tool/monitor/managerules.php', + array('courseid' => $courseid)); + if ($confirm) { + $rule->delete_rule(); + echo $OUTPUT->notification(get_string('ruledeletesuccess', 'tool_monitor'), 'notifysuccess'); + } else { + echo $OUTPUT->confirm(get_string('ruleareyousure', 'tool_monitor', $rule->get_name($context)), + $confirmurl, $cancelurl); + echo $OUTPUT->footer(); + exit(); + } break; default: } @@ -84,6 +102,8 @@ if (!empty($action) && $ruleid) { // User doesn't have permissions. Should never happen for real users. throw new moodle_exception('rulenopermissions', 'tool_monitor', $manageurl, $action); } +} else { + echo $OUTPUT->header(); } // Render the rule list. -- 2.11.4.GIT