From 7fda52b2cdc55e8a7f635a8b50a5b211011eba00 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Mon, 18 Jun 2018 11:51:52 +1200 Subject: [PATCH] Add a MERGE_AS_SUBMITTED_VALUE flag for Form::loadDataFrom --- src/Forms/Form.php | 22 ++++++++--- tests/php/Forms/FormTest.php | 72 +++++++++++++++++++----------------- 2 files changed, 55 insertions(+), 39 deletions(-) diff --git a/src/Forms/Form.php b/src/Forms/Form.php index fe3fcbd92..4ce24d70f 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -1317,10 +1317,11 @@ class Form extends ViewableData implements HasRequestHandler return $result; } - const MERGE_DEFAULT = 0b0000; - const MERGE_CLEAR_MISSING = 0b0001; - const MERGE_IGNORE_FALSEISH = 0b0010; - const MERGE_AS_INTERNAL_VALUE = 0b0100; + const MERGE_DEFAULT = 0b0000; + const MERGE_CLEAR_MISSING = 0b0001; + const MERGE_IGNORE_FALSEISH = 0b0010; + const MERGE_AS_INTERNAL_VALUE = 0b0100; + const MERGE_AS_SUBMITTED_VALUE = 0b1000; /** * Load data from the given DataObject or array. @@ -1363,7 +1364,8 @@ class Form extends ViewableData implements HasRequestHandler * * Passing MERGE_AS_INTERNAL_VALUE forces the data to be parsed using the internal representation of the matching * form field. This is helpful if you are loading an array of values retrieved from `Form::getData()` and you - * do not want them parsed as submitted data. + * do not want them parsed as submitted data. MERGE_AS_SUBMITTED_VALUE does the opposite and forces the data to be + * parsed as it would be submitted from a form. * * For backwards compatibility reasons, this parameter can also be set to === true, which is the same as passing * CLEAR_MISSING @@ -1395,6 +1397,14 @@ class Form extends ViewableData implements HasRequestHandler $submitted = false; } + // Using the `MERGE_AS_INTERNAL_VALUE` or `MERGE_AS_SUBMITTED_VALUE` flags users can explicitly specify which + // `setValue` method to use. + if (($mergeStrategy & self::MERGE_AS_INTERNAL_VALUE) == self::MERGE_AS_INTERNAL_VALUE) { + $submitted = false; + } else if (($mergeStrategy & self::MERGE_AS_SUBMITTED_VALUE) == self::MERGE_AS_SUBMITTED_VALUE) { + $submitted = true; + } + // dont include fields without data $dataFields = $this->Fields()->dataFields(); if (!$dataFields) { @@ -1474,7 +1484,7 @@ class Form extends ViewableData implements HasRequestHandler // pass original data as well so composite fields can act on the additional information if ($setValue) { - if ($submitted && ($mergeStrategy & self::MERGE_AS_INTERNAL_VALUE) != self::MERGE_AS_INTERNAL_VALUE) { + if ($submitted) { $field->setSubmittedValue($val, $data); } else { $field->setValue($val, $data); diff --git a/tests/php/Forms/FormTest.php b/tests/php/Forms/FormTest.php index 39bc95cf1..8b9a2948b 100644 --- a/tests/php/Forms/FormTest.php +++ b/tests/php/Forms/FormTest.php @@ -31,6 +31,7 @@ use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\NullSecurityToken; use SilverStripe\Security\RandomGenerator; use SilverStripe\Security\SecurityToken; +use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; /** @@ -279,21 +280,8 @@ class FormTest extends FunctionalTest 'SomeTimeField' => '17:28:05' ]; - // Test loading our data with the MERGE_FORCE_SET_VALUE - $form = new Form( - Controller::curr(), - 'Form', - new FieldList( - $dateField = DatetimeField::create('SomeDateTimeField') - ->setHTML5(false) - ->setDatetimeFormat("EEE, MMM d, ''yy HH:mm:ss"), - $timeField = TimeField::create('SomeTimeField') - ->setHTML5(false) - ->setTimeFormat("hh 'o''clock' a mm ss") // Swatch Internet Time format - ), - new FieldList() - ); - + // Test loading our data with the MERGE_AS_INTERNAL_VALUE + $form = $this->getStubFormWithWeirdValueFormat(); $form->loadDataFrom($dataInInternalValue, Form::MERGE_AS_INTERNAL_VALUE); $this->assertEquals( @@ -301,22 +289,16 @@ class FormTest extends FunctionalTest $form->getData() ); - - // Test loading our data without the MERGE_FORCE_SET_VALUE - $form = new Form( - Controller::curr(), - 'Form', - new FieldList( - $dateField = DatetimeField::create('SomeDateTimeField') - ->setHTML5(false) - ->setDatetimeFormat("EEE, MMM d, ''yy HH:mm:ss"), - $timeField = TimeField::create('SomeTimeField') - ->setHTML5(false) - ->setTimeFormat("hh 'o''clock' a mm ss") // Swatch Internet Time format - ), - new FieldList() + // Test loading our data with the MERGE_AS_SUBMITTED_VALUE and an data passed as an object + $form = $this->getStubFormWithWeirdValueFormat(); + $form->loadDataFrom(ArrayData::create($dataInSubmittedValue), Form::MERGE_AS_SUBMITTED_VALUE); + $this->assertEquals( + $dataInInternalValue, + $form->getData() ); + // Test loading our data without the MERGE_AS_INTERNAL_VALUE and without MERGE_AS_SUBMITTED_VALUE + $form = $this->getStubFormWithWeirdValueFormat(); $form->loadDataFrom($dataInSubmittedValue); $this->assertEquals( @@ -339,10 +321,10 @@ class FormTest extends FunctionalTest $form->loadDataFrom( array( 'Players' => array( - 14, - 18, - 22 - ), + 14, + 18, + 22 + ), ) ); $form->saveInto($object); @@ -1101,4 +1083,28 @@ class FormTest extends FunctionalTest new FieldList() ); } + + /** + * Some fields handle submitted values differently from their internal values. This forms contains 2 such fields + * * a SomeDateTimeField that expect a date such as `Fri, Jun 15, '18 17:28:05`, + * * a SomeTimeField that expects it's time as `05 o'clock PM 28 05` + * + * @return Form + */ + protected function getStubFormWithWeirdValueFormat() + { + return new Form( + Controller::curr(), + 'Form', + new FieldList( + $dateField = DatetimeField::create('SomeDateTimeField') + ->setHTML5(false) + ->setDatetimeFormat("EEE, MMM d, ''yy HH:mm:ss"), + $timeField = TimeField::create('SomeTimeField') + ->setHTML5(false) + ->setTimeFormat("hh 'o''clock' a mm ss") // Swatch Internet Time format + ), + new FieldList() + ); + } }