From cb49d89ba47780ef1e19bb9021a4ccc670b7508a Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Mon, 20 Nov 2017 13:58:36 +0800 Subject: [PATCH 1/4] MDL-60851 backup: More unit tests for dependencies --- backup/util/settings/tests/settings_test.php | 44 ++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/backup/util/settings/tests/settings_test.php b/backup/util/settings/tests/settings_test.php index 45bc4ecd811..99539f9689c 100644 --- a/backup/util/settings/tests/settings_test.php +++ b/backup/util/settings/tests/settings_test.php @@ -290,6 +290,50 @@ class backp_settings_testcase extends basic_testcase { $this->assertEquals($ubs3->get_status(), $ubs1->get_status()); } + /** + * Test that locked and unlocked states on dependent backup settings at the same level + * correctly do not flow from the parent to the child setting when the setting is locked by permissions. + */ + function test_dependency_empty_locked_by_permission_child_is_not_unlocked() { + // Check dependencies are working ok + $bs1 = new mock_backup_setting('test1', base_setting::IS_INTEGER, 2); + $bs1->set_level(1); + $bs2 = new mock_backup_setting('test2', base_setting::IS_INTEGER, 2); + $bs2->set_level(1); // Same level *must* work + $bs1->add_dependency($bs2, setting_dependency::DISABLED_EMPTY); + + $bs1->set_status(base_setting::LOCKED_BY_PERMISSION); + $this->assertEquals(base_setting::LOCKED_BY_HIERARCHY, $bs2->get_status()); + $this->assertEquals(base_setting::LOCKED_BY_PERMISSION, $bs1->get_status()); + $bs2->set_status(base_setting::LOCKED_BY_PERMISSION); + $this->assertEquals(base_setting::LOCKED_BY_PERMISSION, $bs1->get_status()); + + // Unlocking the parent should NOT unlock the child. + $bs1->set_status(base_setting::NOT_LOCKED); + + $this->assertEquals(base_setting::LOCKED_BY_PERMISSION, $bs2->get_status()); + } + + /** + * Test that locked and unlocked states on dependent backup settings at the same level + * correctly do flow from the parent to the child setting when the setting is locked by config. + */ + function test_dependency_not_empty_locked_by_config_parent_is_unlocked() { + $bs1 = new mock_backup_setting('test1', base_setting::IS_INTEGER, 0); + $bs1->set_level(1); + $bs2 = new mock_backup_setting('test2', base_setting::IS_INTEGER, 0); + $bs2->set_level(1); // Same level *must* work + $bs1->add_dependency($bs2, setting_dependency::DISABLED_NOT_EMPTY); + + $bs1->set_status(base_setting::LOCKED_BY_CONFIG); + $this->assertEquals(base_setting::LOCKED_BY_HIERARCHY, $bs2->get_status()); + $this->assertEquals(base_setting::LOCKED_BY_CONFIG, $bs1->get_status()); + + // Unlocking the parent should unlock the child. + $bs1->set_status(base_setting::NOT_LOCKED); + $this->assertEquals(base_setting::NOT_LOCKED, $bs2->get_status()); + } + /** * test backup_setting class */ From 42c6dadcd44465d256580b0ef23e42d7f25fc581 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Mon, 20 Nov 2017 13:58:05 +0800 Subject: [PATCH 2/4] MDL-60851 backup: Fix undefined $value --- backup/util/settings/setting_dependency.class.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/backup/util/settings/setting_dependency.class.php b/backup/util/settings/setting_dependency.class.php index 079537fdc5f..b87c958f645 100644 --- a/backup/util/settings/setting_dependency.class.php +++ b/backup/util/settings/setting_dependency.class.php @@ -542,6 +542,7 @@ class setting_dependency_disabledif_not_empty extends setting_dependency_disable */ public function is_locked() { // If the setting is locked or the dependent setting should be locked then return true + $value = $this->setting->get_value(); if ($this->setting->get_status() !== base_setting::NOT_LOCKED || !empty($value)) { return true; } @@ -606,6 +607,7 @@ class setting_dependency_disabledif_empty extends setting_dependency_disabledif_ */ public function is_locked() { // If the setting is locked or the dependent setting should be locked then return true + $value = $this->setting->get_value(); if ($this->setting->get_status() !== base_setting::NOT_LOCKED || empty($value)) { return true; } From f86abcf932b7110a50d8bee79a4c1449acd13064 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Mon, 20 Nov 2017 14:19:36 +0800 Subject: [PATCH 3/4] MDL-60851 backup: Sanitise setting dependencies The only different between each setting dependency type is the evaluation of the condition, and the mform js validation arguments - so that should be the only thing that is extended by each subclass. --- .../settings/setting_dependency.class.php | 258 +++++------------- 1 file changed, 65 insertions(+), 193 deletions(-) diff --git a/backup/util/settings/setting_dependency.class.php b/backup/util/settings/setting_dependency.class.php index b87c958f645..a9f98ebb46b 100644 --- a/backup/util/settings/setting_dependency.class.php +++ b/backup/util/settings/setting_dependency.class.php @@ -189,8 +189,8 @@ class setting_dependency_disabledif_equals extends setting_dependency { * @return bool */ public function is_locked() { - // If the setting is locked or the dependent setting should be locked then return true - if ($this->setting->get_status() !== base_setting::NOT_LOCKED || $this->setting->get_value() == $this->value) { + // If the setting is locked or the dependent setting should be locked then return true. + if ($this->setting->get_status() !== base_setting::NOT_LOCKED || $this->evaluate_disabled_condition($this->setting->get_value())) { return true; } // Else the dependent setting is not locked by this setting_dependency. @@ -208,12 +208,20 @@ class setting_dependency_disabledif_equals extends setting_dependency { return false; } $prevalue = $this->dependentsetting->get_value(); - // If the setting is the desired value enact the dependency - if ($this->setting->get_value() == $this->value) { + // If the setting is the desired value enact the dependency. + $settingvalue = $this->setting->get_value(); + if ($this->evaluate_disabled_condition($settingvalue)) { // The dependent setting needs to be locked by hierachy and set to the // default value. $this->dependentsetting->set_status(base_setting::LOCKED_BY_HIERARCHY); - $this->dependentsetting->set_value($this->defaultvalue); + + // For checkboxes the default value is false, but when the setting is + // locked, the value should inherit from the parent setting. + if ($this->defaultvalue === false) { + $this->dependentsetting->set_value($settingvalue); + } else { + $this->dependentsetting->set_value($this->defaultvalue); + } } else if ($this->dependentsetting->get_status() == base_setting::LOCKED_BY_HIERARCHY) { // We can unlock the dependent setting $this->dependentsetting->set_status(base_setting::NOT_LOCKED); @@ -232,8 +240,8 @@ class setting_dependency_disabledif_equals extends setting_dependency { // Store the current status $currentstatus = $this->setting->get_status(); if ($currentstatus == base_setting::NOT_LOCKED) { - if ($prevalue == base_setting::LOCKED_BY_HIERARCHY && $this->setting->get_value() != $this->value) { - // Dependency has changes, is not fine, unlock the dependent setting + if ($prevalue == base_setting::LOCKED_BY_HIERARCHY && !$this->evaluate_disabled_condition($this->setting->get_value())) { + // Dependency has changes, is not fine, unlock the dependent setting. $this->dependentsetting->set_status(base_setting::NOT_LOCKED); } } else { @@ -277,6 +285,17 @@ class setting_dependency_disabledif_equals extends setting_dependency { 'value'=>$this->value ); } + + /** + * Evaluate the current value of the setting and return true if the dependent setting should be locked or false. + * This function should be abstract, but there will probably be existing sub-classes so we must provide a default + * implementation. + * @param mixed $value The value of the parent setting. + * @return bool + */ + protected function evaluate_disabled_condition($value) { + return $value == $this->value; + } } /** @@ -287,27 +306,16 @@ class setting_dependency_disabledif_equals extends setting_dependency { * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class setting_dependency_disabledif_not_equals extends setting_dependency_disabledif_equals { + /** - * Enforces the dependency if required. - * @return bool True if there were changes - */ - public function enforce() { - // This will be set to true if ANYTHING changes - $changes = false; - // First process any value changes - if (!$this->process_value_change($this->setting->get_value())) { - $changes = true; - } - // Second process any status changes - if ($this->process_status_change($this->setting->get_status())) { - $changes = true; - } - // Finally process visibility changes - if ($this->process_visibility_change($this->setting->get_visibility())) { - $changes = true; - } - return $changes; + * Evaluate the current value of the setting and return true if the dependent setting should be locked or false. + * @param mixed $value The value of the parent setting. + * @return bool + */ + protected function evaluate_disabled_condition($value) { + return $value != $this->value; } + /** * Returns an array of properties suitable to be used to define a moodleforms * disabled command @@ -323,100 +331,17 @@ class setting_dependency_disabledif_not_equals extends setting_dependency_disabl } } -//with array -class setting_dependency_disabledif_equals2 extends setting_dependency { +class setting_dependency_disabledif_in_array extends setting_dependency_disabledif_equals { + /** - * The value to compare to - * @var mixed - */ - protected $value; - /** - * Creates the dependency - * - * @param base_setting $setting - * @param base_setting $dependentsetting - * @param mixed $value - * @param mixed $defaultvalue - */ - public function __construct(base_setting $setting, base_setting $dependentsetting, array $value, $defaultvalue = false) { - parent::__construct($setting, $dependentsetting, $defaultvalue); - $this->value = $value; - } - /** - * Returns true if the dependent setting is locked by this setting_dependency. + * Evaluate the current value of the setting and return true if the dependent setting should be locked or false. + * @param mixed $value The value of the parent setting. * @return bool */ - public function is_locked() { - // If the setting is locked or the dependent setting should be locked then return true - if ($this->setting->get_status() !== base_setting::NOT_LOCKED || in_array($this->setting->get_value(), $this->value)) { - return true; - } - // Else the dependent setting is not locked by this setting_dependency. - return false; - } - /** - * Processes a value change in the primary setting - * @param mixed $oldvalue - * @return bool - */ - protected function process_value_change($oldvalue) { - $prevalue = $this->dependentsetting->get_value(); - // If the setting is the desired value enact the dependency - if (in_array($this->setting->get_value(), $this->value)) { - // The dependent setting needs to be locked by hierachy and set to the - // default value. - $this->dependentsetting->set_status(base_setting::LOCKED_BY_HIERARCHY); - $this->dependentsetting->set_value($this->defaultvalue); - } else if ($this->dependentsetting->get_status() == base_setting::LOCKED_BY_HIERARCHY) { - // We can unlock the dependent setting - $this->dependentsetting->set_status(base_setting::NOT_LOCKED); - } - // Return true if the value has changed for the dependent setting - return ($prevalue != $this->dependentsetting->get_value()); - } - /** - * Processes a status change in the primary setting - * @param mixed $oldstatus - * @return bool - */ - protected function process_status_change($oldstatus) { - // Store the dependent status - $prevalue = $this->dependentsetting->get_status(); - // Store the current status - $currentstatus = $this->setting->get_status(); - if ($currentstatus == base_setting::NOT_LOCKED) { - if ($prevalue == base_setting::LOCKED_BY_HIERARCHY && !in_array($this->setting->get_value(), $this->value)) { - // Dependency has changes, is not fine, unlock the dependent setting - $this->dependentsetting->set_status(base_setting::NOT_LOCKED); - } - } else { - // Make sure the dependent setting is also locked, in this case by hierarchy - $this->dependentsetting->set_status(base_setting::LOCKED_BY_HIERARCHY); - } - // Return true if the dependent setting has changed. - return ($prevalue != $this->dependentsetting->get_status()); - } - /** - * Enforces the dependency if required. - * @return bool True if there were changes - */ - public function enforce() { - // This will be set to true if ANYTHING changes - $changes = false; - // First process any value changes - if ($this->process_value_change($this->setting->get_value())) { - $changes = true; - } - // Second process any status changes - if ($this->process_status_change($this->setting->get_status())) { - $changes = true; - } - // Finally process visibility changes - if ($this->process_visibility_change($this->setting->get_visibility())) { - $changes = true; - } - return $changes; + protected function evaluate_disabled_condition($value) { + return in_array($value, $this->value); } + /** * Returns an array of properties suitable to be used to define a moodleforms * disabled command @@ -430,8 +355,12 @@ class setting_dependency_disabledif_equals2 extends setting_dependency { 'value'=>$this->value ); } + } +// This class is here for backwards compatibility (terrible name). +class setting_dependency_disabledif_equals2 extends setting_dependency_disabledif_in_array { +} /** * A dependency that disables the secondary element if the primary element is @@ -497,6 +426,16 @@ class setting_dependency_disabledif_not_empty extends setting_dependency_disable parent::__construct($setting, $dependentsetting, false, $defaultvalue); $this->value = false; } + + /** + * Evaluate the current value of the setting and return true if the dependent setting should be locked or false. + * @param mixed $value The value of the parent setting. + * @return bool + */ + protected function evaluate_disabled_condition($value) { + return !empty($value); + } + /** * Returns an array of properties suitable to be used to define a moodleforms * disabled command @@ -510,45 +449,6 @@ class setting_dependency_disabledif_not_empty extends setting_dependency_disable 'value'=>'' ); } - /** - * Processes a value change in the primary setting - * @param mixed $oldvalue - * @return bool - */ - protected function process_value_change($oldvalue) { - $prevalue = $this->dependentsetting->get_value(); - // If the setting is the desired value enact the dependency - $value = $this->setting->get_value(); - if (!empty($value)) { - // The dependent setting needs to be locked by hierachy and set to the - // default value. - $this->dependentsetting->set_status(base_setting::LOCKED_BY_HIERARCHY); - if ($this->defaultvalue === false) { - $this->dependentsetting->set_value($value); - } else { - $this->dependentsetting->set_value($this->defaultvalue); - } - } else if ($this->dependentsetting->get_status() == base_setting::LOCKED_BY_HIERARCHY) { - // We can unlock the dependent setting - $this->dependentsetting->set_status(base_setting::NOT_LOCKED); - } - // Return true if the value has changed for the dependent setting - return ($prevalue != $this->dependentsetting->get_value()); - } - - /** - * Returns true if the dependent setting is locked by this setting_dependency. - * @return bool - */ - public function is_locked() { - // If the setting is locked or the dependent setting should be locked then return true - $value = $this->setting->get_value(); - if ($this->setting->get_status() !== base_setting::NOT_LOCKED || !empty($value)) { - return true; - } - // Else the dependent setting is not locked by this setting_dependency. - return false; - } } /** @@ -563,6 +463,16 @@ class setting_dependency_disabledif_empty extends setting_dependency_disabledif_ parent::__construct($setting, $dependentsetting, false, $defaultvalue); $this->value = false; } + + /** + * Evaluate the current value of the setting and return true if the dependent setting should be locked or false. + * @param mixed $value The value of the parent setting. + * @return bool + */ + protected function evaluate_disabled_condition($value) { + return empty($value); + } + /** * Returns an array of properties suitable to be used to define a moodleforms * disabled command @@ -576,42 +486,4 @@ class setting_dependency_disabledif_empty extends setting_dependency_disabledif_ 'value'=>'' ); } - /** - * Processes a value change in the primary setting - * @param mixed $oldvalue - * @return bool - */ - protected function process_value_change($oldvalue) { - $prevalue = $this->dependentsetting->get_value(); - // If the setting is the desired value enact the dependency - $value = $this->setting->get_value(); - if (empty($value)) { - // The dependent setting needs to be locked by hierachy and set to the - // default value. - $this->dependentsetting->set_status(base_setting::LOCKED_BY_HIERARCHY); - if ($this->defaultvalue === false) { - $this->dependentsetting->set_value($value); - } else { - $this->dependentsetting->set_value($this->defaultvalue); - } - } else if ($this->dependentsetting->get_status() == base_setting::LOCKED_BY_HIERARCHY) { - // We can unlock the dependent setting - $this->dependentsetting->set_status(base_setting::NOT_LOCKED); - } - // Return true if the value has changed for the dependent setting - return ($prevalue != $this->dependentsetting->get_value()); - } - /** - * Returns true if the dependent setting is locked by this setting_dependency. - * @return bool - */ - public function is_locked() { - // If the setting is locked or the dependent setting should be locked then return true - $value = $this->setting->get_value(); - if ($this->setting->get_status() !== base_setting::NOT_LOCKED || empty($value)) { - return true; - } - // Else the dependent setting is not locked by this setting_dependency. - return false; - } } From 4d6a6fb0336c2a5646219b578a2f0bc863e7940b Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Mon, 20 Nov 2017 14:24:09 +0800 Subject: [PATCH 4/4] MDL-60851 backup: coding style fixes --- .../settings/setting_dependency.class.php | 134 ++++++++++-------- backup/util/settings/tests/settings_test.php | 20 +-- 2 files changed, 81 insertions(+), 73 deletions(-) diff --git a/backup/util/settings/setting_dependency.class.php b/backup/util/settings/setting_dependency.class.php index a9f98ebb46b..22ac0edc05a 100644 --- a/backup/util/settings/setting_dependency.class.php +++ b/backup/util/settings/setting_dependency.class.php @@ -1,5 +1,4 @@ setting = null; $this->dependentsetting = null; } @@ -94,16 +93,19 @@ abstract class setting_dependency { * @return bool */ final public function process_change($changetype, $oldvalue) { - // Check the type of change requested + // Check the type of change requested. switch ($changetype) { - // Process a status change - case base_setting::CHANGED_STATUS: return $this->process_status_change($oldvalue); - // Process a visibility change - case base_setting::CHANGED_VISIBILITY: return $this->process_visibility_change($oldvalue); - // Process a value change - case base_setting::CHANGED_VALUE: return $this->process_value_change($oldvalue); + // Process a status change. + case base_setting::CHANGED_STATUS: + return $this->process_status_change($oldvalue); + // Process a visibility change. + case base_setting::CHANGED_VISIBILITY: + return $this->process_visibility_change($oldvalue); + // Process a value change. + case base_setting::CHANGED_VALUE: + return $this->process_value_change($oldvalue); } - // Throw an exception if we get this far + // Throw an exception if we get this far. throw new backup_ui_exception('unknownchangetype'); } /** @@ -112,11 +114,11 @@ abstract class setting_dependency { * @return bool */ protected function process_visibility_change($oldvisibility) { - // Store the current dependent settings visibility for comparison + // Store the current dependent settings visibility for comparison. $prevalue = $this->dependentsetting->get_visibility(); - // Set it regardless of whether we need to + // Set it regardless of whether we need to. $this->dependentsetting->set_visibility($this->setting->get_visibility()); - // Return true if it changed + // Return true if it changed. return ($prevalue != $this->dependentsetting->get_visibility()); } /** @@ -182,7 +184,7 @@ class setting_dependency_disabledif_equals extends setting_dependency { */ public function __construct(base_setting $setting, base_setting $dependentsetting, $value, $defaultvalue = false) { parent::__construct($setting, $dependentsetting, $defaultvalue); - $this->value = ($value)?(string)$value:0; + $this->value = ($value) ? (string)$value : 0; } /** * Returns true if the dependent setting is locked by this setting_dependency. @@ -190,7 +192,8 @@ class setting_dependency_disabledif_equals extends setting_dependency { */ public function is_locked() { // If the setting is locked or the dependent setting should be locked then return true. - if ($this->setting->get_status() !== base_setting::NOT_LOCKED || $this->evaluate_disabled_condition($this->setting->get_value())) { + if ($this->setting->get_status() !== base_setting::NOT_LOCKED || + $this->evaluate_disabled_condition($this->setting->get_value())) { return true; } // Else the dependent setting is not locked by this setting_dependency. @@ -223,10 +226,10 @@ class setting_dependency_disabledif_equals extends setting_dependency { $this->dependentsetting->set_value($this->defaultvalue); } } else if ($this->dependentsetting->get_status() == base_setting::LOCKED_BY_HIERARCHY) { - // We can unlock the dependent setting + // We can unlock the dependent setting. $this->dependentsetting->set_status(base_setting::NOT_LOCKED); } - // Return true if the value has changed for the dependent setting + // Return true if the value has changed for the dependent setting. return ($prevalue != $this->dependentsetting->get_value()); } /** @@ -235,17 +238,18 @@ class setting_dependency_disabledif_equals extends setting_dependency { * @return bool */ protected function process_status_change($oldstatus) { - // Store the dependent status + // Store the dependent status. $prevalue = $this->dependentsetting->get_status(); - // Store the current status + // Store the current status. $currentstatus = $this->setting->get_status(); if ($currentstatus == base_setting::NOT_LOCKED) { - if ($prevalue == base_setting::LOCKED_BY_HIERARCHY && !$this->evaluate_disabled_condition($this->setting->get_value())) { + if ($prevalue == base_setting::LOCKED_BY_HIERARCHY && + !$this->evaluate_disabled_condition($this->setting->get_value())) { // Dependency has changes, is not fine, unlock the dependent setting. $this->dependentsetting->set_status(base_setting::NOT_LOCKED); } } else { - // Make sure the dependent setting is also locked, in this case by hierarchy + // Make sure the dependent setting is also locked, in this case by hierarchy. $this->dependentsetting->set_status(base_setting::LOCKED_BY_HIERARCHY); } // Return true if the dependent setting has changed. @@ -256,17 +260,17 @@ class setting_dependency_disabledif_equals extends setting_dependency { * @return bool True if there were changes */ public function enforce() { - // This will be set to true if ANYTHING changes + // This will be set to true if ANYTHING changes. $changes = false; - // First process any value changes + // First process any value changes. if ($this->process_value_change($this->setting->get_value())) { $changes = true; } - // Second process any status changes + // Second process any status changes. if ($this->process_status_change($this->setting->get_status())) { $changes = true; } - // Finally process visibility changes + // Finally process visibility changes. if ($this->process_visibility_change($this->setting->get_visibility())) { $changes = true; } @@ -279,10 +283,10 @@ class setting_dependency_disabledif_equals extends setting_dependency { */ public function get_moodleform_properties() { return array( - 'setting'=>$this->dependentsetting->get_ui_name(), - 'dependenton'=>$this->setting->get_ui_name(), - 'condition'=>'eq', - 'value'=>$this->value + 'setting' => $this->dependentsetting->get_ui_name(), + 'dependenton' => $this->setting->get_ui_name(), + 'condition' => 'eq', + 'value' => $this->value ); } @@ -299,12 +303,12 @@ class setting_dependency_disabledif_equals extends setting_dependency { } /** -* A dependency that disables the secondary setting if the primary setting is -* not equal to the provided value -* -* @copyright 2011 Darko Miletic -* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later -*/ + * A dependency that disables the secondary setting if the primary setting is + * not equal to the provided value + * + * @copyright 2011 Darko Miletic + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ class setting_dependency_disabledif_not_equals extends setting_dependency_disabledif_equals { /** @@ -317,20 +321,23 @@ class setting_dependency_disabledif_not_equals extends setting_dependency_disabl } /** - * Returns an array of properties suitable to be used to define a moodleforms - * disabled command - * @return array - */ + * Returns an array of properties suitable to be used to define a moodleforms + * disabled command + * @return array + */ public function get_moodleform_properties() { return array( - 'setting'=>$this->dependentsetting->get_ui_name(), - 'dependenton'=>$this->setting->get_ui_name(), - 'condition'=>'notequal', - 'value'=>$this->value + 'setting' => $this->dependentsetting->get_ui_name(), + 'dependenton' => $this->setting->get_ui_name(), + 'condition' => 'notequal', + 'value' => $this->value ); } } +/** + * Disable if a value is in a list. + */ class setting_dependency_disabledif_in_array extends setting_dependency_disabledif_equals { /** @@ -349,16 +356,17 @@ class setting_dependency_disabledif_in_array extends setting_dependency_disabled */ public function get_moodleform_properties() { return array( - 'setting'=>$this->dependentsetting->get_ui_name(), - 'dependenton'=>$this->setting->get_ui_name(), - 'condition'=>'eq', - 'value'=>$this->value + 'setting' => $this->dependentsetting->get_ui_name(), + 'dependenton' => $this->setting->get_ui_name(), + 'condition' => 'eq', + 'value' => $this->value ); } - } -// This class is here for backwards compatibility (terrible name). +/** + * This class is here for backwards compatibility (terrible name). + */ class setting_dependency_disabledif_equals2 extends setting_dependency_disabledif_in_array { } @@ -381,9 +389,9 @@ class setting_dependency_disabledif_checked extends setting_dependency_disabledi */ public function get_moodleform_properties() { return array( - 'setting'=>$this->dependentsetting->get_ui_name(), - 'dependenton'=>$this->setting->get_ui_name(), - 'condition'=>'checked' + 'setting' => $this->dependentsetting->get_ui_name(), + 'dependenton' => $this->setting->get_ui_name(), + 'condition' => 'checked' ); } } @@ -407,9 +415,9 @@ class setting_dependency_disabledif_not_checked extends setting_dependency_disab */ public function get_moodleform_properties() { return array( - 'setting'=>$this->dependentsetting->get_ui_name(), - 'dependenton'=>$this->setting->get_ui_name(), - 'condition'=>'notchecked' + 'setting' => $this->dependentsetting->get_ui_name(), + 'dependenton' => $this->setting->get_ui_name(), + 'condition' => 'notchecked' ); } } @@ -443,10 +451,10 @@ class setting_dependency_disabledif_not_empty extends setting_dependency_disable */ public function get_moodleform_properties() { return array( - 'setting'=>$this->dependentsetting->get_ui_name(), - 'dependenton'=>$this->setting->get_ui_name(), - 'condition'=>'notequal', - 'value'=>'' + 'setting' => $this->dependentsetting->get_ui_name(), + 'dependenton' => $this->setting->get_ui_name(), + 'condition' => 'notequal', + 'value' => '' ); } } @@ -480,10 +488,10 @@ class setting_dependency_disabledif_empty extends setting_dependency_disabledif_ */ public function get_moodleform_properties() { return array( - 'setting'=>$this->dependentsetting->get_ui_name(), - 'dependenton'=>$this->setting->get_ui_name(), - 'condition'=>'notequal', - 'value'=>'' + 'setting' => $this->dependentsetting->get_ui_name(), + 'dependenton' => $this->setting->get_ui_name(), + 'condition' => 'notequal', + 'value' => '' ); } } diff --git a/backup/util/settings/tests/settings_test.php b/backup/util/settings/tests/settings_test.php index 99539f9689c..63999fe2469 100644 --- a/backup/util/settings/tests/settings_test.php +++ b/backup/util/settings/tests/settings_test.php @@ -45,7 +45,7 @@ class backp_settings_testcase extends basic_testcase { /** * test base_setting class */ - function test_base_setting() { + public function test_base_setting() { // Instantiate base_setting and check everything $bs = new mock_base_setting('test', base_setting::IS_BOOLEAN); $this->assertTrue($bs instanceof base_setting); @@ -294,12 +294,12 @@ class backp_settings_testcase extends basic_testcase { * Test that locked and unlocked states on dependent backup settings at the same level * correctly do not flow from the parent to the child setting when the setting is locked by permissions. */ - function test_dependency_empty_locked_by_permission_child_is_not_unlocked() { - // Check dependencies are working ok + public function test_dependency_empty_locked_by_permission_child_is_not_unlocked() { + // Check dependencies are working ok. $bs1 = new mock_backup_setting('test1', base_setting::IS_INTEGER, 2); $bs1->set_level(1); $bs2 = new mock_backup_setting('test2', base_setting::IS_INTEGER, 2); - $bs2->set_level(1); // Same level *must* work + $bs2->set_level(1); // Same level *must* work. $bs1->add_dependency($bs2, setting_dependency::DISABLED_EMPTY); $bs1->set_status(base_setting::LOCKED_BY_PERMISSION); @@ -318,11 +318,11 @@ class backp_settings_testcase extends basic_testcase { * Test that locked and unlocked states on dependent backup settings at the same level * correctly do flow from the parent to the child setting when the setting is locked by config. */ - function test_dependency_not_empty_locked_by_config_parent_is_unlocked() { + public function test_dependency_not_empty_locked_by_config_parent_is_unlocked() { $bs1 = new mock_backup_setting('test1', base_setting::IS_INTEGER, 0); $bs1->set_level(1); $bs2 = new mock_backup_setting('test2', base_setting::IS_INTEGER, 0); - $bs2->set_level(1); // Same level *must* work + $bs2->set_level(1); // Same level *must* work. $bs1->add_dependency($bs2, setting_dependency::DISABLED_NOT_EMPTY); $bs1->set_status(base_setting::LOCKED_BY_CONFIG); @@ -337,7 +337,7 @@ class backp_settings_testcase extends basic_testcase { /** * test backup_setting class */ - function test_backup_setting() { + public function test_backup_setting() { // Instantiate backup_setting class and set level $bs = new mock_backup_setting('test', base_setting::IS_INTEGER, null); $bs->set_level(1); @@ -384,7 +384,7 @@ class backp_settings_testcase extends basic_testcase { /** * test activity_backup_setting class */ - function test_activity_backup_setting() { + public function test_activity_backup_setting() { $bs = new mock_activity_backup_setting('test', base_setting::IS_INTEGER, null); $this->assertEquals($bs->get_level(), backup_setting::ACTIVITY_LEVEL); @@ -399,7 +399,7 @@ class backp_settings_testcase extends basic_testcase { /** * test section_backup_setting class */ - function test_section_backup_setting() { + public function test_section_backup_setting() { $bs = new mock_section_backup_setting('test', base_setting::IS_INTEGER, null); $this->assertEquals($bs->get_level(), backup_setting::SECTION_LEVEL); @@ -414,7 +414,7 @@ class backp_settings_testcase extends basic_testcase { /** * test course_backup_setting class */ - function test_course_backup_setting() { + public function test_course_backup_setting() { $bs = new mock_course_backup_setting('test', base_setting::IS_INTEGER, null); $this->assertEquals($bs->get_level(), backup_setting::COURSE_LEVEL);