fix: refuse private/unpublished menu links, write only changed menu items
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:
@@ -62,4 +62,11 @@ final class MenuOperationsTest extends TestCase
|
||||
$result
|
||||
);
|
||||
}
|
||||
|
||||
public function testRemoveIgnoresParentId(): void
|
||||
{
|
||||
$result = (new MenuOperations())->apply($this->tree(), [['op' => 'remove', 'id' => 3, 'parent_id' => 99]]);
|
||||
|
||||
self::assertSame([1], array_column($result, 'id'));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -128,19 +128,77 @@ final class MenuSetItemsAbilityTest extends TestCase
|
||||
$items = [self::item(10, 0, 'Start'), self::item(11, 10, 'Team'), self::item(12, 0, 'Kontakt', 'custom', '/k')];
|
||||
$items[0]->url = 'https://example.test/';
|
||||
Functions\when('wp_get_nav_menu_items')->justReturn($items);
|
||||
Functions\when('wp_update_nav_menu_item')->alias(static fn (int $menu, int $id): int => $id);
|
||||
Functions\expect('wp_delete_post')->never();
|
||||
Functions\expect('wp_update_nav_menu_item')->never();
|
||||
$log = Mockery::mock(ChangeLog::class);
|
||||
$log->allows('record')->andReturn(23);
|
||||
$log->expects('record')->never();
|
||||
|
||||
$result = (new MenuSetItemsAbility(new MenuWriter(), new MenuOperations(), $log))->execute(
|
||||
['menu_id' => 3, 'items' => MenuTree::build($items)]
|
||||
);
|
||||
|
||||
self::assertStringContainsString('0 neu, 3 geändert, 0 entfernt', $result['summary']);
|
||||
self::assertSame('Menü "Hauptmenü": keine Änderung.', $result['summary']);
|
||||
self::assertSame([], $result['changed']);
|
||||
self::assertNull($result['log_id']);
|
||||
self::assertSame($result['previous_items'], $result['items']);
|
||||
}
|
||||
|
||||
public function testAllowUnpublishedIsPassedToThePlan(): void
|
||||
{
|
||||
$writer = Mockery::mock(MenuWriter::class);
|
||||
$writer->expects('plan')->with(Mockery::type('array'), Mockery::type('array'), true)
|
||||
->andReturn(['errors' => ['items.0' => 'x'], 'removals' => [], 'warnings' => []]);
|
||||
$ability = new MenuSetItemsAbility($writer, new MenuOperations(), Mockery::mock(ChangeLog::class));
|
||||
|
||||
$result = $ability->execute(
|
||||
['menu_id' => 3, 'items' => [['type' => 'post', 'object_id' => 7]], 'allow_unpublished' => true]
|
||||
);
|
||||
|
||||
self::assertSame('lit_validation_failed', $result->get_error_code());
|
||||
self::assertArrayHasKey('allow_unpublished', $ability->inputSchema()['properties']);
|
||||
}
|
||||
|
||||
public function testADraftLinkIsRefusedWithoutTheFlagAndWrittenWithAWarningWithIt(): void
|
||||
{
|
||||
Functions\when('current_user_can')->justReturn(true);
|
||||
Functions\when('get_post')->justReturn(
|
||||
new \WP_Post(['ID' => 7, 'post_type' => 'page', 'post_status' => 'draft', 'post_title' => 'Neu'])
|
||||
);
|
||||
Functions\when('get_post_type')->justReturn('page');
|
||||
Functions\expect('wp_update_nav_menu_item')->once()->andReturn(60);
|
||||
$log = Mockery::mock(ChangeLog::class);
|
||||
$log->allows('record')->andReturn(24);
|
||||
$ability = new MenuSetItemsAbility(new MenuWriter(), new MenuOperations(), $log);
|
||||
$input = ['menu_id' => 3, 'operations' => [['op' => 'add', 'item' => ['type' => 'post', 'object_id' => 7]]]];
|
||||
|
||||
$refused = $ability->execute($input);
|
||||
$allowed = $ability->execute($input + ['allow_unpublished' => true]);
|
||||
|
||||
self::assertSame('lit_validation_failed', $refused->get_error_code());
|
||||
self::assertStringContainsString('allow_unpublished', $refused->get_error_data()['fields']['items.1']);
|
||||
self::assertStringContainsString('1 neu', $allowed['summary']);
|
||||
self::assertStringContainsString('sofort im Menü sichtbar', $allowed['warnings'][0]);
|
||||
}
|
||||
|
||||
public function testAPrivatePostIsNeverLinked(): void
|
||||
{
|
||||
Functions\when('current_user_can')->justReturn(true);
|
||||
Functions\when('get_post')->justReturn(
|
||||
new \WP_Post(['ID' => 7, 'post_type' => 'page', 'post_status' => 'private', 'post_title' => 'Intern'])
|
||||
);
|
||||
Functions\expect('wp_update_nav_menu_item')->never();
|
||||
$ability = new MenuSetItemsAbility(new MenuWriter(), new MenuOperations(), Mockery::mock(ChangeLog::class));
|
||||
|
||||
$result = $ability->execute([
|
||||
'menu_id' => 3,
|
||||
'allow_unpublished' => true,
|
||||
'operations' => [['op' => 'add', 'item' => ['type' => 'post', 'object_id' => 7]]],
|
||||
]);
|
||||
|
||||
self::assertSame('lit_validation_failed', $result->get_error_code());
|
||||
self::assertStringContainsString('Private Inhalte', $result->get_error_data()['fields']['items.1']);
|
||||
}
|
||||
|
||||
public function testAWriteFailureIsPassedOnAsWriteFailed(): void
|
||||
{
|
||||
$writer = Mockery::mock(MenuWriter::class);
|
||||
@@ -190,5 +248,6 @@ final class MenuSetItemsAbilityTest extends TestCase
|
||||
self::assertSame(['readonly' => false, 'destructive' => true, 'idempotent' => false], $ability->annotations());
|
||||
self::assertStringContainsString('confirm: true', $ability->description());
|
||||
self::assertStringContainsString('nur nötig', $ability->description());
|
||||
self::assertStringContainsString('CSS-Klassen', $ability->description());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -65,7 +65,7 @@ final class MenuWriterTest extends TestCase
|
||||
['type' => 'post', 'object_id' => 7],
|
||||
['type' => 'custom', 'url' => 'javascript:alert(1)', 'title' => 'X'],
|
||||
['type' => 'custom', 'url' => '/kontakt'],
|
||||
], $this->current());
|
||||
], $this->current(), true);
|
||||
|
||||
self::assertStringContainsString('mehrfach', $plan['errors']['items.0.children.0']);
|
||||
self::assertStringContainsString('kein Eintrag dieses Menüs', $plan['errors']['items.1']);
|
||||
@@ -73,7 +73,45 @@ final class MenuWriterTest extends TestCase
|
||||
self::assertStringContainsString('url', $plan['errors']['items.3']);
|
||||
self::assertStringContainsString('title', $plan['errors']['items.4']);
|
||||
self::assertSame([['id' => 10, 'title' => 'Home']], $plan['removals']);
|
||||
self::assertStringContainsString('"Neu" ist nicht veröffentlicht', $plan['warnings'][0]);
|
||||
self::assertSame(
|
||||
['Eintrag "Neu" ist sofort im Menü sichtbar; Besucher erhalten bis zur Veröffentlichung einen toten Link.'],
|
||||
$plan['warnings']
|
||||
);
|
||||
}
|
||||
|
||||
/** @return array<string, array{0: string, 1: bool, 2: bool}> status, allow_unpublished, accepted */
|
||||
public static function statuses(): array
|
||||
{
|
||||
return [
|
||||
'published' => ['publish', false, true],
|
||||
'private is always refused' => ['private', true, false],
|
||||
'draft without flag' => ['draft', false, false],
|
||||
'pending without flag' => ['pending', false, false],
|
||||
'future without flag' => ['future', false, false],
|
||||
'draft with flag' => ['draft', true, true],
|
||||
'future with flag' => ['future', true, true],
|
||||
'trash' => ['trash', true, false],
|
||||
];
|
||||
}
|
||||
|
||||
/** @dataProvider statuses */
|
||||
public function testPostStatusDecidesWhetherItCanBeLinked(string $status, bool $allow, bool $accepted): void
|
||||
{
|
||||
Functions\when('current_user_can')->justReturn(true);
|
||||
Functions\when('get_post')->justReturn(
|
||||
new WP_Post(['ID' => 7, 'post_type' => 'page', 'post_status' => $status, 'post_title' => 'Seite'])
|
||||
);
|
||||
|
||||
$plan = (new MenuWriter())->plan([['type' => 'post', 'object_id' => 7]], [], $allow);
|
||||
|
||||
self::assertSame($accepted, ! isset($plan['errors']['items.0']), $plan['errors']['items.0'] ?? '');
|
||||
self::assertSame($accepted && $status !== 'publish' ? 1 : 0, count($plan['warnings']));
|
||||
if ($status === 'private') {
|
||||
self::assertStringContainsString('Private Inhalte', $plan['errors']['items.0']);
|
||||
}
|
||||
if (! $accepted && in_array($status, ['draft', 'pending', 'future'], true)) {
|
||||
self::assertStringContainsString('allow_unpublished', $plan['errors']['items.0']);
|
||||
}
|
||||
}
|
||||
|
||||
public function testPlanRejectsChildrenThatAreNoListAndNodesThatAreNoObject(): void
|
||||
@@ -92,7 +130,6 @@ final class MenuWriterTest extends TestCase
|
||||
Functions\when('get_term')->justReturn(new WP_Term(['term_id' => 4, 'taxonomy' => 'internal']));
|
||||
Functions\when('is_taxonomy_viewable')->justReturn(false);
|
||||
Functions\expect('current_user_can')->with('read_post', 7)->andReturn(false);
|
||||
Functions\expect('current_user_can')->with('edit_term', 4)->andReturn(false);
|
||||
|
||||
$plan = (new MenuWriter())->plan(
|
||||
[['type' => 'post', 'object_id' => 7], ['type' => 'taxonomy', 'object_id' => 4]],
|
||||
@@ -100,7 +137,10 @@ final class MenuWriterTest extends TestCase
|
||||
);
|
||||
|
||||
self::assertSame('object_id muss ein vorhandener, für dich lesbarer Eintrag sein.', $plan['errors']['items.0']);
|
||||
self::assertSame('object_id muss ein vorhandener, für dich lesbarer Begriff sein.', $plan['errors']['items.1']);
|
||||
self::assertSame(
|
||||
'object_id muss ein vorhandener Begriff einer öffentlichen Taxonomie sein.',
|
||||
$plan['errors']['items.1']
|
||||
);
|
||||
self::assertSame([], $plan['warnings']);
|
||||
}
|
||||
|
||||
@@ -126,6 +166,42 @@ final class MenuWriterTest extends TestCase
|
||||
self::assertSame(['items.1' => 'Ungültige url (erlaubt: /pfad, #anker, http(s), mailto, tel).'], $plan['errors']);
|
||||
}
|
||||
|
||||
/** @return array<string, array{0: string}> */
|
||||
public static function badUrls(): array
|
||||
{
|
||||
return [
|
||||
'mixed-case javascript' => ['JaVaScRiPt:alert(1)'],
|
||||
'embedded tab' => ["java\tscript:alert(1)"],
|
||||
'data' => ['data:text/html,<script>alert(1)</script>'],
|
||||
'vbscript' => ['vbscript:msgbox(1)'],
|
||||
'protocol-relative' => ['//evil.test/'],
|
||||
'protocol-relative backslash' => ['/\\evil.test/'],
|
||||
'leading space' => [' https://example.test/'],
|
||||
'control character' => ["https://example.test/\x00"],
|
||||
];
|
||||
}
|
||||
|
||||
/** @dataProvider badUrls */
|
||||
public function testDangerousUrlsAreRejected(string $url): void
|
||||
{
|
||||
$plan = (new MenuWriter())->plan([['type' => 'custom', 'url' => $url, 'title' => 'X']], []);
|
||||
|
||||
self::assertStringContainsString('url', $plan['errors']['items.0']);
|
||||
}
|
||||
|
||||
public function testSafeUrlsAreAccepted(): void
|
||||
{
|
||||
$plan = (new MenuWriter())->plan([
|
||||
['type' => 'custom', 'url' => '/kontakt', 'title' => 'A'],
|
||||
['type' => 'custom', 'url' => '#', 'title' => 'B'],
|
||||
['type' => 'custom', 'url' => 'HTTPS://example.test/', 'title' => 'C'],
|
||||
['type' => 'custom', 'url' => 'mailto:info@example.test', 'title' => 'D'],
|
||||
['type' => 'custom', 'url' => 'tel:+4912345', 'title' => 'E'],
|
||||
], []);
|
||||
|
||||
self::assertSame([], $plan['errors']);
|
||||
}
|
||||
|
||||
public function testIdZeroMeansANewItem(): void
|
||||
{
|
||||
$plan = (new MenuWriter())->plan([['id' => 0, 'type' => 'custom', 'url' => '#', 'title' => 'Mehr']], []);
|
||||
@@ -160,22 +236,34 @@ final class MenuWriterTest extends TestCase
|
||||
public function testMenuGetOutputRoundTripsAsANoOp(): void
|
||||
{
|
||||
Functions\expect('get_post')->never();
|
||||
Functions\expect('wp_update_nav_menu_item')->never();
|
||||
Functions\expect('wp_delete_post')->never();
|
||||
$current = $this->realistic();
|
||||
$tree = json_decode((string) json_encode(MenuTree::build(array_values($current))), true);
|
||||
$calls = [];
|
||||
Functions\when('wp_update_nav_menu_item')->alias(static function (int $menu, int $id, array $args) use (&$calls) {
|
||||
$calls[$id] = $args;
|
||||
return $id;
|
||||
});
|
||||
Functions\expect('wp_delete_post')->never();
|
||||
|
||||
$writer = new MenuWriter();
|
||||
$plan = $writer->plan($tree, $current);
|
||||
$stats = $writer->apply(3, $tree, $current);
|
||||
|
||||
self::assertSame(['errors' => [], 'removals' => [], 'warnings' => []], $plan);
|
||||
self::assertSame(['created' => 0, 'updated' => 0, 'removed' => 0], $stats);
|
||||
}
|
||||
|
||||
public function testOnlyMovedItemsAreRewrittenAndKeepTheirData(): void
|
||||
{
|
||||
$current = $this->realistic();
|
||||
$calls = [];
|
||||
Functions\when('wp_update_nav_menu_item')->alias(static function (int $menu, int $id, array $args) use (&$calls) {
|
||||
$calls[$id] = $args;
|
||||
return $id;
|
||||
});
|
||||
|
||||
// Kontakt(13) moves to the front: every item's position shifts by one.
|
||||
$tree = MenuTree::build(array_values($current));
|
||||
array_unshift($tree, array_pop($tree));
|
||||
$stats = (new MenuWriter())->apply(3, $tree, $current);
|
||||
|
||||
self::assertSame(['created' => 0, 'updated' => 4, 'removed' => 0], $stats);
|
||||
self::assertSame([10, 11, 12, 13], array_keys($calls));
|
||||
foreach ($current as $id => $item) {
|
||||
self::assertSame($item->post_title, $calls[$id]['menu-item-title'], "title of $id");
|
||||
self::assertSame((int) $item->menu_item_parent, $calls[$id]['menu-item-parent-id'], "parent of $id");
|
||||
@@ -183,7 +271,27 @@ final class MenuWriterTest extends TestCase
|
||||
}
|
||||
self::assertSame('', $calls[10]['menu-item-url']);
|
||||
self::assertSame('/kontakt', $calls[13]['menu-item-url']);
|
||||
self::assertSame([1, 2, 3, 4], array_column($calls, 'menu-item-position'));
|
||||
self::assertSame([1, 2, 3, 4], [$calls[13]['menu-item-position'], $calls[10]['menu-item-position'],
|
||||
$calls[11]['menu-item-position'], $calls[12]['menu-item-position']]);
|
||||
}
|
||||
|
||||
public function testMovingOnlyTheLastItemUnderAnotherRewritesOnlyThatItem(): void
|
||||
{
|
||||
$current = $this->realistic();
|
||||
$calls = [];
|
||||
Functions\when('wp_update_nav_menu_item')->alias(static function (int $menu, int $id, array $args) use (&$calls) {
|
||||
$calls[$id] = $args;
|
||||
return $id;
|
||||
});
|
||||
|
||||
$stats = (new MenuWriter())->apply(3, [
|
||||
['id' => 10],
|
||||
['id' => 11, 'children' => [['id' => 12], ['id' => 13]]],
|
||||
], $current);
|
||||
|
||||
self::assertSame(['created' => 0, 'updated' => 1, 'removed' => 0], $stats);
|
||||
self::assertSame([13], array_keys($calls));
|
||||
self::assertSame(11, $calls[13]['menu-item-parent-id']);
|
||||
}
|
||||
|
||||
public function testChangedTitleOfAnExistingItemBecomesItsLabel(): void
|
||||
@@ -201,8 +309,8 @@ final class MenuWriterTest extends TestCase
|
||||
['id' => 13, 'title' => 'Kontakt', 'url' => 'mailto:info@example.test'],
|
||||
], $current);
|
||||
|
||||
self::assertSame([10, 13], array_keys($calls));
|
||||
self::assertSame('Startseite', $calls[10]['menu-item-title']);
|
||||
self::assertSame('Wir über uns', $calls[11]['menu-item-title']);
|
||||
self::assertSame('mailto:info@example.test', $calls[13]['menu-item-url']);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user