fix: S40 - short form and attrs on the same path refused for new nodes too
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7
This commit is contained in:
@@ -44,6 +44,11 @@ final class Composer
|
|||||||
|
|
||||||
private const LABEL_PATH = 'module.meta.adminLabel';
|
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<string, string> */
|
/** @var array<string, string> */
|
||||||
private array $errors = [];
|
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);
|
$attrs = $this->presetAndAttrs($block->name(), $attrs, $change, $field);
|
||||||
if (array_key_exists('label', $change)) {
|
if (array_key_exists('label', $change)) {
|
||||||
$attrs = $change['label'] === null || $change['label'] === ''
|
$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" –
|
* Rulings S38/S40: one change or new node may not reach the same attribute path through the
|
||||||
* composite values are replaced whole per breakpoint, so combining them would silently lose
|
* short form and "attrs" – composite values are replaced whole per breakpoint, so combining
|
||||||
* sub-keys. Paths overlap when equal or when one lies below the other.
|
* them would silently lose sub-keys. Paths overlap when equal or when one lies below the other.
|
||||||
*
|
*
|
||||||
* @param array<array-key, mixed> $change
|
* @param array<string, string> $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;
|
if ($short === [] || ! is_array($full)) {
|
||||||
$full = $change['attrs'] ?? null;
|
|
||||||
if (! is_array($set) || ! is_array($full) || $set === [] || $full === []) {
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
$short = $this->shortform->paths($type, array_map('strval', array_keys($set)));
|
|
||||||
foreach (array_keys($full) as $path) {
|
foreach (array_keys($full) as $path) {
|
||||||
$path = (string) $path;
|
$path = (string) $path;
|
||||||
foreach ($short as $name => $shortPath) {
|
foreach ($short as $name => $shortPath) {
|
||||||
@@ -151,8 +161,9 @@ final class Composer
|
|||||||
|| str_starts_with($shortPath, $path . '.')
|
|| str_starts_with($shortPath, $path . '.')
|
||||||
) {
|
) {
|
||||||
$this->errors[$field . '.attrs.' . $path] = sprintf(
|
$this->errors[$field . '.attrs.' . $path] = sprintf(
|
||||||
'derselbe Attributpfad wird auch über set.%s geändert; bitte als zwei Änderungen senden',
|
'derselbe Attributpfad wird auch über %s geändert; %s',
|
||||||
$name
|
$name,
|
||||||
|
$advice
|
||||||
);
|
);
|
||||||
break;
|
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<array-key, mixed> $values Field => value.
|
||||||
|
* @return array<string, string>
|
||||||
|
*/
|
||||||
|
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: "<Abschnitt> – <Modultitel>" or the title alone. */
|
/** Admin label for a new module: "<Abschnitt> – <Modultitel>" or the title alone. */
|
||||||
public function autoLabel(string $name, string $context): string
|
public function autoLabel(string $name, string $context): string
|
||||||
{
|
{
|
||||||
@@ -227,6 +254,12 @@ final class Composer
|
|||||||
|
|
||||||
if (array_key_exists('background_color', $spec)) {
|
if (array_key_exists('background_color', $spec)) {
|
||||||
$attrs = $this->backgroundColor($attrs, $spec['background_color'], $path . '.background_color');
|
$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);
|
$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
|
// "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".
|
// 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)));
|
$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)) {
|
if ($name === 'divi/heading' && ! array_key_exists('level', $values)) {
|
||||||
$values['level'] = 'h2';
|
$values['level'] = 'h2';
|
||||||
}
|
}
|
||||||
@@ -471,14 +505,14 @@ final class Composer
|
|||||||
private function backgroundColor(array $attrs, mixed $color, string $path): array
|
private function backgroundColor(array $attrs, mixed $color, string $path): array
|
||||||
{
|
{
|
||||||
try {
|
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) {
|
} catch (InvalidArgumentException $e) {
|
||||||
$this->errors[$path] = $e->getMessage();
|
$this->errors[$path] = $e->getMessage();
|
||||||
|
|
||||||
return $attrs;
|
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
|
private function children(Block $parent, mixed $specs, string $path, string $context): void
|
||||||
|
|||||||
@@ -203,6 +203,34 @@ final class ComposerTest extends TestCase
|
|||||||
self::assertSame([], $separate['errors'], 'different paths may be combined');
|
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. */
|
/** Ruling S13: labels go through the choke point; a label that fails is an error, never stored raw. */
|
||||||
public function testLabelsAreCleanedOrReported(): void
|
public function testLabelsAreCleanedOrReported(): void
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -278,6 +278,25 @@ final class DiviWriteAbilitiesTest extends TestCase
|
|||||||
self::assertSame([], $this->saved);
|
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. */
|
/** Ruling S7: builder-only paths are refused for users without unfiltered_html – visible to the client. */
|
||||||
public function testBuilderOnlyPathsAreRefusedForEditors(): void
|
public function testBuilderOnlyPathsAreRefusedForEditors(): void
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user