From 180791f62adafd7df00a2c1e956979983183b871 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Thu, 13 Aug 2026 09:48:47 +0100 Subject: [PATCH] fix `ensureFieldHasConfig` for fields from imported fieldsets `getTabFields` built its lookup from the raw blueprint contents using `keyBy('handle')`. An `import:` row has no `handle`, so the fields inside an imported fieldset were invisible to the lookup, while `hasFieldInTab` resolves imports and reported them as present. That mismatch made `ensureFieldHasConfig` throw `Undefined array key`. It now resolves `import:` rows into their fields and writes the override into the import's `config` array, keyed by the field's handle within the fieldset, which is the same shape `ensureField` already produces. `removeField` no longer errors on an imported field either. It can't be removed on its own, since the only entry in the contents is the whole fieldset, so it's left in place. Co-Authored-By: Claude Opus 5 --- src/Fields/Blueprint.php | 31 ++++-- tests/Feature/Entries/CreateEntryTest.php | 40 ++++++++ tests/Fields/BlueprintTest.php | 115 ++++++++++++++++++++++ 3 files changed, 179 insertions(+), 7 deletions(-) diff --git a/src/Fields/Blueprint.php b/src/Fields/Blueprint.php index 1ad89742eb0..4591b368ef2 100644 --- a/src/Fields/Blueprint.php +++ b/src/Fields/Blueprint.php @@ -632,6 +632,12 @@ public function removeFieldFromTab($handle, $tab) return $this; } + // A field from an imported fieldset can't be removed on its + // own, since it would take the rest of the fieldset with it. + if (isset($fields[$handle]['import'])) { + return $this; + } + $fieldKey = $fields[$handle]['fieldIndex']; $sectionIndex = $fields[$handle]['sectionIndex']; @@ -644,10 +650,18 @@ public function removeFieldFromTab($handle, $tab) private function getTabFields($tab) { return collect($this->contents['tabs'][$tab]['sections'])->flatMap(function ($section, $sectionIndex) { - return collect($section['fields'] ?? [])->map(function ($field, $fieldIndex) use ($sectionIndex) { - return $field + ['fieldIndex' => $fieldIndex, 'sectionIndex' => $sectionIndex]; + return collect($section['fields'] ?? [])->flatMap(function ($field, $fieldIndex) use ($sectionIndex) { + $indexes = ['fieldIndex' => $fieldIndex, 'sectionIndex' => $sectionIndex]; + + // An imported fieldset is a single entry in the contents, but may + // contain any number of fields, each pointing back at the import. + if (isset($field['import'])) { + return (new Fields([$field]))->all()->map(fn () => $field + $indexes)->all(); + } + + return [$field['handle'] => $field + $indexes]; }); - })->keyBy('handle'); + }); } protected function ensureFieldInTabHasConfig($handle, $tab, $config) @@ -669,10 +683,13 @@ protected function ensureFieldInTabHasConfig($handle, $tab, $config) $field = $this->contents['tabs'][$tab]['sections'][$sectionKey]['fields'][$fieldKey]; - $fieldValue = Arr::get($field, 'field'); - $isImportedField = is_string($fieldValue); - - if ($isImportedField) { + if (isset($field['import'])) { + // An import keeps its overrides in a `config` array keyed by the + // field handles within the fieldset, before any prefix is applied. + $importedHandle = Str::after($handle, $field['prefix'] ?? ''); + $existingConfig = Arr::get($field, "config.{$importedHandle}", []); + $this->contents['tabs'][$tab]['sections'][$sectionKey]['fields'][$fieldKey]['config'][$importedHandle] = array_merge($existingConfig, $config); + } elseif (is_string(Arr::get($field, 'field'))) { $existingConfig = Arr::get($field, 'config', []); $this->contents['tabs'][$tab]['sections'][$sectionKey]['fields'][$fieldKey]['config'] = array_merge($existingConfig, $config); } else { diff --git a/tests/Feature/Entries/CreateEntryTest.php b/tests/Feature/Entries/CreateEntryTest.php index ff3db43b69b..9f1bdde5177 100644 --- a/tests/Feature/Entries/CreateEntryTest.php +++ b/tests/Feature/Entries/CreateEntryTest.php @@ -2,8 +2,12 @@ namespace Tests\Feature\Entries; +use Facades\Statamic\Fields\BlueprintRepository; +use Facades\Statamic\Fields\FieldsetRepository; use PHPUnit\Framework\Attributes\Test; +use Statamic\Facades\Blueprint; use Statamic\Facades\Collection; +use Statamic\Facades\Fieldset; use Statamic\Facades\User; use Tests\FakesRoles; use Tests\PreventSavingStacheItemsToDisk; @@ -89,4 +93,40 @@ public function the_publish_state_can_be_managed_when_able_to_configure_collecti ->assertOk() ->assertInertia(fn ($page) => $page->where('canManagePublishState', true)); } + + #[Test] + public function the_author_field_is_read_only_when_it_comes_from_an_imported_fieldset() + { + $this->setTestRoles(['test' => ['access cp', 'create test entries']]); + $user = tap(User::make()->assignRole('test'))->save(); + $collection = tap(Collection::make('test'))->save(); + + FieldsetRepository::partialMock(); + FieldsetRepository::shouldReceive('find')->with('author')->andReturn( + Fieldset::make('author')->setContents(['fields' => [ + ['handle' => 'author', 'field' => ['type' => 'users', 'max_items' => 1]], + ]]) + ); + + $blueprint = Blueprint::make('test')->setContents(['tabs' => [ + 'main' => ['sections' => [['fields' => [['import' => 'author']]]]], + ]]); + + BlueprintRepository::partialMock(); + BlueprintRepository::shouldReceive('in') + ->with('collections/'.$collection->handle()) + ->andReturn(collect([$blueprint])); + + $this + ->actingAs($user) + ->get(cp_route('collections.entries.create', ['test', 'en'])) + ->assertOk() + ->assertInertia(function ($page) { + $fields = collect($page->toArray()['props']['blueprint']['tabs']) + ->flatMap(fn ($tab) => collect($tab['sections'])->flatMap(fn ($section) => $section['fields'])) + ->keyBy('handle'); + + $this->assertEquals('read_only', $fields['author']['visibility']); + }); + } } diff --git a/tests/Fields/BlueprintTest.php b/tests/Fields/BlueprintTest.php index 93a40b856ef..718a60808b3 100644 --- a/tests/Fields/BlueprintTest.php +++ b/tests/Fields/BlueprintTest.php @@ -930,6 +930,88 @@ public function it_ensures_a_field_has_config() // todo: duplicate or tweak above test but make the target field not in the first section. + #[Test] + public function it_ensures_a_field_within_an_imported_fieldset_has_config() + { + FieldsetRepository::shouldReceive('find')->with('the_partial')->andReturn( + (new Fieldset)->setContents(['fields' => [ + [ + 'handle' => 'author', + 'field' => ['type' => 'users', 'do_not_touch_other_config' => true], + ], + [ + 'handle' => 'the_field', + 'field' => ['type' => 'text'], + ], + ]]) + ); + + $blueprint = (new Blueprint)->setContents(['tabs' => [ + 'tab_one' => [ + 'sections' => [ + [ + 'fields' => [ + ['handle' => 'title', 'field' => ['type' => 'text']], + ], + ], + [ + 'fields' => [ + ['import' => 'the_partial'], + ], + ], + ], + ], + ]]); + + $fields = $blueprint + ->ensureFieldHasConfig('author', ['visibility' => 'read_only']) + ->fields(); + + $this->assertEquals(['type' => 'text'], $fields->get('title')->config()); + $this->assertEquals(['type' => 'text'], $fields->get('the_field')->config()); + + $this->assertEquals([ + 'type' => 'users', + 'do_not_touch_other_config' => true, + 'visibility' => 'read_only', + ], $fields->get('author')->config()); + } + + #[Test] + public function it_ensures_a_prefixed_field_within_an_imported_fieldset_has_config() + { + FieldsetRepository::shouldReceive('find')->with('the_partial')->andReturn( + (new Fieldset)->setContents(['fields' => [ + [ + 'handle' => 'author', + 'field' => ['type' => 'users', 'do_not_touch_other_config' => true], + ], + ]]) + ); + + $blueprint = (new Blueprint)->setContents(['tabs' => [ + 'tab_one' => [ + 'sections' => [ + [ + 'fields' => [ + ['import' => 'the_partial', 'prefix' => 'prefixed_'], + ], + ], + ], + ], + ]]); + + $fields = $blueprint + ->ensureFieldHasConfig('prefixed_author', ['visibility' => 'read_only']) + ->fields(); + + $this->assertEquals([ + 'type' => 'users', + 'do_not_touch_other_config' => true, + 'visibility' => 'read_only', + ], $fields->get('prefixed_author')->config()); + } + #[Test] public function it_can_ensure_an_deferred_ensured_field_has_specific_config() { @@ -1447,6 +1529,39 @@ public function it_removes_a_field_from_a_specific_tab() $this->assertTrue($blueprint->hasField('four')); } + #[Test] + public function it_leaves_fields_within_an_imported_fieldset_alone_when_removing_a_field() + { + FieldsetRepository::shouldReceive('find')->with('the_partial')->andReturn( + (new Fieldset)->setContents(['fields' => [ + ['handle' => 'two', 'field' => ['type' => 'text']], + ['handle' => 'three', 'field' => ['type' => 'text']], + ]]) + ); + + $blueprint = (new Blueprint)->setHandle('test')->setContents([ + 'title' => 'Test', + 'tabs' => [ + 'tab_one' => [ + 'sections' => [ + [ + 'fields' => [ + ['handle' => 'one', 'field' => ['type' => 'text']], + ['import' => 'the_partial'], + ], + ], + ], + ], + ], + ]); + + $blueprint->removeField('one')->removeField('two'); + + $this->assertFalse($blueprint->hasField('one')); + $this->assertTrue($blueprint->hasField('two')); + $this->assertTrue($blueprint->hasField('three')); + } + #[Test] public function it_removes_a_specific_tab() {