MDL-60851 backup: coding style fixes

This commit is contained in:
Damyon Wiese 2017-11-20 14:24:09 +08:00
parent 9462d6f1c8
commit 4c775e4618
2 changed files with 81 additions and 73 deletions

View file

@ -1,5 +1,4 @@
<?php <?php
// This file is part of Moodle - http://moodle.org/ // This file is part of Moodle - http://moodle.org/
// //
// Moodle is free software: you can redistribute it and/or modify // Moodle is free software: you can redistribute it and/or modify
@ -82,7 +81,7 @@ abstract class setting_dependency {
* Destroy all circular references. It helps PHP 5.2 a lot! * Destroy all circular references. It helps PHP 5.2 a lot!
*/ */
public function destroy() { public function destroy() {
// No need to destroy anything recursively here, direct reset // No need to destroy anything recursively here, direct reset.
$this->setting = null; $this->setting = null;
$this->dependentsetting = null; $this->dependentsetting = null;
} }
@ -94,16 +93,19 @@ abstract class setting_dependency {
* @return bool * @return bool
*/ */
final public function process_change($changetype, $oldvalue) { final public function process_change($changetype, $oldvalue) {
// Check the type of change requested // Check the type of change requested.
switch ($changetype) { switch ($changetype) {
// Process a status change // Process a status change.
case base_setting::CHANGED_STATUS: return $this->process_status_change($oldvalue); case base_setting::CHANGED_STATUS:
// Process a visibility change return $this->process_status_change($oldvalue);
case base_setting::CHANGED_VISIBILITY: return $this->process_visibility_change($oldvalue); // Process a visibility change.
// Process a value change case base_setting::CHANGED_VISIBILITY:
case base_setting::CHANGED_VALUE: return $this->process_value_change($oldvalue); 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'); throw new backup_ui_exception('unknownchangetype');
} }
/** /**
@ -112,11 +114,11 @@ abstract class setting_dependency {
* @return bool * @return bool
*/ */
protected function process_visibility_change($oldvisibility) { 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(); $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()); $this->dependentsetting->set_visibility($this->setting->get_visibility());
// Return true if it changed // Return true if it changed.
return ($prevalue != $this->dependentsetting->get_visibility()); 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) { public function __construct(base_setting $setting, base_setting $dependentsetting, $value, $defaultvalue = false) {
parent::__construct($setting, $dependentsetting, $defaultvalue); 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. * 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() { public function is_locked() {
// If the setting is locked or the dependent setting should be locked then return true. // 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; return true;
} }
// Else the dependent setting is not locked by this setting_dependency. // 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); $this->dependentsetting->set_value($this->defaultvalue);
} }
} else if ($this->dependentsetting->get_status() == base_setting::LOCKED_BY_HIERARCHY) { } 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); $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()); return ($prevalue != $this->dependentsetting->get_value());
} }
/** /**
@ -235,17 +238,18 @@ class setting_dependency_disabledif_equals extends setting_dependency {
* @return bool * @return bool
*/ */
protected function process_status_change($oldstatus) { protected function process_status_change($oldstatus) {
// Store the dependent status // Store the dependent status.
$prevalue = $this->dependentsetting->get_status(); $prevalue = $this->dependentsetting->get_status();
// Store the current status // Store the current status.
$currentstatus = $this->setting->get_status(); $currentstatus = $this->setting->get_status();
if ($currentstatus == base_setting::NOT_LOCKED) { 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. // Dependency has changes, is not fine, unlock the dependent setting.
$this->dependentsetting->set_status(base_setting::NOT_LOCKED); $this->dependentsetting->set_status(base_setting::NOT_LOCKED);
} }
} else { } 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); $this->dependentsetting->set_status(base_setting::LOCKED_BY_HIERARCHY);
} }
// Return true if the dependent setting has changed. // 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 * @return bool True if there were changes
*/ */
public function enforce() { public function enforce() {
// This will be set to true if ANYTHING changes // This will be set to true if ANYTHING changes.
$changes = false; $changes = false;
// First process any value changes // First process any value changes.
if ($this->process_value_change($this->setting->get_value())) { if ($this->process_value_change($this->setting->get_value())) {
$changes = true; $changes = true;
} }
// Second process any status changes // Second process any status changes.
if ($this->process_status_change($this->setting->get_status())) { if ($this->process_status_change($this->setting->get_status())) {
$changes = true; $changes = true;
} }
// Finally process visibility changes // Finally process visibility changes.
if ($this->process_visibility_change($this->setting->get_visibility())) { if ($this->process_visibility_change($this->setting->get_visibility())) {
$changes = true; $changes = true;
} }
@ -279,10 +283,10 @@ class setting_dependency_disabledif_equals extends setting_dependency {
*/ */
public function get_moodleform_properties() { public function get_moodleform_properties() {
return array( return array(
'setting'=>$this->dependentsetting->get_ui_name(), 'setting' => $this->dependentsetting->get_ui_name(),
'dependenton'=>$this->setting->get_ui_name(), 'dependenton' => $this->setting->get_ui_name(),
'condition'=>'eq', 'condition' => 'eq',
'value'=>$this->value '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 * A dependency that disables the secondary setting if the primary setting is
* not equal to the provided value * not equal to the provided value
* *
* @copyright 2011 Darko Miletic <dmiletic@moodlerooms.com> * @copyright 2011 Darko Miletic <dmiletic@moodlerooms.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/ */
class setting_dependency_disabledif_not_equals extends setting_dependency_disabledif_equals { class setting_dependency_disabledif_not_equals extends setting_dependency_disabledif_equals {
/** /**
@ -323,14 +327,17 @@ class setting_dependency_disabledif_not_equals extends setting_dependency_disabl
*/ */
public function get_moodleform_properties() { public function get_moodleform_properties() {
return array( return array(
'setting'=>$this->dependentsetting->get_ui_name(), 'setting' => $this->dependentsetting->get_ui_name(),
'dependenton'=>$this->setting->get_ui_name(), 'dependenton' => $this->setting->get_ui_name(),
'condition'=>'notequal', 'condition' => 'notequal',
'value'=>$this->value 'value' => $this->value
); );
} }
} }
/**
* Disable if a value is in a list.
*/
class setting_dependency_disabledif_in_array extends setting_dependency_disabledif_equals { 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() { public function get_moodleform_properties() {
return array( return array(
'setting'=>$this->dependentsetting->get_ui_name(), 'setting' => $this->dependentsetting->get_ui_name(),
'dependenton'=>$this->setting->get_ui_name(), 'dependenton' => $this->setting->get_ui_name(),
'condition'=>'eq', 'condition' => 'eq',
'value'=>$this->value '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 { 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() { public function get_moodleform_properties() {
return array( return array(
'setting'=>$this->dependentsetting->get_ui_name(), 'setting' => $this->dependentsetting->get_ui_name(),
'dependenton'=>$this->setting->get_ui_name(), 'dependenton' => $this->setting->get_ui_name(),
'condition'=>'checked' 'condition' => 'checked'
); );
} }
} }
@ -407,9 +415,9 @@ class setting_dependency_disabledif_not_checked extends setting_dependency_disab
*/ */
public function get_moodleform_properties() { public function get_moodleform_properties() {
return array( return array(
'setting'=>$this->dependentsetting->get_ui_name(), 'setting' => $this->dependentsetting->get_ui_name(),
'dependenton'=>$this->setting->get_ui_name(), 'dependenton' => $this->setting->get_ui_name(),
'condition'=>'notchecked' 'condition' => 'notchecked'
); );
} }
} }
@ -443,10 +451,10 @@ class setting_dependency_disabledif_not_empty extends setting_dependency_disable
*/ */
public function get_moodleform_properties() { public function get_moodleform_properties() {
return array( return array(
'setting'=>$this->dependentsetting->get_ui_name(), 'setting' => $this->dependentsetting->get_ui_name(),
'dependenton'=>$this->setting->get_ui_name(), 'dependenton' => $this->setting->get_ui_name(),
'condition'=>'notequal', 'condition' => 'notequal',
'value'=>'' 'value' => ''
); );
} }
} }
@ -480,10 +488,10 @@ class setting_dependency_disabledif_empty extends setting_dependency_disabledif_
*/ */
public function get_moodleform_properties() { public function get_moodleform_properties() {
return array( return array(
'setting'=>$this->dependentsetting->get_ui_name(), 'setting' => $this->dependentsetting->get_ui_name(),
'dependenton'=>$this->setting->get_ui_name(), 'dependenton' => $this->setting->get_ui_name(),
'condition'=>'notequal', 'condition' => 'notequal',
'value'=>'' 'value' => ''
); );
} }
} }

View file

@ -45,7 +45,7 @@ class backp_settings_testcase extends basic_testcase {
/** /**
* test base_setting class * test base_setting class
*/ */
function test_base_setting() { public function test_base_setting() {
// Instantiate base_setting and check everything // Instantiate base_setting and check everything
$bs = new mock_base_setting('test', base_setting::IS_BOOLEAN); $bs = new mock_base_setting('test', base_setting::IS_BOOLEAN);
$this->assertTrue($bs instanceof base_setting); $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 * 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. * 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() { public function test_dependency_empty_locked_by_permission_child_is_not_unlocked() {
// Check dependencies are working ok // Check dependencies are working ok.
$bs1 = new mock_backup_setting('test1', base_setting::IS_INTEGER, 2); $bs1 = new mock_backup_setting('test1', base_setting::IS_INTEGER, 2);
$bs1->set_level(1); $bs1->set_level(1);
$bs2 = new mock_backup_setting('test2', base_setting::IS_INTEGER, 2); $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->add_dependency($bs2, setting_dependency::DISABLED_EMPTY);
$bs1->set_status(base_setting::LOCKED_BY_PERMISSION); $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 * 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. * 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 = new mock_backup_setting('test1', base_setting::IS_INTEGER, 0);
$bs1->set_level(1); $bs1->set_level(1);
$bs2 = new mock_backup_setting('test2', base_setting::IS_INTEGER, 0); $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->add_dependency($bs2, setting_dependency::DISABLED_NOT_EMPTY);
$bs1->set_status(base_setting::LOCKED_BY_CONFIG); $bs1->set_status(base_setting::LOCKED_BY_CONFIG);
@ -337,7 +337,7 @@ class backp_settings_testcase extends basic_testcase {
/** /**
* test backup_setting class * test backup_setting class
*/ */
function test_backup_setting() { public function test_backup_setting() {
// Instantiate backup_setting class and set level // Instantiate backup_setting class and set level
$bs = new mock_backup_setting('test', base_setting::IS_INTEGER, null); $bs = new mock_backup_setting('test', base_setting::IS_INTEGER, null);
$bs->set_level(1); $bs->set_level(1);
@ -384,7 +384,7 @@ class backp_settings_testcase extends basic_testcase {
/** /**
* test activity_backup_setting class * 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); $bs = new mock_activity_backup_setting('test', base_setting::IS_INTEGER, null);
$this->assertEquals($bs->get_level(), backup_setting::ACTIVITY_LEVEL); $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 * 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); $bs = new mock_section_backup_setting('test', base_setting::IS_INTEGER, null);
$this->assertEquals($bs->get_level(), backup_setting::SECTION_LEVEL); $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 * 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); $bs = new mock_course_backup_setting('test', base_setting::IS_INTEGER, null);
$this->assertEquals($bs->get_level(), backup_setting::COURSE_LEVEL); $this->assertEquals($bs->get_level(), backup_setting::COURSE_LEVEL);