From 17402549c5768f078909a80b8c80969fef20e07f Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Sat, 26 Sep 2026 15:47:01 +0200 Subject: [PATCH] test: Task 7 review - broaden move/delete/duplicate coverage, fix outline for delete-only changes Co-Authored-By: Claude Opus 5.5 (1M context) --- src/Integrations/Divi/Pages/DiviWriter.php | 4 +- .../Divi/Pages/DiviWriteAbilitiesTest.php | 201 +++++++++++++++++- 2 files changed, 195 insertions(+), 10 deletions(-) diff --git a/src/Integrations/Divi/Pages/DiviWriter.php b/src/Integrations/Divi/Pages/DiviWriter.php index 8e7d2a4..7aad973 100644 --- a/src/Integrations/Divi/Pages/DiviWriter.php +++ b/src/Integrations/Divi/Pages/DiviWriter.php @@ -194,7 +194,9 @@ class DiviWriter { $sections = []; foreach ($change->entries() as $entry) { - $id = $entry['block'] !== null ? $index->idOf($entry['block']) : null; + // Same id resolution as the "changed" entries below: a removed node has no block + // any more, but its id from before the removal still names the (surviving) section. + $id = $entry['id'] ?? ($entry['block'] !== null ? $index->idOf($entry['block']) : null); if ($id !== null) { $sections[explode('.', $id)[0]] = true; } diff --git a/tests/Unit/Integrations/Divi/Pages/DiviWriteAbilitiesTest.php b/tests/Unit/Integrations/Divi/Pages/DiviWriteAbilitiesTest.php index a663fad..ca60114 100644 --- a/tests/Unit/Integrations/Divi/Pages/DiviWriteAbilitiesTest.php +++ b/tests/Unit/Integrations/Divi/Pages/DiviWriteAbilitiesTest.php @@ -254,38 +254,221 @@ final class DiviWriteAbilitiesTest extends TestCase self::assertSame([], $this->saved); } - /** Ruling S14: move there and back restores the exact bytes; delete of the copy too. */ - public function testMoveDuplicateAndDelete(): void + /** Ruling S14: move there and back restores the exact bytes (full-content assertSame, no strlen). */ + public function testMoveThereAndBackIsByteExact(): void { $moved = $this->move()->execute([ 'post_id' => 7, 'tree_rev' => $this->treeRev(), 'id' => 's0.r0.c1.m2', 'parent' => 's1.r0.c0', ]); self::assertSame([['id' => 's1.r0.c0.m1', 'type' => 'divi/button', 'action' => 'moved', 'label' => 'Hero – Button']], $moved['changed']); + self::assertStringContainsString('Leistungen', (string) $moved['outline'], 'the outline is embedded in the result'); $back = $this->move()->execute([ 'post_id' => 7, 'tree_rev' => Rev::of($this->saved[0]), 'id' => 's1.r0.c0.m1', 'parent' => 's0.r0.c1', ]); self::assertSame([['id' => 's0.r0.c1.m2', 'type' => 'divi/button', 'action' => 'moved', 'label' => 'Hero – Button']], $back['changed']); self::assertSame($this->original, $this->saved[1], 'move there and back restores the exact bytes (S14)'); + } - $duplicated = $this->duplicate()->execute([ - 'post_id' => 7, 'tree_rev' => $this->treeRev(), 'id' => 's1', + /** StructureRules apply on move: a module directly into a section, or into a specialty column. */ + public function testMoveRefusedByStructureRules(): void + { + $mixed = DiviPageLoaderTest::fixture('synthetic-mixed.html'); + $this->post->post_content = $mixed; + $treeRev = Rev::of($mixed); + + // s1 is a regular section: it holds rows, not modules directly. + $intoSection = $this->move()->execute([ + 'post_id' => 7, 'tree_rev' => $treeRev, 'id' => 's0.c0.m0', 'parent' => 's1', ]); + self::assertSame('lit_validation_failed', $intoSection->get_error_code()); + self::assertStringContainsString('Zeilen (row)', self::clientError($intoSection)['fields']['position']); + + // s0.c1 is a specialty column (specialtyColumns set, holds a row-inner): refuses every child. + $intoSpecialtyColumn = $this->move()->execute([ + 'post_id' => 7, 'tree_rev' => $treeRev, 'id' => 's0.c0.m0', 'parent' => 's0.c1', + ]); + self::assertSame('lit_validation_failed', $intoSpecialtyColumn->get_error_code()); + self::assertStringContainsString('Spezialspalten', self::clientError($intoSpecialtyColumn)['fields']['position']); + + self::assertSame([], $this->saved); + self::assertSame($mixed, $this->post->post_content); + } + + public function testMoveIntoItsOwnDescendantIsRefused(): void + { + $result = $this->move()->execute([ + 'post_id' => 7, 'tree_rev' => $this->treeRev(), 'id' => 's0', 'parent' => 's0.r0.c1', + ]); + + self::assertSame('lit_validation_failed', $result->get_error_code()); + self::assertStringContainsString( + 'nicht in sich selbst verschoben', + self::clientError($result)['fields']['position'] + ); + self::assertSame([], $this->saved); + } + + public function testMoveRefusesAStaleTreeRev(): void + { + $result = $this->move()->execute([ + 'post_id' => 7, 'tree_rev' => '000000000000', 'id' => 's0.r0.c1.m2', 'parent' => 's1.r0.c0', + ]); + + self::assertSame('lit_rev_mismatch', $result->get_error_code()); + self::assertSame('rev_mismatch', self::clientError($result)['code'], 'the client sees the code without lit_'); + self::assertSame([], $this->saved); + } + + /** The stored content equals the original minus exactly that block's bytes; revision_id is the restore point. */ + public function testDeleteAModuleRemovesExactlyItsBytes(): void + { + $buttonBytes = ''; + self::assertStringContainsString($buttonBytes, $this->original, 'precondition: exact bytes as expected'); + + $result = $this->delete()->execute([ + 'post_id' => 7, 'tree_rev' => $this->treeRev(), 'ids' => ['s0.r0.c1.m2'], 'confirm' => true, + ]); + + self::assertSame([['id' => 's0.r0.c1.m2', 'type' => 'divi/button', 'action' => 'deleted', 'label' => 'Hero – Button']], $result['changed']); + self::assertSame(str_replace($buttonBytes, '', $this->original), $this->saved[0]); + self::assertSame(811, $result['revision_id'], 'the restore-point revision'); + self::assertStringContainsString('Hero', (string) $result['outline'], 'the outline is embedded in the result'); + } + + /** An opaque/global module is removed as a whole – it never gets descended into. */ + public function testDeleteRemovesAnOpaqueGlobalModuleAsAWhole(): void + { + $mixed = DiviPageLoaderTest::fixture('synthetic-mixed.html'); + $this->post->post_content = $mixed; + $globalRow = ''; + self::assertStringContainsString($globalRow, $mixed, 'precondition: exact bytes as expected'); + + $result = $this->delete()->execute([ + 'post_id' => 7, 'tree_rev' => Rev::of($mixed), 'ids' => ['s1.g1'], 'confirm' => true, + ]); + + self::assertSame([['id' => 's1.g1', 'type' => 'divi/row', 'action' => 'deleted', 'label' => 'Globale Zeile']], $result['changed']); + self::assertSame(str_replace($globalRow, '', $mixed), $this->saved[0]); + } + + public function testDeletingTheLastChildLeavesAValidTree(): void + { + $mixed = DiviPageLoaderTest::fixture('synthetic-mixed.html'); + $this->post->post_content = $mixed; + + $result = $this->delete()->execute([ + 'post_id' => 7, 'tree_rev' => Rev::of($mixed), 'ids' => ['s0.c1.r0.c0.m0'], 'confirm' => true, + ]); + + self::assertTrue($result['ok'], (string) json_encode($result)); + $index = NodeIndex::build((new Parser())->parse($this->saved[0])); + $emptyColumn = $index->find('s0.c1.r0.c0'); + self::assertNotNull($emptyColumn, 'the now-empty column-inner is still a valid, findable node'); + self::assertSame([], $emptyColumn->children()); + } + + public function testDeleteUnknownIdReturnsNotFound(): void + { + $result = $this->delete()->execute([ + 'post_id' => 7, 'tree_rev' => $this->treeRev(), 'ids' => ['s9'], 'confirm' => true, + ]); + + self::assertSame('lit_not_found', $result->get_error_code()); + self::assertSame('not_found', self::clientError($result)['code']); + self::assertSame([], $this->saved); + } + + /** Same annotations as the existing destructive tools (e.g. MediaDeleteAbility). */ + public function testDeleteAnnotationsMatchExistingDestructiveTools(): void + { + self::assertSame( + ['readonly' => false, 'destructive' => true, 'idempotent' => false], + $this->delete()->annotations() + ); + } + + /** Ruling S14 (Task 7): the deleted node's own confirm question, unaffected by other tests. */ + public function testDeleteAsksBeforeRemovingNestedNodesOnlyOnce(): void + { + $duplicated = $this->duplicate()->execute(['post_id' => 7, 'tree_rev' => $this->treeRev(), 'id' => 's1']); self::assertSame('s2', $duplicated['changed'][0]['id']); - self::assertSame('Leistungen (Kopie)', $duplicated['changed'][0]['label']); $delete = $this->delete(); - $ask = $delete->execute(['post_id' => 7, 'tree_rev' => Rev::of($this->saved[2]), 'ids' => ['s2', 's2.r0']]); + $ask = $delete->execute(['post_id' => 7, 'tree_rev' => Rev::of($this->saved[0]), 'ids' => ['s2', 's2.r0']]); self::assertSame('lit_confirm_required', $ask->get_error_code()); self::assertSame( '1 Divi-Knoten würden samt Inhalt entfernt: s2 section "Leistungen (Kopie)" (1 Kind-Knoten).', json_decode($ask->get_error_message(), true)['message'] ); - $deleted = $delete->execute(['post_id' => 7, 'tree_rev' => Rev::of($this->saved[2]), 'ids' => ['s2'], 'confirm' => true]); + $deleted = $delete->execute(['post_id' => 7, 'tree_rev' => Rev::of($this->saved[0]), 'ids' => ['s2'], 'confirm' => true]); self::assertSame([['id' => 's2', 'type' => 'divi/section', 'action' => 'deleted', 'label' => 'Leistungen (Kopie)']], $deleted['changed']); - self::assertSame($this->saved[1], $this->saved[3], 'deleting the copy restores the exact bytes'); - self::assertTrue($delete->annotations()['destructive']); + self::assertSame($this->original, $this->saved[1], 'deleting the copy restores the exact bytes'); + } + + /** The copy sits directly after the original; its own child content is byte-identical, nothing else changes. */ + public function testDuplicateInsertsTheCopyDirectlyAfterTheOriginal(): void + { + $sectionOpen = ''; + $sectionClose = ''; + $start = strpos($this->original, $sectionOpen) + strlen($sectionOpen); + $end = strpos($this->original, $sectionClose, $start); + $rowBytes = substr($this->original, $start, $end - $start); + $original = $sectionOpen . $rowBytes . $sectionClose; + self::assertStringContainsString($original, $this->original, 'precondition: exact bytes as expected'); + + $result = $this->duplicate()->execute(['post_id' => 7, 'tree_rev' => $this->treeRev(), 'id' => 's1']); + + self::assertSame('s2', $result['changed'][0]['id']); + self::assertSame('Leistungen (Kopie)', $result['changed'][0]['label']); + self::assertStringContainsString('Leistungen', (string) $result['outline'], 'the outline is embedded in the result'); + + $copyOpen = str_replace('"Leistungen"', '"Leistungen (Kopie)"', $sectionOpen); + $copy = $copyOpen . $rowBytes . $sectionClose; + self::assertSame( + str_replace($original, $original . $copy, $this->original), + $this->saved[0], + 'copy directly after the original, byte-identical child content, nothing else changes' + ); + } + + /** A global module copy stays a byte-identical reference (read-only nodes move/duplicate as a whole). */ + public function testDuplicateGlobalModuleCopyIsAByteIdenticalReference(): void + { + $mixed = DiviPageLoaderTest::fixture('synthetic-mixed.html'); + $this->post->post_content = $mixed; + $globalRow = ''; + self::assertStringContainsString($globalRow, $mixed, 'precondition: exact bytes as expected'); + + $result = $this->duplicate()->execute(['post_id' => 7, 'tree_rev' => Rev::of($mixed), 'id' => 's1.g1']); + + self::assertSame('s1.g2', $result['changed'][0]['id']); + self::assertSame('divi/row', $result['changed'][0]['type']); + self::assertSame('Globale Zeile', $result['changed'][0]['label'], 'opaque copies keep their label unchanged'); + self::assertSame(str_replace($globalRow, $globalRow . $globalRow, $mixed), $this->saved[0]); + } + + /** Permission denied (edit_post false) for all three new abilities. */ + public function testPermissionDeniedWhenNotEditable(): void + { + Functions\when('current_user_can')->justReturn(false); + + foreach ([$this->move(), $this->delete(), $this->duplicate()] as $ability) { + $result = $ability->permission(['post_id' => 7, 'tree_rev' => $this->treeRev()]); + self::assertInstanceOf(WP_Error::class, $result, $ability->name()); + self::assertSame('lit_forbidden', $result->get_error_code(), $ability->name()); + } + self::assertSame([], $this->saved); } public function testReadOnlyNodesCannotBeUpdated(): void