Streamline and simplify

Author: jackmcdade

src/Fields/Fieldset.php

@@ -63,14 +63,25 @@ public function initialPath($path = null)
         return $this;
     }
 
+    /**
+     * Canonical storage: either top-level `fields` (flat) or non-empty `sections`, never both.
+     * Hand-edited YAML or mixed input is normalized on save.
+     */
     public function setContents(array $contents)
     {
-        $contents['fields'] = $this->normalizeFields(Arr::get($contents, 'fields', []));
-
         if (array_key_exists('sections', $contents)) {
             $contents['sections'] = $this->normalizeSections(Arr::get($contents, 'sections', []));
         }
 
+        $usesSections = array_key_exists('sections', $contents) && ! empty($contents['sections']);
+
+        if ($usesSections) {
+            unset($contents['fields']);
+        } else {
+            unset($contents['sections']);
+            $contents['fields'] = $this->normalizeFields(Arr::get($contents, 'fields', []));
+        }
+
         $this->contents = $contents;
 
         return $this;

src/Http/Controllers/CP/Fields/FieldsetController.php

@@ -132,38 +132,27 @@ public function update(Request $request, $fieldset)
             'fields' => 'array',
         ]);
 
-        $contents = array_merge($fieldset->contents(), [
-            'title' => $request->title,
-        ]);
+        $base = array_merge(
+            Arr::except($fieldset->contents(), ['fields', 'sections']),
+            ['title' => $request->title],
+        );
 
         if ($request->has('sections')) {
-            $sections = collect($request->sections)->map(function ($section) {
-                return Arr::removeNullValues([
-                    'display' => Arr::get($section, 'display'),
-                    'instructions' => Arr::get($section, 'instructions'),
-                    'collapsible' => ($collapsible = Arr::get($section, 'collapsible')) ?: null,
-                    'collapsed' => ($collapsible && Arr::get($section, 'collapsed')) ?: null,
-                    'fields' => collect(Arr::get($section, 'fields', []))->map(function ($field) {
-                        return FieldTransformer::fromVue($field);
-                    })->all(),
-                ]);
-            })->all();
-
-            if ($this->shouldStoreAsFlatFields($sections)) {
-                $contents['fields'] = Arr::get($sections, '0.fields', []);
-                unset($contents['sections']);
-            } else {
-                $contents['sections'] = $sections;
-            }
+            $sections = $this->sectionsFromVueRequest($request->sections);
+
+            $fieldset->setContents(
+                $this->shouldStoreAsFlatFields($sections)
+                    ? $base + ['fields' => Arr::get($sections, '0.fields', [])]
+                    : $base + ['sections' => $sections]
+            );
         } else {
-            $contents['fields'] = collect($request->fields)->map(function ($field) {
-                return FieldTransformer::fromVue($field);
-            })->all();
-            unset($contents['sections']);
+            $fieldset->setContents($base + [
+                'fields' => collect($request->fields)
+                    ->map(fn ($field) => FieldTransformer::fromVue($field))
+                    ->all(),
+            ]);
         }
 
-        $fieldset->setContents($contents);
-
         $fieldset->validateRecursion();
 
         $fieldset->save();
@@ -242,6 +231,21 @@ private function groupKey(Fieldset $fieldset): string
         return __('My Fieldsets');
     }
 
+    private function sectionsFromVueRequest(array $sections): array
+    {
+        return collect($sections)->map(function ($section) {
+            return Arr::removeNullValues([
+                'display' => Arr::get($section, 'display'),
+                'instructions' => Arr::get($section, 'instructions'),
+                'collapsible' => ($collapsible = Arr::get($section, 'collapsible')) ?: null,
+                'collapsed' => ($collapsible && Arr::get($section, 'collapsed')) ?: null,
+                'fields' => collect(Arr::get($section, 'fields', []))
+                    ->map(fn ($field) => FieldTransformer::fromVue($field))
+                    ->all(),
+            ]);
+        })->all();
+    }
+
     private function sectionsToVue(Fieldset $fieldset): array
     {
         $sections = $fieldset->hasSections()

tests/Feature/Fieldsets/UpdateFieldsetTest.php

@@ -160,9 +160,6 @@ public function fieldset_gets_saved_with_sections()
         $this->assertEquals([
             'title' => 'Updated title',
             'foo' => 'bar',
-            'fields' => [
-                ['handle' => 'legacy', 'field' => ['type' => 'text']],
-            ],
             'sections' => [
                 [
                     'display' => 'SEO',
@@ -273,7 +270,6 @@ public function import_section_behavior_is_saved_when_flattening_sections()
 
         $this->assertEquals([
             'title' => 'Updated',
-            'fields' => [],
             'sections' => [
                 [
                     'display' => 'Main',

tests/Fields/FieldsetTest.php

@@ -155,6 +155,46 @@ public function it_gets_fields_from_sections()
         $this->assertEquals(['text', 'textarea'], $fields->map->type()->values()->all());
     }
 
+    #[Test]
+    public function it_drops_empty_sections_when_storing_flat_fields()
+    {
+        $fieldset = new Fieldset;
+
+        $fieldset->setContents([
+            'title' => 'Test',
+            'sections' => [],
+            'fields' => [
+                ['handle' => 'one', 'field' => ['type' => 'text']],
+            ],
+        ]);
+
+        $this->assertArrayNotHasKey('sections', $fieldset->contents());
+        $this->assertEquals('one', $fieldset->fields()->all()->first()->handle());
+    }
+
+    #[Test]
+    public function it_drops_top_level_fields_when_storing_sections()
+    {
+        $fieldset = new Fieldset;
+
+        $fieldset->setContents([
+            'sections' => [
+                [
+                    'display' => 'A',
+                    'fields' => [
+                        ['handle' => 'one', 'field' => ['type' => 'text']],
+                    ],
+                ],
+            ],
+            'fields' => [
+                ['handle' => 'stale', 'field' => ['type' => 'text']],
+            ],
+        ]);
+
+        $this->assertArrayNotHasKey('fields', $fieldset->contents());
+        $this->assertEquals(['one'], $fieldset->fields()->all()->keys()->all());
+    }
+
     #[Test]
     public function it_normalizes_legacy_section_fields_syntax()
     {