From 0e290bc008b76ac75fd142526045460b1580d445 Mon Sep 17 00:00:00 2001 From: sam marshall Date: Thu, 18 Sep 2014 12:59:19 +0100 Subject: [PATCH] MDL-47322 Availability: empty availability should be saved as null --- availability/classes/tree.php | 8 ++++++ availability/tests/tree_test.php | 16 ++++++++++++ course/modlib.php | 18 +++++++++++++ course/tests/courselib_test.php | 45 +++++++++++++++++++++++++++++--- 4 files changed, 84 insertions(+), 3 deletions(-) diff --git a/availability/classes/tree.php b/availability/classes/tree.php index 29c3b662fa7..76e738b5568 100644 --- a/availability/classes/tree.php +++ b/availability/classes/tree.php @@ -575,6 +575,14 @@ class tree extends tree_node { return $result; } + /** + * Checks whether this tree is empty (contains no children). + * @return boolean True if empty + */ + public function is_empty() { + return count($this->children) === 0; + } + /** * Recursively gets all children of a particular class (you can use a base * class to get all conditions, or a specific class). diff --git a/availability/tests/tree_test.php b/availability/tests/tree_test.php index dd7c6b82ff3..38182136754 100644 --- a/availability/tests/tree_test.php +++ b/availability/tests/tree_test.php @@ -23,6 +23,7 @@ */ use core_availability\capability_checker; +use \core_availability\tree; defined('MOODLE_INTERNAL') || die(); @@ -488,6 +489,21 @@ class tree_testcase extends \advanced_testcase { $tree->get_full_information($info)); } + /** + * Tests the is_empty() function. + */ + public function test_is_empty() { + // Tree with nothing in should be empty. + $structure = self::tree(array()); + $tree = new tree($structure); + $this->assertTrue($tree->is_empty()); + + // Tree with something in is not empty. + $structure = self::tree(array(self::mock(array('m' => '1')))); + $tree = new tree($structure); + $this->assertFalse($tree->is_empty()); + } + /** * Tests the get_all_children() function. */ diff --git a/course/modlib.php b/course/modlib.php index 14c91bf50b2..4eb9d0656e2 100644 --- a/course/modlib.php +++ b/course/modlib.php @@ -86,6 +86,15 @@ function add_moduleinfo($moduleinfo, $course, $mform = null) { } else if (property_exists($moduleinfo, 'availability')) { $newcm->availability = $moduleinfo->availability; } + // If there is any availability data, verify it. + if ($newcm->availability) { + $tree = new \core_availability\tree(json_decode($newcm->availability)); + // Save time and database space by setting null if the only data + // is an empty tree. + if ($tree->is_empty()) { + $newcm->availability = null; + } + } } if (isset($moduleinfo->showdescription)) { $newcm->showdescription = $moduleinfo->showdescription; @@ -502,6 +511,15 @@ function update_moduleinfo($cm, $moduleinfo, $course, $mform = null) { } else if (property_exists($moduleinfo, 'availability')) { $cm->availability = $moduleinfo->availability; } + // If there is any availability data, verify it. + if ($cm->availability) { + $tree = new \core_availability\tree(json_decode($cm->availability)); + // Save time and database space by setting null if the only data + // is an empty tree. + if ($tree->is_empty()) { + $cm->availability = null; + } + } } if (isset($moduleinfo->showdescription)) { $cm->showdescription = $moduleinfo->showdescription; diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index cbb5fb436e4..e8c513a127b 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -460,12 +460,12 @@ class core_course_courselib_testcase extends advanced_testcase { // Conditional activity. $coursegradeitem = grade_item::fetch_course_item($moduleinfo->course); //the activity will become available only when the user reach some grade into the course itself. - $moduleinfo->availability = '{"op":"&","showc":[true,true],"c":[' . + $moduleinfo->availability = '{"op":"&","showc":[true,true,true,true,true],"c":[' . '{"type":"date","d":">=","t":' . time() . '},' . - '{"type":"date","d":"<","t":' . (time() + (7 * 24 * 3600)) . '}' . + '{"type":"date","d":"<","t":' . (time() + (7 * 24 * 3600)) . '},' . '{"type":"grade","id":' . $coursegradeitem->id . ',"min":10,"max":80},' . '{"type":"profile","sf":"email","op":"contains","v":"@"},'. - '{"type":"completion","id":'. $assigncm->id . ',"e":' . COMPLETION_COMPLETE . '}' . + '{"type":"completion","cm":'. $assigncm->id . ',"e":' . COMPLETION_COMPLETE . '}' . ']}'; // Grading and Advanced grading. @@ -2497,4 +2497,43 @@ class core_course_courselib_testcase extends advanced_testcase { $this->assertEventLegacyLogData($expectedlegacydata, $event); $this->assertEventContextNotUsed($event); } + + /** + * Tests that when creating or updating a module, if the availability settings + * are present but set to an empty tree, availability is set to null in + * database. + */ + public function test_empty_availability_settings() { + global $DB; + $this->setAdminUser(); + $this->resetAfterTest(); + + // Enable availability. + set_config('enableavailability', 1); + + // Test add. + $emptyavailability = '{"op":"&","c":[],"showc":[]}'; + $course = self::getDataGenerator()->create_course(); + $label = self::getDataGenerator()->create_module('label', array( + 'course' => $course, 'availability' => $emptyavailability)); + $this->assertNull($DB->get_field('course_modules', 'availability', + array('id' => $label->cmid))); + + // Test update. + $formdata = $DB->get_record('course_modules', array('id' => $label->cmid)); + unset($formdata->availability); + $formdata->availabilityconditionsjson = $emptyavailability; + $formdata->modulename = 'label'; + $formdata->coursemodule = $label->cmid; + $draftid = 0; + file_prepare_draft_area($draftid, context_module::instance($label->cmid)->id, + 'mod_label', 'intro', 0); + $formdata->introeditor = array( + 'itemid' => $draftid, + 'text' => '

Yo

', + 'format' => FORMAT_HTML); + update_module($formdata); + $this->assertNull($DB->get_field('course_modules', 'availability', + array('id' => $label->cmid))); + } }