fix: Variablen-Detach meldet verbleibende eingebettete Verweise und lehnt Werte mit Verweis, Markup oder leerem Inhalt ab

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Jan-Niclas Loosen
2026-10-19 14:19:41 +02:00
co-authored by Claude Sonnet 5.5
parent e951cc1353
commit 62f8465e60
9 changed files with 303 additions and 35 deletions
@@ -24,6 +24,8 @@ final class DetachTarget
* @param string $nestedIn Where the preset step removes references (messages).
* @param list<string> $notes Warnings of every answer.
* @param string $doneHint The next step once nothing references the target.
* @param bool $complete False when references stay that this call cannot
* remove (embedded ones in presets): no done hint.
*/
public function __construct(
public readonly string $subject,
@@ -33,7 +35,8 @@ final class DetachTarget
public readonly ?Closure $presets,
public readonly string $nestedIn,
public readonly array $notes,
public readonly string $doneHint
public readonly string $doneHint,
public readonly bool $complete = true
) {
}
@@ -50,6 +53,6 @@ final class DetachTarget
*/
public function finished(array $skipped, int $hidden): bool
{
return $this->postIds === null && $skipped === [] && $hidden === 0;
return $this->complete && $this->postIds === null && $skipped === [] && $hidden === 0;
}
}
@@ -235,20 +235,58 @@ final class DiviDesignDetachAbility implements Ability, RequiresDiviFeatures
'Verweise auf diese Variable im Divi-Builder lösen.'
);
}
$nested = $postIds === null && PresetBook::fromDivi($this->writer->presets())->referencing($id) !== [];
$refusal = $this->unsafeValue($value, $label, $type);
if ($refusal !== null) {
return $refusal;
}
$presets = PresetBook::fromDivi($this->writer->presets());
$nested = $postIds === null && $presets->referencing($id) !== [];
$affixes = $type === 'strings';
$notes = [];
if ($nested) {
$left = $presets->withVariableResolved($id, $value, $affixes)->referencing($id);
if ($left !== []) {
$notes[] = sprintf(
'In %d Presets steht die Variable eingebettet in einem längeren Text (%s); diese Verweise bleiben '
. 'unverändert – im Divi-Builder prüfen.',
count($left),
implode(', ', array_column($left, 'name'))
);
}
}
return new DetachTarget(
sprintf('Variable „%s“', $label),
$postIds,
new ReferenceUsageQuery([$id]),
static fn (): NodeRewrite => new VariableDetachRewrite($id, $value, $type),
fn (): NodeRewrite => new VariableDetachRewrite($id, $value, $type, ($this->unfiltered)()),
$nested
? static fn (PresetBook $presets): PresetBook => $presets->withVariableResolved($id, $value, $affixes)
: null,
'Presets',
[],
self::ARCHIVE_HINT
$notes,
self::ARCHIVE_HINT,
$notes === []
);
}
/**
* Refuses a value that must not be inlined into content (S1, D7): empty (it would blank every reference), a
* reference marker, or markup without unfiltered_html. DiviSaver writes without kses, so nothing else checks.
*/
private function unsafeValue(string $value, string $label, string $type): ?WP_Error
{
$problem = match (true) {
$value === '' => 'ist leer',
VariableDetachRewrite::unsafe($value, true) => 'enthält einen Verweis',
VariableDetachRewrite::unsafe($value, ($this->unfiltered)()) => 'enthält Markup (< oder >), das dieses '
. 'Konto nicht speichern darf (unfiltered_html)',
default => null,
};
return $problem === null ? null : Errors::unsupported(
sprintf('Der Wert der Variablen „%s“ (Typ %s) %s; nichts wurde geändert.', $label, $type, $problem),
'Verweise auf diese Variable im Divi-Builder lösen.'
);
}
@@ -328,7 +366,13 @@ final class DiviDesignDetachAbility implements Ability, RequiresDiviFeatures
}
if ($presets === [] && PageRewriteReport::isEmpty($preview)) {
$summary = sprintf('Keine Verweise auf %s gefunden – nichts zu tun.', $target->subject);
$summary = $preview['skipped'] === [] && $target->complete
? sprintf('Keine Verweise auf %s gefunden – nichts zu tun.', $target->subject)
: sprintf(
'Verweise auf %s lassen sich nicht automatisch lösen und bleiben unverändert (siehe skipped und '
. 'warnings) – nichts zu tun.',
$target->subject
);
$done = $target->finished($preview['skipped'], $preview['hidden_count']);
return PageRewriteReport::nothingToDo(
@@ -0,0 +1,19 @@
<?php
declare(strict_types=1);
namespace LIT\MCP\Integrations\Divi\Design;
use LIT\MCP\Integrations\Divi\BlockTree\Block;
defined('ABSPATH') || exit;
/**
* Optional extension of a NodeRewrite: it names the readable blocks that still hold what it removes after rewrite()
* (changed or not), so TreeRewrite lists them as skipped and the caller never announces "done" (VariableDetachRewrite).
*/
interface ReportsLeftovers
{
/** Why $block stays (partly) as it is, or null when nothing of it is left behind. */
public function leftover(Block $block): ?string;
}
+9 -6
View File
@@ -14,8 +14,9 @@ defined('ABSPATH') || exit;
* Applies a NodeRewrite to every block of a NodeIndex, in place (ruling R8): readable blocks are offered to
* rewrite(), a changed one gets its new attributes and then its notes; opaque blocks – global instances, blocks with
* unreadable attributes – are never offered and never changed, but listed as skipped when the strategy says the
* change concerns them (a global module is edited in its library entry). The reason names the library entry's ID
* only with $revealLibraryIds (ruling P13: the caller may see library IDs); otherwise it points to the Divi Builder
* change concerns them (a global module is edited in its library entry). A ReportsLeftovers strategy also gets its
* readable blocks listed as skipped when they keep what it removes. The reason names the library entry's ID only
* with $revealLibraryIds (ruling P13: the caller may see library IDs); otherwise it points to the Divi Builder
* (fail closed). Pure PHP.
*/
final class TreeRewrite
@@ -45,14 +46,16 @@ final class TreeRewrite
}
$attrs = $this->rewrite->rewrite($block);
if ($attrs === null) {
continue;
}
if ($attrs !== null) {
$block->setAttrs($attrs);
$changed[] = ['id' => $id, 'module' => $block->name(), 'label' => NodeSummary::label($block)];
array_push($warnings, ...$this->rewrite->notes($block, $id));
}
$leftover = $this->rewrite instanceof ReportsLeftovers ? $this->rewrite->leftover($block) : null;
if ($leftover !== null) {
$skipped[] = ['id' => $id, 'reason' => $leftover];
}
}
return ['changed' => $changed, 'warnings' => $warnings, 'skipped' => $skipped];
}
@@ -17,13 +17,31 @@ defined('ABSPATH') || exit;
* change the text) and are named in notes(). Other references – colors, other variables, dynamic content – stay
* untouched. Pure PHP.
*/
final class VariableDetachRewrite implements NodeRewrite
final class VariableDetachRewrite implements NodeRewrite, ReportsLeftovers
{
private const EMBEDDED = '%s: Die Variable steht hier eingebettet in einem längeren Text; dieser Verweis bleibt '
. 'stehen – im Divi-Builder ersetzen.';
private const EMBEDDED = 'Die Variable steht hier eingebettet in einem längeren Text; dieser Verweis bleibt '
. 'unverändert – im Divi-Builder prüfen.';
public function __construct(private string $id, private string $value, private string $variableType)
private const UNSAFE = 'Vor- oder Nachtext des Verweises enthält Markup oder einen Verweis; der Verweis bleibt '
. 'unverändert – im Divi-Builder prüfen.';
/** @param bool $markupAllowed Whether "<" and ">" may enter the content (unfiltered_html); markers never may. */
public function __construct(
private string $id,
private string $value,
private string $variableType,
private bool $markupAllowed = false
) {
}
/**
* Whether $text must not be inlined into content: it holds a reference marker (it never went through
* VariableRef::canonical()) or – without unfiltered_html – markup (D7, S1).
*/
public static function unsafe(string $text, bool $markupAllowed): bool
{
return str_contains($text, VariableRef::MARKER) || str_contains($text, VariableRef::END)
|| (! $markupAllowed && (str_contains($text, '<') || str_contains($text, '>')));
}
/**
@@ -44,8 +62,8 @@ final class VariableDetachRewrite implements NodeRewrite
public function rewrite(Block $block): ?array
{
if (! $block->isDivi()) {
return null;
if (! $block->isDivi() || $this->hasUnsafeAffix($block->attrs())) {
return null; // fail closed: the block stays whole and leftover() reports it
}
$changed = false;
@@ -56,7 +74,19 @@ final class VariableDetachRewrite implements NodeRewrite
public function notes(Block $block, string $nodeId): array
{
return $this->embeds($block->attrs()) ? [sprintf(self::EMBEDDED, $nodeId)] : [];
return []; // leftovers go through leftover(): they also matter for blocks rewrite() did not change
}
public function leftover(Block $block): ?string
{
if (! $block->isDivi()) {
return null;
}
if ($this->hasUnsafeAffix($block->attrs())) {
return self::UNSAFE;
}
return $this->embeds($block->attrs()) ? self::EMBEDDED : null;
}
public function concernsInstance(Block $instance): bool
@@ -86,6 +116,36 @@ final class VariableDetachRewrite implements NodeRewrite
return $value;
}
/** Whether a whole reference below $value carries before/after text that must not be inlined. */
private function hasUnsafeAffix(mixed $value): bool
{
if (is_string($value)) {
if ($this->variableType !== 'strings' || VariableRef::nameOf($value) !== $this->id) {
return false;
}
$settings = VariableRef::settingsOf($value);
foreach (['before', 'after'] as $key) {
$text = is_string($settings[$key] ?? null) ? $settings[$key] : '';
if (self::unsafe($text, $this->markupAllowed)) {
return true;
}
}
return false;
}
if (! is_array($value)) {
return false;
}
foreach ($value as $item) {
if ($this->hasUnsafeAffix($item)) {
return true;
}
}
return false;
}
/** Whether a string below $value names the variable inside a longer text. */
private function embeds(mixed $value): bool
{
@@ -88,10 +88,10 @@ final class DiviDesignDetachAbilityTest extends TestCase
}
/** Reads variables() (the Variablen-Verwaltung is allowed unless $manage is false), never saves. */
private static function variableAdapter(bool $manage = true): GlobalVariablesAdapter&MockInterface
private static function variableAdapter(bool $manage = true, string $radius = '8px'): GlobalVariablesAdapter&MockInterface
{
$variables = Mockery::mock(GlobalVariablesAdapter::class);
$variables->allows('read')->andReturn(self::variables());
$variables->allows('read')->andReturn(self::variables($radius));
$variables->allows('canManage')->andReturn($manage);
$variables->expects('save')->never();
@@ -99,11 +99,11 @@ final class DiviDesignDetachAbilityTest extends TestCase
}
/** @return array<string, array<string, array<string, mixed>>> Global variables in Divi's stored form. */
private static function variables(): array
private static function variables(string $radius = '8px'): array
{
return [
'numbers' => [self::RADIUS => [
'id' => self::RADIUS, 'label' => 'Eckenradius', 'value' => '8px', 'order' => '1',
'id' => self::RADIUS, 'label' => 'Eckenradius', 'value' => $radius, 'order' => '1',
'status' => 'active', 'lastUpdated' => '2026-10-01T10:00:00.000Z', 'variableType' => 'numbers',
]],
// Archived or not does not matter for a detach (V9).
@@ -294,6 +294,74 @@ final class DiviDesignDetachAbilityTest extends TestCase
);
}
/** @return array<string, array{string, bool}> */
public static function unsafeValues(): array
{
return [
'empty' => ['', true],
'marker' => ['$variable({"type":"content"})$', true],
'end marker' => ['x)$', true],
'markup without unfiltered_html' => ['<b>8px</b>', false],
];
}
/** @dataProvider unsafeValues */
public function testAValueThatMustNotBeInlinedRefusesTheCall(string $value, bool $unfiltered): void
{
$ability = $this->ability(
self::untouched(),
null,
null,
null,
null,
$unfiltered,
self::variableAdapter(true, $value)
);
$error = self::assertToolError($ability->execute(['variable' => 'Eckenradius']), 'unsupported');
self::assertStringContainsString('Divi-Builder', $error['hint']);
}
public function testMarkupInTheValueIsAllowedWithUnfilteredHtml(): void
{
$pages = Mockery::mock(PageRewriter::class);
$pages->expects('preview')->andReturn(PageRewriteReportTest::preview(1));
$ability = $this->ability($pages, null, null, null, null, true, self::variableAdapter(true, '<b>8px</b>'));
self::assertToolError($ability->execute(['variable' => 'Eckenradius']), 'confirm_required');
}
public function testReferencesLeftOnPagesNeverGetTheDoneHint(): void
{
$preview = PageRewriteReportTest::preview(0);
$preview['skipped'] = [['post_id' => 12, 'reason' => 's0.r0.c0.m0: Die Variable steht hier eingebettet']];
$pages = Mockery::mock(PageRewriter::class);
$pages->expects('preview')->andReturn($preview);
$result = $this->ability($pages)->execute(['variable' => 'Eckenradius']);
self::assertStringNotContainsString('lit/divi-variables-set', $result['summary']);
self::assertStringNotContainsString('Keine Verweise', $result['summary']);
self::assertSame($preview['skipped'], $result['skipped']);
}
public function testEmbeddedReferencesInPresetsAreReportedAndWithholdTheDoneHint(): void
{
$stored = PresetBookTest::stored();
$stored['module']['divi/button']['items']['aaaaaaaaaa']['attrs'] = ['button' => ['decoration' => [
'border' => ['desktop' => ['value' => ['radius' => 'Ab ' . VariableDetachRewriteTest::ref(self::RADIUS) . ' x']]],
]]];
$pages = Mockery::mock(PageRewriter::class);
$pages->expects('preview')->andReturn(PageRewriteReportTest::preview(0));
$result = $this->ability($pages, self::readOnlyWriter($stored))->execute(['variable' => 'Eckenradius']);
self::assertStringNotContainsString('lit/divi-variables-set', $result['summary']);
self::assertStringContainsString('In 1 Presets steht die Variable eingebettet', $result['warnings'][0]);
self::assertStringContainsString('CTA primär', $result['warnings'][0]);
}
public function testVariablePostIdsSkipThePresetStep(): void
{
$pages = Mockery::mock(PageRewriter::class);
@@ -9,6 +9,8 @@ use Closure;
use LIT\MCP\Abilities\WriteFailed;
use LIT\MCP\Integrations\Divi\BlockTree\Block;
use LIT\MCP\Integrations\Divi\BlockTree\GlobalInstance;
use LIT\MCP\Integrations\Divi\BlockTree\Json;
use LIT\MCP\Integrations\Divi\Design\VariableDetachRewrite;
use LIT\MCP\Integrations\Divi\Design\NodeRewrite;
use LIT\MCP\Integrations\Divi\Design\PageRewriter;
use LIT\MCP\Integrations\Divi\Library\GlobalUsages;
@@ -368,6 +370,33 @@ final class PageRewriterTest extends TestCase
self::assertSame(['102'], array_column($this->log->records, 'object_id'));
}
/** V9: a page that only mentions the variable inside a longer text is not saved, but reported as skipped. */
public function testVariableReferencesLeftBehindAreSkippedEvenOnAPageThatIsNotSaved(): void
{
$embedded = '<!-- wp:divi/text ' . Json::encode(['content' => ['innerContent' => ['desktop' => ['value' => 'Ab '
. VariableDetachRewriteTest::ref('gvid-radius0001') . ' Rundung']]]]) . ' /-->';
$whole = '<!-- wp:divi/text ' . Json::encode(['content' => ['innerContent' => ['desktop' => ['value'
=> VariableDetachRewriteTest::ref('gvid-radius0001')]]]]) . ' /-->';
$rewriter = $this->rewriter([
self::page(101, self::layout($embedded)),
self::page(102, self::layout($whole, $embedded)),
]);
$rewrite = static fn (): NodeRewrite => new VariableDetachRewrite('gvid-radius0001', '8px', 'numbers');
$preview = $rewriter->preview(self::query(), $rewrite, null);
$result = $rewriter->apply(self::ABILITY, self::query(), $rewrite, null, self::SUMMARY);
self::assertSame(1, $preview['pages_count']);
self::assertSame(
[101, 102],
array_column($preview['skipped'], 'post_id')
);
self::assertStringStartsWith('s0.r0.c0.m0: Die Variable steht hier eingebettet', $preview['skipped'][0]['reason']);
self::assertStringStartsWith('s0.r0.c0.m1: ', $preview['skipped'][1]['reason']);
self::assertSame([102], array_column($result['saved'], 'post_id'));
self::assertSame([101, 102], array_column($result['skipped'], 'post_id'));
}
/** R8: pages loadWritable() refuses (Divi 4, unparsable – fail-closed candidates) are skipped, not remaining. */
public function testRefusedPagesAreSkippedWithTheLoaderMessage(): void
{
@@ -12,6 +12,7 @@ use LIT\MCP\Integrations\Divi\BlockTree\Parser;
use LIT\MCP\Integrations\Divi\BlockTree\Serializer;
use LIT\MCP\Integrations\Divi\Design\NodeRewrite;
use LIT\MCP\Integrations\Divi\Design\TreeRewrite;
use LIT\MCP\Integrations\Divi\Design\VariableDetachRewrite;
use LIT\MCP\Tests\Unit\TestCase;
final class TreeRewriteTest extends TestCase
@@ -134,4 +135,22 @@ final class TreeRewriteTest extends TestCase
$result['skipped']
);
}
public function testAReportsLeftoversStrategyListsUnchangedBlocksAsSkipped(): void
{
$embedded = 'Ab ' . VariableDetachRewriteTest::ref('gvid-radius0001') . ' Rundung';
$text = static fn (string $value): string => '<!-- wp:divi/text ' . \LIT\MCP\Integrations\Divi\BlockTree\Json::encode(
['content' => ['innerContent' => ['desktop' => ['value' => $value]]]]
) . ' /-->';
$document = self::document(
$text($embedded) . $text(VariableDetachRewriteTest::ref('gvid-radius0001')) . '<!-- wp:divi/text /-->'
);
$result = (new TreeRewrite(new VariableDetachRewrite('gvid-radius0001', '8px', 'numbers')))
->apply(NodeIndex::build($document));
self::assertSame(['s0.r0.c0.m1'], array_column($result['changed'], 'id'));
self::assertSame(['s0.r0.c0.m0'], array_column($result['skipped'], 'id'), 'embedded only: reported');
self::assertSame([], $result['warnings']);
}
}
@@ -148,19 +148,42 @@ final class VariableDetachRewriteTest extends TestCase
$attrs = $rewrite->rewrite($first);
self::assertSame($embedded, $attrs['content']['innerContent']['desktop']['value'] ?? null);
$first->setAttrs((array) $attrs);
self::assertSame(
['s0.r0.c0.m0: Die Variable steht hier eingebettet in einem längeren Text; dieser Verweis bleibt stehen – '
. 'im Divi-Builder ersetzen.'],
$rewrite->notes($first, 's0.r0.c0.m0')
);
self::assertStringContainsString('eingebettet', (string) $rewrite->leftover($first));
self::assertStringContainsString('bleibt unverändert', (string) $rewrite->leftover($first));
self::assertNotNull($rewrite->leftover(self::node($index, 'm1')), 'embedded only: still reported');
self::assertNull($rewrite->rewrite(self::node($index, 'm1')), 'only embedded: nothing to replace');
self::assertSame(
[],
(new VariableDetachRewrite(self::CLAIM, 'x', 'strings'))->notes($first, 's0.r0.c0.m0'),
'another variable is not noted'
self::assertNull(
(new VariableDetachRewrite(self::CLAIM, 'x', 'strings'))->leftover($first),
'another variable is not reported'
);
}
public function testUnsafeBeforeAndAfterLeaveTheBlockWholeAndReportIt(): void
{
$index = self::index([
['divi/text', ['content' => ['innerContent' => self::desktop(self::ref(self::CLAIM, ['before' => '<b>']))]]],
['divi/text', ['content' => ['innerContent' => self::desktop(
// Built by hand: VariableRef::serialize() itself refuses a marker in the settings.
'$variable(' . Json::encode(['type' => 'content', 'value' => [
'name' => self::CLAIM, 'settings' => ['after' => 'a $variable(b']
]]) . ')$'
)]]],
['divi/text', ['content' => ['innerContent' => self::desktop(self::ref(self::CLAIM, ['before' => '<b>']))]]],
]);
$strict = new VariableDetachRewrite(self::CLAIM, 'x', 'strings');
$allowed = new VariableDetachRewrite(self::CLAIM, 'x', 'strings', true);
self::assertNull($strict->rewrite(self::node($index, 'm0')));
self::assertStringContainsString('Markup', (string) $strict->leftover(self::node($index, 'm0')));
self::assertNull($allowed->rewrite(self::node($index, 'm1')), 'a marker is never inlined');
self::assertNotNull($allowed->leftover(self::node($index, 'm1')));
$third = self::node($index, 'm2');
$attrs = (array) $allowed->rewrite($third);
self::assertSame('<b>x', $attrs['content']['innerContent']['desktop']['value'] ?? null);
$third->setAttrs($attrs);
self::assertNull($allowed->leftover($third));
}
public function testInstancesThatMentionTheIdConcernTheRewrite(): void
{
$index = self::index([