From c78ebeac1ba3983ef59c97067095fc8007c70cfa Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Tue, 12 Jan 2016 14:41:32 +0800 Subject: [PATCH] MDL-52485 tool_lp: Do not validate unchanged due dates in plans --- admin/tool/lp/classes/plan.php | 12 ++++++++++++ admin/tool/lp/tests/plan_test.php | 28 +++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/admin/tool/lp/classes/plan.php b/admin/tool/lp/classes/plan.php index c153c425c42..a41de91da54 100644 --- a/admin/tool/lp/classes/plan.php +++ b/admin/tool/lp/classes/plan.php @@ -436,6 +436,18 @@ class plan extends persistent { return true; } + // During update. + if ($this->get_id()) { + $before = $this->beforeupdate->get_duedate(); + $beforestatus = $this->beforeupdate->get_status(); + + // The value has not changed, then it's always OK. Though if we're going + // from draft to active it has to has to be validated. + if ($before == $value && $beforestatus != self::STATUS_DRAFT) { + return true; + } + } + if ($value <= time()) { // We cannot set the date in the past. return new lang_string('errorcannotsetduedateinthepast', 'tool_lp'); diff --git a/admin/tool/lp/tests/plan_test.php b/admin/tool/lp/tests/plan_test.php index 608d7e37474..43ea6f86a1f 100644 --- a/admin/tool/lp/tests/plan_test.php +++ b/admin/tool/lp/tests/plan_test.php @@ -25,6 +25,8 @@ defined('MOODLE_INTERNAL') || die(); global $CFG; +use tool_lp\plan; + /** * Plan persistent testcase. * @@ -301,6 +303,23 @@ class tool_lp_plan_testcase extends advanced_testcase { // Updating active plan. $plan->update(); + // Active to active: past => same past (pass). + $record = $plan->to_record(); + $record->duedate = 1; + $DB->update_record(plan::TABLE, $record); + $plan->read(); + $plan->set_description(uniqid()); // Force revalidation. + $this->assertTrue($plan->is_valid()); + + // Active to active: past => unset (pass). + $plan->set_duedate(0); + $this->assertTrue($plan->is_valid()); + $plan->update(); + + // Active to active: unset => unset (pass). + $plan->set_description(uniqid()); // Force revalidation. + $this->assertTrue($plan->is_valid()); + // Active to active: unset date => past date(fail). $plan->set_duedate(time() - 100); $expected = array( @@ -322,6 +341,10 @@ class tool_lp_plan_testcase extends advanced_testcase { // Updating active plan with future date. $plan->update(); + // Active to active: future => same future (pass). + $plan->set_description(uniqid()); // Force revalidation. + $this->assertTrue($plan->is_valid()); + // Active to active: future date => unset date (pass). $plan->set_duedate(0); $this->assertEquals(true, $plan->validate()); @@ -411,7 +434,10 @@ class tool_lp_plan_testcase extends advanced_testcase { $success = tool_lp\api::reopen_plan($plan->get_id()); $this->assertTrue($success); $plan->read(); - $this->assertEquals(time() + tool_lp\plan::DUEDATE_THRESHOLD + 10, $plan->get_duedate()); + + // Check that the due date has not changed, but allow for PHP Unit latency. + $this->assertTrue($plan->get_duedate() >= time() + tool_lp\plan::DUEDATE_THRESHOLD + 10); + $this->assertTrue($plan->get_duedate() <= time() + tool_lp\plan::DUEDATE_THRESHOLD + 15); } }