From 3e605a554fae2f1c8704cdb1a0a28a7757c0927e 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). --- public/customfield/classes/data.php | 45 ++++++++++++++----- .../customfield/classes/data_controller.php | 20 ++------- public/lib/classes/persistent.php | 2 +- 3 files changed, 37 insertions(+), 30 deletions(-) diff --git a/public/customfield/classes/data.php b/public/customfield/classes/data.php index 99d5a205fac..da1fabf9e5a 100644 --- a/public/customfield/classes/data.php +++ b/public/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 */ @@ -121,4 +111,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/public/customfield/classes/data_controller.php b/public/customfield/classes/data_controller.php index ead81c30ff8..77d4b01015d 100644 --- a/public/customfield/classes/data_controller.php +++ b/public/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 */ @@ -212,15 +202,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); // Set component, area and itemid from the handler. diff --git a/public/lib/classes/persistent.php b/public/lib/classes/persistent.php index 8308cd7988a..5ebe0076055 100644 --- a/public/lib/classes/persistent.php +++ b/public/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.