From 25ec5f84acecedfe388eb8e279997fd687e0a206 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 13 May 2024 16:25:30 -0400 Subject: [PATCH 1/7] prep refactor --- src/Fields/FieldTransformer.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Fields/FieldTransformer.php b/src/Fields/FieldTransformer.php index 76a0cc116b..28eea44a8f 100644 --- a/src/Fields/FieldTransformer.php +++ b/src/Fields/FieldTransformer.php @@ -28,12 +28,11 @@ private static function inlineTabField(array $submitted) { $fieldtype = FieldtypeRepository::find($submitted['fieldtype'] ?? $submitted['config']['type']); - $defaultConfig = Field::commonFieldOptions()->all() - ->merge($fieldtype->configFields()->all()) - ->map->defaultValue()->filter(); + $fields = Field::commonFieldOptions()->all() + ->merge($fieldtype->configFields()->all()); $field = collect($submitted['config']) - ->reject(function ($value, $key) use ($defaultConfig) { + ->reject(function ($value, $key) use ($fields) { if (in_array($key, ['isNew', 'icon', 'duplicate'])) { return true; } @@ -46,7 +45,11 @@ private static function inlineTabField(array $submitted) return true; } - return $defaultConfig->has($key) && $defaultConfig->get($key) === $value; + if (! $field = $fields->get($key)) { + return false; + } + + return $field->defaultValue() === $value; }) ->map(function ($value, $key) { if ($key === 'sets') { From 8ee8464f8eef23b5710f477636337a2392653e54 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 13 May 2024 17:42:25 -0400 Subject: [PATCH 2/7] make test more generic - define the fieldtype inline rather than relying on the "text" fieldtype --- tests/Fields/FieldTransformerTest.php | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/Fields/FieldTransformerTest.php b/tests/Fields/FieldTransformerTest.php index c911b95d93..20deacf534 100644 --- a/tests/Fields/FieldTransformerTest.php +++ b/tests/Fields/FieldTransformerTest.php @@ -3,6 +3,7 @@ namespace Tests\Fields; use Statamic\Fields\FieldTransformer; +use Statamic\Fields\Fieldtype; use Tests\TestCase; class FieldTransformerTest extends TestCase @@ -68,15 +69,30 @@ public function it_normalizes_required_validation() /** @test */ public function it_removes_redundant_config_options() { + $fieldtype = new class extends Fieldtype + { + protected static $handle = 'test'; + + public function configFieldItems(): array + { + return [ + 'input_type' => ['type' => 'text', 'default' => 'text'], + 'character_limit' => ['type' => 'integer', 'default' => null], + 'max_items' => ['type' => 'integer', 'default' => 1, 'force_in_config' => true], + ]; + } + }; + $fieldtype::register(); + $fromVue = FieldTransformer::fromVue([ - 'fieldtype' => 'text', + 'fieldtype' => 'test', 'handle' => 'test', 'type' => 'inline', 'config' => [ // Fieldtype config options 'input_type' => 'text', // The default. - 'icon' => 'text', // The default. 'character_limit' => 100, // This one has been changed. + 'max_items' => 1, // Even though it matches the default, it has been flagged to be explicitly kept. 'foo' => 'bar', // Manually added by user. // Common field options @@ -88,6 +104,7 @@ public function it_removes_redundant_config_options() $this->assertEquals([ 'character_limit' => 100, 'listable' => true, + 'max_items' => 1, 'foo' => 'bar', ], $fromVue['field']); } From fa0053a54657f9cb582097281bdefc2c972e1227 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 13 May 2024 17:43:26 -0400 Subject: [PATCH 3/7] add explicit tests for the 3 issues --- tests/Fields/FieldTransformerTest.php | 75 +++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/tests/Fields/FieldTransformerTest.php b/tests/Fields/FieldTransformerTest.php index 20deacf534..4d2f9d98fe 100644 --- a/tests/Fields/FieldTransformerTest.php +++ b/tests/Fields/FieldTransformerTest.php @@ -2,12 +2,16 @@ namespace Tests\Fields; +use Statamic\Facades\AssetContainer; use Statamic\Fields\FieldTransformer; use Statamic\Fields\Fieldtype; +use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; class FieldTransformerTest extends TestCase { + use PreventSavingStacheItemsToDisk; + protected function configToVue($config) { return FieldTransformer::toVue(['handle' => 'test', 'field' => $config])['config']; @@ -291,4 +295,75 @@ public function blank_instructions_and_icon_are_removed_from_set_groups() ], ], $fromVue); } + + /** + * @test + * + * @see https://github.com/statamic/cms/issues/10056 + */ + public function it_doesnt_remove_max_items_from_form_fieldtype() + { + $fromVue = FieldTransformer::fromVue([ + 'fieldtype' => 'form', + 'handle' => 'test', + 'type' => 'inline', + 'config' => [ + 'foo' => 'bar', + 'max_items' => 1, + ], + ]); + + $this->assertEquals([ + 'foo' => 'bar', + 'max_items' => 1, + ], $fromVue['field']); + } + + /** + * @test + * + * @see https://github.com/statamic/cms/issues/10050 + */ + public function it_ensures_the_asset_container_is_saved_on_the_assets_fieldtype() + { + AssetContainer::make('test')->save(); + + $fromVue = FieldTransformer::fromVue([ + 'fieldtype' => 'assets', + 'handle' => 'test', + 'type' => 'inline', + 'config' => [ + 'foo' => 'bar', + 'container' => 'test', + ], + ]); + + $this->assertEquals([ + 'foo' => 'bar', + 'container' => 'test', + ], $fromVue['field']); + } + + /** + * @test + * + * @see https://github.com/statamic/cms/issues/10040 + */ + public function it_saves_a_toggle_as_false_where_the_default_is_true() + { + $fromVue = FieldTransformer::fromVue([ + 'fieldtype' => 'text', + 'handle' => 'test', + 'type' => 'inline', + 'config' => [ + 'replicator_preview' => false, + 'duplicate' => false, + ], + ]); + + $this->assertEquals([ + 'replicator_preview' => false, + 'duplicate' => false, + ], $fromVue['field']); + } } From b478791b1caa067ab54b563df9c591ad2fd65700 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 13 May 2024 17:45:18 -0400 Subject: [PATCH 4/7] "duplicate" shouldn't have been moved to this condition ... before the other PR, the logic was to remove "duplicate" if it was true. this is now saying to always remove it, which is incorrect. partly solves #10040 --- src/Fields/FieldTransformer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Fields/FieldTransformer.php b/src/Fields/FieldTransformer.php index 28eea44a8f..246cb62b8b 100644 --- a/src/Fields/FieldTransformer.php +++ b/src/Fields/FieldTransformer.php @@ -33,7 +33,7 @@ private static function inlineTabField(array $submitted) $field = collect($submitted['config']) ->reject(function ($value, $key) use ($fields) { - if (in_array($key, ['isNew', 'icon', 'duplicate'])) { + if (in_array($key, ['isNew', 'icon'])) { return true; } From a0860fed73afba9251fe5488824f7e141fc3b4ad Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 13 May 2024 17:47:21 -0400 Subject: [PATCH 5/7] add a way to prevent fields from being stripped out at all ... a force_in_config option will allow configs to avoid getting stripped out. it only applies to fields when used as configs. --- src/Fields/ConfigField.php | 5 +++++ src/Fields/Field.php | 2 +- src/Fields/FieldTransformer.php | 4 ++++ src/Fieldtypes/Assets/Assets.php | 1 + src/Forms/Fieldtype.php | 1 + 5 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Fields/ConfigField.php b/src/Fields/ConfigField.php index dd51455f8f..4f59c87b6a 100644 --- a/src/Fields/ConfigField.php +++ b/src/Fields/ConfigField.php @@ -12,4 +12,9 @@ public function preProcess() return $this->newInstance()->setValue($value); } + + public function mustRemainInConfig(): bool + { + return $this->get('force_in_config') === true; + } } diff --git a/src/Fields/Field.php b/src/Fields/Field.php index d7685ffae0..397f374aab 100644 --- a/src/Fields/Field.php +++ b/src/Fields/Field.php @@ -551,6 +551,6 @@ public static function commonFieldOptions(): Fields ], ])->map(fn ($field, $handle) => compact('handle', 'field'))->values()->all(); - return new Fields($fields); + return new ConfigFields($fields); } } diff --git a/src/Fields/FieldTransformer.php b/src/Fields/FieldTransformer.php index 246cb62b8b..62db3cada9 100644 --- a/src/Fields/FieldTransformer.php +++ b/src/Fields/FieldTransformer.php @@ -49,6 +49,10 @@ private static function inlineTabField(array $submitted) return false; } + if ($field->mustRemainInConfig()) { + return false; + } + return $field->defaultValue() === $value; }) ->map(function ($value, $key) { diff --git a/src/Fieldtypes/Assets/Assets.php b/src/Fieldtypes/Assets/Assets.php index 4fc4719630..8cda4fbe11 100644 --- a/src/Fieldtypes/Assets/Assets.php +++ b/src/Fieldtypes/Assets/Assets.php @@ -56,6 +56,7 @@ protected function configFieldItems(): array 'mode' => 'select', 'required' => true, 'default' => AssetContainer::all()->count() == 1 ? AssetContainer::all()->first()->handle() : null, + 'force_in_config' => true, ], 'folder' => [ 'display' => __('Folder'), diff --git a/src/Forms/Fieldtype.php b/src/Forms/Fieldtype.php index 384f8d5dac..be55e589fc 100644 --- a/src/Forms/Fieldtype.php +++ b/src/Forms/Fieldtype.php @@ -31,6 +31,7 @@ protected function configFieldItems(): array 'display' => __('Max Items'), 'default' => 1, 'instructions' => __('statamic::fieldtypes.form.config.max_items'), + 'force_in_config' => true, ], 'mode' => [ 'display' => __('UI Mode'), From 1289d32502d996195b93b2436776bb6f3a979c97 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 13 May 2024 17:49:43 -0400 Subject: [PATCH 6/7] maintain literal falses. strip out nulls, empty arrays, and empty strings. --- src/Fields/FieldTransformer.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Fields/FieldTransformer.php b/src/Fields/FieldTransformer.php index 62db3cada9..35360f4965 100644 --- a/src/Fields/FieldTransformer.php +++ b/src/Fields/FieldTransformer.php @@ -91,7 +91,6 @@ private static function inlineTabField(array $submitted) return $value; }) - ->filter() ->sortBy(function ($value, $key) { // Push sets & fields to the end of the config. if ($key === 'sets' || $key === 'fields') { @@ -104,7 +103,7 @@ private static function inlineTabField(array $submitted) return array_filter([ 'handle' => $submitted['handle'], - 'field' => $field, + 'field' => Arr::removeNullValues($field), ]); } From 024057c8f5309fd2191ad6d309716d700d0d2c73 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 13 May 2024 17:49:11 -0400 Subject: [PATCH 7/7] make hide_display part of the common options ... it was only being added on the edit route, but should have also been there for the update route. the reason it didnt show an issue was because all falsey/null values were being stripped out anyway. --- src/Fields/Field.php | 4 ++++ src/Http/Controllers/CP/Fields/FieldsController.php | 4 +--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Fields/Field.php b/src/Fields/Field.php index 397f374aab..04895d1624 100644 --- a/src/Fields/Field.php +++ b/src/Fields/Field.php @@ -477,6 +477,10 @@ public static function commonFieldOptions(): Fields 'instructions' => __('statamic::messages.fields_display_instructions'), 'type' => 'field_display', ], + 'hide_display' => [ + 'type' => 'toggle', + 'visibility' => 'hidden', + ], 'handle' => [ 'display' => __('Handle'), 'instructions' => __('statamic::messages.fields_handle_instructions'), diff --git a/src/Http/Controllers/CP/Fields/FieldsController.php b/src/Http/Controllers/CP/Fields/FieldsController.php index 95f454a0f7..b594742296 100644 --- a/src/Http/Controllers/CP/Fields/FieldsController.php +++ b/src/Http/Controllers/CP/Fields/FieldsController.php @@ -32,9 +32,7 @@ public function edit(Request $request) $fieldtype = FieldtypeRepository::find($request->type); - $blueprint = $this - ->blueprint($fieldtype->configBlueprint()) - ->ensureField('hide_display', ['type' => 'toggle', 'visibility' => 'hidden']); + $blueprint = $this->blueprint($fieldtype->configBlueprint()); $fields = $blueprint ->fields()