From 195d4b5d620f82620fde7ebd709b7cf167968257 Mon Sep 17 00:00:00 2001
From: Justus Dieckmann <justusdieckmann@wwu.de>
Date: Wed, 25 Sep 2019 20:04:16 +0200
Subject: [PATCH] Delays: Code cleanup, return cached data when
mform->get_data() would return null, only delete delays for selected
workflow/globally
---
classes/form/form_delays_filter.php | 36 +++++++++++++++-
classes/table/delayed_courses_table.php | 57 +++++++++++++------------
db/caches.php | 5 +--
delayedcourses.php | 2 +-
4 files changed, 67 insertions(+), 33 deletions(-)
diff --git a/classes/form/form_delays_filter.php b/classes/form/form_delays_filter.php
index 662cb5a..529d04c 100644
--- a/classes/form/form_delays_filter.php
+++ b/classes/form/form_delays_filter.php
@@ -23,6 +23,7 @@
*/
namespace tool_lifecycle\form;
+use cache;
use tool_lifecycle\action;
use tool_lifecycle\entity\step_subplugin;
use tool_lifecycle\entity\workflow;
@@ -43,6 +44,11 @@ require_once($CFG->libdir . '/formslib.php');
*/
class form_delays_filter extends \moodleform {
+ /**
+ * @var $cache cache the cache to get stored data from;
+ */
+ private $cache;
+
/**
* Constructor
*
@@ -82,8 +88,12 @@ class form_delays_filter extends \moodleform {
$this->add_action_buttons(true, get_string('apply', 'tool_lifecycle'));
}
+ /**
+ * This method is called after definition(), data submission and set_data().
+ * All form setup that is dependent on form values should go in here.
+ */
public function definition_after_data() {
- $cache = \cache::make('tool_lifecycle', 'mformdata');
+ $cache = $this->get_cache();
if ($this->is_submitted() && $this->is_validated()) {
if ($this->is_cancelled()) {
$cache->delete('delays_filter');
@@ -95,4 +105,28 @@ class form_delays_filter extends \moodleform {
}
}
+ /**
+ * Override get data to return cached data when nothing was submitted.
+ * @return object
+ */
+ public function get_data() {
+ $data = parent::get_data();
+ if ($data) {
+ return $data;
+ } else {
+ return $this->get_cache()->get('delays_filter');
+ }
+ }
+
+ /**
+ * @return cache the cache to get the cached mform data.
+ */
+ private function get_cache() {
+ if (!$this->cache) {
+ $this->cache = cache::make('tool_lifecycle', 'mformdata');
+ }
+ return $this->cache;
+ }
+
+
}
diff --git a/classes/table/delayed_courses_table.php b/classes/table/delayed_courses_table.php
index d655883..cf467b4 100644
--- a/classes/table/delayed_courses_table.php
+++ b/classes/table/delayed_courses_table.php
@@ -36,15 +36,16 @@ require_once($CFG->libdir . '/tablelib.php');
*/
class delayed_courses_table extends \table_sql {
- /** @var $filterdata object|null data from mform */
- private $filterdata;
+ /** @var $workflow null|int|string workflow to filter for. Might be workflowid or "global" to filter for global delays. */
+ private $workflow;
/**
* Constructor for delayed_courses_table.
+ *
+ * @param $filterdata object|null filterdata from moodle form.
*/
public function __construct($filterdata) {
parent::__construct('tool_lifecycle-delayed-courses');
- $this->filterdata = $filterdata;
$fields = 'c.id as courseid, c.fullname as coursefullname, cat.name as category, ';
@@ -54,16 +55,16 @@ class delayed_courses_table extends \table_sql {
if ($filterdata && $filterdata->workflow) {
if ($filterdata->workflow == 'global') {
$selectseperatedelays = false;
+ $this->workflow = 'global';
} else if (is_number($filterdata->workflow)) {
$selectglobaldelays = false;
$workflowfilterid = $filterdata->workflow;
+ $this->workflow = $filterdata->workflow;
} else {
- throw new \coding_exception('action has to be "global" or a int value');
+ throw new \coding_exception('workflow has to be "global" or a int value');
}
}
-
-
if ($selectseperatedelays) {
$fields .= 'dw.workflowid, w.title as workflow, dw.delayeduntil AS workflowdelay, maxtable.wfcount AS workflowcount, ';
} else {
@@ -102,8 +103,13 @@ class delayed_courses_table extends \table_sql {
'JOIN {tool_lifecycle_workflow} w ON dw.workflowid = w.id ';
if ($selectglobaldelays) {
- $from .= 'FULL JOIN {tool_lifecycle_delayed} d ON dw.courseid = d.courseid ' .
+ $from .= 'FULL JOIN (' .
+ 'SELECT * FROM {tool_lifecycle_delayed} d ' .
+ 'WHERE d.delayeduntil > :time2 ' .
+ ') d ON dw.courseid = d.courseid ' .
'JOIN {course} c ON c.id = COALESCE(dw.courseid, d.courseid) ';
+
+ $params['time2'] = time();
} else {
$from .= 'JOIN {course} c ON c.id = dw.courseid ';
}
@@ -129,11 +135,9 @@ class delayed_courses_table extends \table_sql {
*
* @param $row
* @return string
- * @throws \coding_exception
- * @throws \dml_exception
*/
public function col_workflow($row) {
- if($row->globaldelay >= time()) {
+ if ($row->globaldelay >= time()) {
if ($row->workflowcount == 1) {
$text = get_string('delayed_globally_and_seperately_for_one', 'tool_lifecycle');
} else if ($row->workflowcount > 1) {
@@ -146,7 +150,7 @@ class delayed_courses_table extends \table_sql {
} else {
if ($row->workflowcount <= 0) {
$text = '';
- } else if($row->workflowcount == 1) {
+ } else if ($row->workflowcount == 1) {
$dateformat = get_string('strftimedatetimeshort', 'core_langconfig');
$date = userdate($row->workflowdelay, $dateformat);
$text = get_string('delayed_for_workflow_until', 'tool_lifecycle',
@@ -166,25 +170,20 @@ class delayed_courses_table extends \table_sql {
*
* @param $row
* @return string
- * @throws \coding_exception
- * @throws \dml_exception
*/
private function get_mouseover($row) {
global $DB;
$text = '';
$dateformat = get_string('strftimedatetimeshort', 'core_langconfig');
- if($row->globaldelay >= time()) {
+ if ($row->globaldelay >= time()) {
$date = userdate($row->globaldelay, $dateformat);
$text .= get_string('globally_until_date', 'tool_lifecycle', $date) . ' ';
}
- if($row->workflowcount == 1) {
+ if ($row->workflowcount == 1) {
$date = userdate($row->workflowdelay, $dateformat);
$text .= get_string('name_until_date', 'tool_lifecycle',
array('name' => $row->workflow, 'date' => $date)) . ' ';
} else if ($row->workflowcount > 1) {
-
- // TODO Make sure dw.workflowid is distinct.
-
$sql = 'SELECT dw.id, dw.delayeduntil, w.title
FROM {tool_lifecycle_delayed_workf} dw
JOIN {tool_lifecycle_workflow} w ON dw.workflowid = w.id
@@ -204,19 +203,21 @@ class delayed_courses_table extends \table_sql {
* Render tools column.
* @param object $row Row data.
* @return string pluginname of the subplugin
- * @throws \coding_exception
- * @throws \invalid_parameter_exception
*/
public function col_tools($row) {
global $PAGE, $OUTPUT;
- // TODO Add view specific workflow parameter.
-
- $button = new \single_button(
- new \moodle_url($PAGE->url, array(
- 'action' => 'delete',
- 'cid' => $row->courseid,
- 'sesskey' => sesskey()
- )),
+
+ $params = array(
+ 'action' => 'delete',
+ 'cid' => $row->courseid,
+ 'sesskey' => sesskey()
+ );
+
+ if ($this->workflow) {
+ $params['workflow'] = $this->workflow;
+ }
+
+ $button = new \single_button(new \moodle_url($PAGE->url, $params),
get_string('delete_delay', 'tool_lifecycle'));
return $OUTPUT->render($button);
}
diff --git a/db/caches.php b/db/caches.php
index 209ad71..e68e32e 100644
--- a/db/caches.php
+++ b/db/caches.php
@@ -15,10 +15,9 @@
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
/**
- * Cache Definition for Admin Approve Step
+ * Cache Definition for tool_lifecycle
*
- * @package tool_lifecycle_step
- * @subpackage adminapprove
+ * @package tool_lifecycle
* @copyright 2019 Justus Dieckmann WWU
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
diff --git a/delayedcourses.php b/delayedcourses.php
index c648443..e78a820 100644
--- a/delayedcourses.php
+++ b/delayedcourses.php
@@ -42,7 +42,7 @@ if ($action) {
$cid = required_param('cid', PARAM_INT);
$workflow = optional_param('workflow', null, PARAM_ALPHANUM);
if ($workflow) {
- if (is_int($workflow)) {
+ if (is_number($workflow)) {
$DB->delete_records('tool_lifecycle_delayed_workf', array('courseid' => $cid, 'workflowid' => $workflow));
} else if ($workflow == 'global') {
$DB->delete_records('tool_lifecycle_delayed', array('courseid' => $cid));
--
GitLab