L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-52485 tool_lp: Template due date can be changed at will

Browse Source 
The only rule is that it must be set in the future (though we permit
due dates time minutes old), or it must be unset.
This commit is contained in:
Frederic Massart
2016-04-18 10:58:49 +08:00
parent 3c5811644c
commit a967d2aa8c
6 changed files with 134 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
admin/tool/lp/classes/plan.php
+3 -2
View File
@@ -428,8 +428,9 @@ class plan extends persistent {
*/
protected function validate_duedate($value) {
// We do not check duedate when plan is draft, complete or unset.
if ($this->get_status() == self::STATUS_DRAFT
// We do not check duedate when plan is draft, complete, unset, or based on a template.
if ($this->is_based_on_template()
|| $this->get_status() == self::STATUS_DRAFT
|| $this->get_status() == self::STATUS_COMPLETE
|| empty($value)) {
return true;
admin/tool/lp/classes/template.php
+12 -19
View File
@@ -38,9 +38,6 @@ class template extends persistent {
const TABLE = 'tool_lp_template';
/** One day threshold **/
const DUEDATE_THRESHOLD = 86400;
/** @var template object before update. */
protected $beforeupdate = null;
@@ -166,7 +163,10 @@ class template extends persistent {
/**
* Validate the due date.
* We set a threshold to avoid creating templates with duedate too soon, so the plans can be created safely.
*
* The due date can always be changed, but when it is it must be:
* - unset
* - set in the future.
*
* @param int $value The due date.
* @return bool|lang_string
@@ -176,25 +176,18 @@ class template extends persistent {
// During update.
if ($this->get_id()) {
$before = $this->beforeupdate->get_duedate();
$haschanged = $before != $value;
if (!empty($before) && $before <= time() && $haschanged) {
// We cannot set a due date after it was reached.
return new lang_string('errorcannotchangeapastduedate', 'tool_lp');
// The value has not changed, then it's always OK.
if ($before == $value) {
return true;
}
}
// During create and update when due date in the past or too soon.
if (!empty($value)) {
if ($value <= time()) {
// We cannot set the date in the past, but we can leave it empty.
return new lang_string('errorcannotsetduedateinthepast', 'tool_lp');
}
if ($value <= time() + self::DUEDATE_THRESHOLD) {
// We cannot set the date too soon, but we can leave it empty.
return new lang_string('errorcannotsetduedatetoosoon', 'tool_lp');
}
// During create and update, the date must be set in the future, or not set.
if (!empty($value) && $value <= time() - 600) {
// We cannot set the date in the past. But we allow for 10 minutes of margin so that
// a user can set the due date to "now" without risking to hit a validation error.
return new lang_string('errorcannotsetduedateinthepast', 'tool_lp');
}
return true;
admin/tool/lp/classes/template_cohort.php
+2 -5
View File
@@ -163,21 +163,18 @@ class template_cohort extends persistent {
global $DB;
// Safe enough with 2 hours.
$time = time() + self::DUEDATE_THRESHOLD;
$skipsql = !$unlinkedaremissing ? '(t.id = p.templateid OR t.id = p.origtemplateid)' : 't.id = p.templateid';
// TODO MDL-52526 only unexpired template are considered and fix the time()+1 duedate issue.
$sql = "SELECT cm.userid, t.*
FROM {cohort_members} cm
JOIN {" . self::TABLE . "} tc ON cm.cohortid = tc.cohortid
JOIN {" . template::TABLE . "} t ON (tc.templateid = t.id
AND t.visible = 1
AND (t.duedate > :time OR t.duedate = 0))
JOIN {" . template::TABLE . "} t ON (tc.templateid = t.id AND t.visible = 1)
LEFT JOIN {" . plan::TABLE . "} p ON (cm.userid = p.userid AND $skipsql)
WHERE p.id IS NULL
ORDER BY t.id";
$results = $DB->get_records_sql($sql, array('time' => $time));
$results = $DB->get_records_sql($sql);
$missingplans = array();
foreach ($results as $usertemplate) {
admin/tool/lp/tests/api_test.php
+1 -1
View File
@@ -681,7 +681,7 @@ class tool_lp_api_testcase extends advanced_testcase {
$data->shortname = 'Awesome!';
$data->description = 'This is too awesome!';
$data->descriptionformat = FORMAT_HTML;
$data->duedate = $time + tool_lp\template::DUEDATE_THRESHOLD + 200;
$data->duedate = $time + 200;
api::update_template($data);
$tpl1->read();
admin/tool/lp/tests/task_test.php
+1 -1
View File
@@ -128,7 +128,7 @@ class tool_lp_task_testcase extends advanced_testcase {
$user5 = $dg->create_user();
$cohort = $dg->create_cohort();
$tpl = $lpg->create_template(array('duedate' => time() + tool_lp\template::DUEDATE_THRESHOLD + 400));
$tpl = $lpg->create_template(array('duedate' => time() + 400));
// Add 2 users to the cohort.
cohort_add_member($cohort->id, $user1->id);
admin/tool/lp/tests/template_test.php
+115
View File
@@ -0,0 +1,115 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
/**
* Template persistent class tests.
*
* @package tool_lp
* @copyright 2016 Frédéric Massart - FMCorz.net
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
defined('MOODLE_INTERNAL') || die();
global $CFG;
use tool_lp\template;
/**
* Template persistent testcase.
*
* @package tool_lp
* @copyright 2016 Frédéric Massart - FMCorz.net
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class tool_lp_template_testcase extends advanced_testcase {
public function test_validate_duedate() {
global $DB;
$this->resetAfterTest();
$tpl = $this->getDataGenerator()->get_plugin_generator('tool_lp')->create_template();
// No due date -> pass.
$tpl->set_duedate(0);
$this->assertTrue($tpl->is_valid());
// Setting new due date in the past -> fail.
$tpl->set_duedate(1);
$errors = $tpl->get_errors();
$this->assertCount(1, $errors);
$this->assertArrayHasKey('duedate', $errors);
// Setting new due date in very close past -> pass.
$tpl->set_duedate(time() - 10);
$this->assertTrue($tpl->is_valid());
// Setting new due date in future -> pass.
$tpl->set_duedate(time() + 600);
$this->assertTrue($tpl->is_valid());
// Save due date in the future.
$tpl->update();
// Going from future date to past -> fail.
$tpl->set_duedate(1);
$errors = $tpl->get_errors();
$this->assertCount(1, $errors);
$this->assertArrayHasKey('duedate', $errors);
// Going from future date to none -> pass.
$tpl->set_duedate(0);
$this->assertTrue($tpl->is_valid());
// Going from future date to other future -> pass.
$tpl->set_duedate(time() + 6000);
$this->assertTrue($tpl->is_valid());
// Going from future date to close past -> pass.
$tpl->set_duedate(time() - 10);
$this->assertTrue($tpl->is_valid());
// Mocking past due date.
$record = $tpl->to_record();
$record->duedate = 1;
$DB->update_record(template::TABLE, $record);
$tpl->read();
$this->assertEquals(1, $tpl->get_duedate());
// Not changing the past due date -> pass.
// Note: changing visibility to force validation.
$tpl->set_visible(0);
$tpl->set_visible(1);
$this->assertTrue($tpl->is_valid());
// Changing past due date to other past -> fail.
$tpl->set_duedate(10);
$errors = $tpl->get_errors();
$this->assertCount(1, $errors);
$this->assertArrayHasKey('duedate', $errors);
// Changing past due date close past -> pass.
$tpl->set_duedate(time() + 10);
$this->assertTrue($tpl->is_valid());
// Changing past due date to future -> pass.
$tpl->set_duedate(time() + 1000);
$this->assertTrue($tpl->is_valid());
// Changing past due date to none -> pass.
$tpl->set_duedate(0);
$this->assertTrue($tpl->is_valid());
}
}