From 22bb1528db9fd0f600b5522018aded17f99fed6b Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Sat, 26 Sep 2026 15:17:42 +0200 Subject: [PATCH] fix: S40 - short form and attrs on the same path refused for new nodes too Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7 --- src/Integrations/Divi/Compose/Composer.php | 62 ++++++++++++++----- .../Divi/Compose/ComposerTest.php | 28 +++++++++ .../Divi/Pages/DiviWriteAbilitiesTest.php | 19 ++++++ 3 files changed, 95 insertions(+), 14 deletions(-) diff --git a/src/Integrations/Divi/Compose/Composer.php b/src/Integrations/Divi/Compose/Composer.php index f53107d..c4dbd95 100644 --- a/src/Integrations/Divi/Compose/Composer.php +++ b/src/Integrations/Divi/Compose/Composer.php @@ -44,6 +44,11 @@ final class Composer private const LABEL_PATH = 'module.meta.adminLabel'; + private const BACKGROUND = 'module.decoration.background'; + + /** Advice of the S40 overlap error for new nodes. */ + private const ONE_FORM = 'für diesen Pfad nur Kurzform oder attrs angeben'; + /** @var array */ private array $errors = []; @@ -116,7 +121,13 @@ final class Composer } } - $this->overlaps($block->shortName(), $change, $field); + $set = $change['set'] ?? null; + $this->overlaps( + is_array($set) ? $this->shortPaths($block->shortName(), $set, 'set.') : [], + $change['attrs'] ?? null, + $field, + 'bitte als zwei Änderungen senden' + ); $attrs = $this->presetAndAttrs($block->name(), $attrs, $change, $field); if (array_key_exists('label', $change)) { $attrs = $change['label'] === null || $change['label'] === '' @@ -128,21 +139,20 @@ final class Composer } /** - * Ruling S38: one change may not reach the same attribute path through "set" and "attrs" – - * composite values are replaced whole per breakpoint, so combining them would silently lose - * sub-keys. Paths overlap when equal or when one lies below the other. + * Rulings S38/S40: one change or new node may not reach the same attribute path through the + * short form and "attrs" – composite values are replaced whole per breakpoint, so combining + * them would silently lose sub-keys. Paths overlap when equal or when one lies below the other. * - * @param array $change + * @param array $short Input name of the short-form value ("set.text", "text") => its path. + * @param mixed $full The "attrs" input. + * @param string $advice How to resolve it, for the error message. */ - private function overlaps(string $type, array $change, string $field): void + private function overlaps(array $short, mixed $full, string $field, string $advice): void { - $set = $change['set'] ?? null; - $full = $change['attrs'] ?? null; - if (! is_array($set) || ! is_array($full) || $set === [] || $full === []) { + if ($short === [] || ! is_array($full)) { return; } - $short = $this->shortform->paths($type, array_map('strval', array_keys($set))); foreach (array_keys($full) as $path) { $path = (string) $path; foreach ($short as $name => $shortPath) { @@ -151,8 +161,9 @@ final class Composer || str_starts_with($shortPath, $path . '.') ) { $this->errors[$field . '.attrs.' . $path] = sprintf( - 'derselbe Attributpfad wird auch über set.%s geändert; bitte als zwei Änderungen senden', - $name + 'derselbe Attributpfad wird auch über %s geändert; %s', + $name, + $advice ); break; } @@ -160,6 +171,22 @@ final class Composer } } + /** + * Paths of the short-form fields a client gave, keyed by their input name ($prefix . field). + * + * @param array $values Field => value. + * @return array + */ + private function shortPaths(string $type, array $values, string $prefix): array + { + $paths = []; + foreach ($this->shortform->paths($type, array_map('strval', array_keys($values))) as $name => $path) { + $paths[$prefix . $name] = $path; + } + + return $paths; + } + /** Admin label for a new module: " – " or the title alone. */ public function autoLabel(string $name, string $context): string { @@ -227,6 +254,12 @@ final class Composer if (array_key_exists('background_color', $spec)) { $attrs = $this->backgroundColor($attrs, $spec['background_color'], $path . '.background_color'); + $this->overlaps( + ['background_color' => self::BACKGROUND], + $spec['attrs'] ?? null, + $path, + self::ONE_FORM + ); } $attrs = $this->common('divi/section', $attrs, $spec, $path, null); @@ -312,6 +345,7 @@ final class Composer // "type" and "label" of a node are its module type and admin label; the short-form fields // of the same name (contact-field type, social network label) go through lit-divi-update "set". $values = array_intersect_key($spec, array_flip(array_diff($fields, self::COMMON))); + $this->overlaps($this->shortPaths($short, $values, ''), $spec['attrs'] ?? null, $path, self::ONE_FORM); if ($name === 'divi/heading' && ! array_key_exists('level', $values)) { $values['level'] = 'h2'; } @@ -471,14 +505,14 @@ final class Composer private function backgroundColor(array $attrs, mixed $color, string $path): array { try { - $clean = $this->shortform->color('divi/section', 'module.decoration.background', $color, 'color'); + $clean = $this->shortform->color('divi/section', self::BACKGROUND, $color, 'color'); } catch (InvalidArgumentException $e) { $this->errors[$path] = $e->getMessage(); return $attrs; } - return AttrMap::set($attrs, 'module.decoration.background', ['desktop' => ['value' => ['color' => $clean]]]); + return AttrMap::set($attrs, self::BACKGROUND, ['desktop' => ['value' => ['color' => $clean]]]); } private function children(Block $parent, mixed $specs, string $path, string $context): void diff --git a/tests/Unit/Integrations/Divi/Compose/ComposerTest.php b/tests/Unit/Integrations/Divi/Compose/ComposerTest.php index fe47b7e..c11e067 100644 --- a/tests/Unit/Integrations/Divi/Compose/ComposerTest.php +++ b/tests/Unit/Integrations/Divi/Compose/ComposerTest.php @@ -203,6 +203,34 @@ final class ComposerTest extends TestCase self::assertSame([], $separate['errors'], 'different paths may be combined'); } + /** Ruling S40: a new node may not reach the same attribute path through short form and attrs. */ + public function testComposeRefusesShortFormAndAttrsOnTheSamePath(): void + { + $result = ComposeFixture::composer()->compose([[ + 'type' => 'section', + 'background_color' => '#ffffff', + 'attrs' => ['module.decoration.background' => ['desktop' => ['color' => '#000000']]], + 'modules' => [[ + 'type' => 'button', + 'text' => 'Neu', + 'attrs' => ['button.innerContent' => ['desktop' => ['linkUrl' => '/kontakt']]], + ]], + ]], null, 'nodes'); + + self::assertStringContainsString('background_color', $result['errors']['nodes[0].attrs.module.decoration.background']); + self::assertStringContainsString('text', $result['errors']['nodes[0].modules[0].modules[0][0].attrs.button.innerContent']); + self::assertSame([], $result['invalid_paths']); + + $separate = ComposeFixture::composer()->compose([[ + 'type' => 'section', + 'modules' => [ + ['type' => 'button', 'text' => 'Neu', 'attrs' => ['button.decoration.background' => ['desktop' => ['color' => '#ffffff']]]], + ['type' => 'heading', 'text' => 'Titel', 'attrs' => ['title.decoration.font.font' => ['phone' => ['size' => '20px']]]], + ], + ]], null, 'nodes'); + self::assertSame([], $separate['errors'], 'different paths, and the default heading level, may be combined'); + } + /** Ruling S13: labels go through the choke point; a label that fails is an error, never stored raw. */ public function testLabelsAreCleanedOrReported(): void { diff --git a/tests/Unit/Integrations/Divi/Pages/DiviWriteAbilitiesTest.php b/tests/Unit/Integrations/Divi/Pages/DiviWriteAbilitiesTest.php index 0672cfb..e65f426 100644 --- a/tests/Unit/Integrations/Divi/Pages/DiviWriteAbilitiesTest.php +++ b/tests/Unit/Integrations/Divi/Pages/DiviWriteAbilitiesTest.php @@ -278,6 +278,25 @@ final class DiviWriteAbilitiesTest extends TestCase self::assertSame([], $this->saved); } + /** Ruling S40: the S38 overlap rule for new nodes too. */ + public function testInsertRefusesShortFormAndAttrsOnTheSamePath(): void + { + $result = $this->insert()->execute([ + 'post_id' => 7, + 'tree_rev' => $this->treeRev(), + 'parent' => 's0.r0.c1', + 'nodes' => [[ + 'type' => 'button', + 'text' => 'Neu', + 'attrs' => ['button.innerContent' => ['desktop' => ['linkUrl' => '/neu']]], + ]], + ]); + + self::assertSame('lit_validation_failed', $result->get_error_code()); + self::assertArrayHasKey('nodes[0].attrs.button.innerContent', self::clientError($result)['fields']); + self::assertSame([], $this->saved); + } + /** Ruling S7: builder-only paths are refused for users without unfiltered_html – visible to the client. */ public function testBuilderOnlyPathsAreRefusedForEditors(): void {