From f9f4c49fd250aab54572c95ea259d0c5b6765613 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Thu, 17 Jul 2025 22:16:06 +0100 Subject: [PATCH] MDL-86063 customfield: internally validate numeric data in persistent. Move previous validation from the data controller, added in 89dbe63d, into the persistent class itself so that it can internally validate itself rather than relying on callers. This resolves problems with empty/null numeric fields contained within course backups (e.g. during course copy). --- customfield/classes/data.php | 45 ++++++++++++++++++------- customfield/classes/data_controller.php | 20 ++--------- lib/classes/persistent.php | 2 +- 3 files changed, 37 insertions(+), 30 deletions(-) diff --git a/customfield/classes/data.php b/customfield/classes/data.php index 677bf24485b..d978344b849 100644 --- a/customfield/classes/data.php +++ b/customfield/classes/data.php @@ -14,24 +14,14 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Data persistent class - * - * @package core_customfield - * @copyright 2018 Toni Barbera - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - namespace core_customfield; use core\persistent; -defined('MOODLE_INTERNAL') || die; - /** - * Class data + * Data persistent class * - * @package core_customfield + * @package core_customfield * @copyright 2018 Toni Barbera * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ @@ -110,4 +100,35 @@ class data extends persistent { ); } + /** + * For integer data field, persistent won't allow empty string, swap for null + * + * @param string|null $value + * @return self + */ + protected function set_intvalue(?string $value): self { + $value = (string) $value === '' ? null : (int) $value; + return $this->raw_set('intvalue', $value); + } + + /** + * For decimal data field, persistent won't allow empty string, swap for null + * + * @param string|null $value + * @return self + */ + protected function set_decvalue(?string $value): self { + $value = (string) $value === '' ? null : (float) $value; + return $this->raw_set('decvalue', $value); + } + + /** + * Ensure value field observes non-nullability + * + * @param string|null $value + * @return self + */ + protected function set_value(?string $value): self { + return $this->raw_set('value', (string) $value); + } } diff --git a/customfield/classes/data_controller.php b/customfield/classes/data_controller.php index 905aaec96d9..de01354d4ed 100644 --- a/customfield/classes/data_controller.php +++ b/customfield/classes/data_controller.php @@ -14,21 +14,11 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Customfield component data controller abstract class - * - * @package core_customfield - * @copyright 2018 Toni Barbera - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - namespace core_customfield; use backup_nested_element; use core_customfield\output\field_data; -defined('MOODLE_INTERNAL') || die; - /** * Base class for custom fields data controllers * @@ -38,7 +28,7 @@ defined('MOODLE_INTERNAL') || die; * Custom field plugins must define a class * \{pluginname}\data_controller extends \core_customfield\data_controller * - * @package core_customfield + * @package core_customfield * @copyright 2018 Toni Barbera * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ @@ -200,15 +190,11 @@ abstract class data_controller { if (!property_exists($datanew, $elementname)) { return; } - $datafieldvalue = $value = $datanew->{$elementname}; - // For numeric datafields, persistent won't allow empty string, swap for null. $datafield = $this->datafield(); - if ($datafield === 'intvalue' || $datafield === 'decvalue') { - $datafieldvalue = $datafieldvalue === '' ? null : $datafieldvalue; - } + $value = $datanew->{$elementname}; - $this->data->set($datafield, $datafieldvalue); + $this->data->set($datafield, $value); $this->data->set('value', $value); $this->save(); } diff --git a/lib/classes/persistent.php b/lib/classes/persistent.php index 8308cd7988a..5ebe0076055 100644 --- a/lib/classes/persistent.php +++ b/lib/classes/persistent.php @@ -95,7 +95,7 @@ abstract class persistent { * Data setter. * * This is the main setter for all the properties. Developers can implement their own setters (set_propertyname) - * and they will be called by this function. Custom setters should call internal_set() to finally set the value. + * and they will be called by this function. Custom setters should call {@see raw_set} to finally set the value. * Internally this is not used {@link self::to_record()} or * {@link self::from_record()} because the data is not expected to be validated or changed when reading/writing * raw records from the DB.