From ad8f1c9311e25ec761c8d834e06d5178996b628a Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Sun, 27 Sep 2026 17:44:00 +0200 Subject: [PATCH] feat(navigation): let apps remove navigation entries Apps can add navigation entries (LoadAdditionalEntriesEvent) but not take any away. An app that gives some users access to itself only, like the Social app's self-registered external users, then has to hide the app menu with CSS while the page still carries every app in its initial state. NavigationManager::getAll() now dispatches NavigationEntriesFilterEvent with the entries of the requested type before sorting them. A listener can only remove entries: entries it adds or changes are ignored, so the event cannot be used to rewrite another app's link. The event is only dispatched when an app listens for it. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/NavigationManager.php | 8 +++ .../Events/NavigationEntriesFilterEvent.php | 61 +++++++++++++++++++ tests/lib/NavigationManagerTest.php | 46 ++++++++++++++ 5 files changed, 117 insertions(+) create mode 100644 lib/public/Navigation/Events/NavigationEntriesFilterEvent.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 255465c0ffb2e..1d47158fb55d7 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -805,6 +805,7 @@ 'OCP\\Migration\\IRepairStepExpensive' => $baseDir . '/lib/public/Migration/IRepairStepExpensive.php', 'OCP\\Migration\\SimpleMigrationStep' => $baseDir . '/lib/public/Migration/SimpleMigrationStep.php', 'OCP\\Navigation\\Events\\LoadAdditionalEntriesEvent' => $baseDir . '/lib/public/Navigation/Events/LoadAdditionalEntriesEvent.php', + 'OCP\\Navigation\\Events\\NavigationEntriesFilterEvent' => $baseDir . '/lib/public/Navigation/Events/NavigationEntriesFilterEvent.php', 'OCP\\Notification\\AlreadyProcessedException' => $baseDir . '/lib/public/Notification/AlreadyProcessedException.php', 'OCP\\Notification\\IAction' => $baseDir . '/lib/public/Notification/IAction.php', 'OCP\\Notification\\IApp' => $baseDir . '/lib/public/Notification/IApp.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 046d2cd20b693..5a23f98113aa8 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -846,6 +846,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\Migration\\IRepairStepExpensive' => __DIR__ . '/../../..' . '/lib/public/Migration/IRepairStepExpensive.php', 'OCP\\Migration\\SimpleMigrationStep' => __DIR__ . '/../../..' . '/lib/public/Migration/SimpleMigrationStep.php', 'OCP\\Navigation\\Events\\LoadAdditionalEntriesEvent' => __DIR__ . '/../../..' . '/lib/public/Navigation/Events/LoadAdditionalEntriesEvent.php', + 'OCP\\Navigation\\Events\\NavigationEntriesFilterEvent' => __DIR__ . '/../../..' . '/lib/public/Navigation/Events/NavigationEntriesFilterEvent.php', 'OCP\\Notification\\AlreadyProcessedException' => __DIR__ . '/../../..' . '/lib/public/Notification/AlreadyProcessedException.php', 'OCP\\Notification\\IAction' => __DIR__ . '/../../..' . '/lib/public/Notification/IAction.php', 'OCP\\Notification\\IApp' => __DIR__ . '/../../..' . '/lib/public/Notification/IApp.php', diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php index 2bdb8bebaf788..394ac479fff01 100644 --- a/lib/private/NavigationManager.php +++ b/lib/private/NavigationManager.php @@ -19,6 +19,7 @@ use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Navigation\Events\LoadAdditionalEntriesEvent; +use OCP\Navigation\Events\NavigationEntriesFilterEvent; use Override; use Psr\Log\LoggerInterface; @@ -154,6 +155,13 @@ public function getAll(string $type = 'link'): array { }); } + if ($this->eventDispatcher->hasListeners(NavigationEntriesFilterEvent::class)) { + $event = new NavigationEntriesFilterEvent($result, $type); + $this->eventDispatcher->dispatchTyped($event); + // a listener removes entries; adding one is LoadAdditionalEntriesEvent's job + $result = array_intersect_key($result, $event->getEntries()); + } + return $this->proceedNavigation($result, $type); } diff --git a/lib/public/Navigation/Events/NavigationEntriesFilterEvent.php b/lib/public/Navigation/Events/NavigationEntriesFilterEvent.php new file mode 100644 index 0000000000000..5ca91b61e2207 --- /dev/null +++ b/lib/public/Navigation/Events/NavigationEntriesFilterEvent.php @@ -0,0 +1,61 @@ + $entries the entries, keyed by their id + * @param string $type the type that was asked for, see INavigationManager::TYPE_* + * @since 36.0.0 + */ + public function __construct( + private array $entries, + private string $type, + ) { + parent::__construct(); + } + + /** + * @return array the entries, keyed by their id + * @since 36.0.0 + */ + public function getEntries(): array { + return $this->entries; + } + + /** + * @param array $entries the entries to keep, keyed by their id + * @since 36.0.0 + */ + public function setEntries(array $entries): void { + $this->entries = $entries; + } + + /** + * The type of entries that was asked for, see INavigationManager::TYPE_* + * + * @since 36.0.0 + */ + public function getType(): string { + return $this->type; + } +} diff --git a/tests/lib/NavigationManagerTest.php b/tests/lib/NavigationManagerTest.php index fc84c7b51a7f8..c5f5c28d7e767 100644 --- a/tests/lib/NavigationManagerTest.php +++ b/tests/lib/NavigationManagerTest.php @@ -21,6 +21,7 @@ use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Navigation\Events\LoadAdditionalEntriesEvent; +use OCP\Navigation\Events\NavigationEntriesFilterEvent; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -244,6 +245,51 @@ public function testGetAllFiltersActions(): void { $this->assertEquals(['files', 'logout'], array_keys($this->navigationManager->getAll(INavigationManager::TYPE_ALL))); } + public function testGetAllLetsListenersRemoveEntries(): void { + $this->navigationManager->add(['id' => 'files', 'name' => 'Files', 'order' => 1, 'href' => 'url']); + $this->navigationManager->add(['id' => 'social', 'name' => 'Social', 'order' => 2, 'href' => 'url']); + $this->navigationManager->add(['id' => 'logout', 'name' => 'Log out', 'order' => 3, 'href' => 'url', 'type' => INavigationManager::TYPE_ACTION]); + + $types = []; + $this->dispatcher->method('hasListeners')->with(NavigationEntriesFilterEvent::class)->willReturn(true); + $this->dispatcher->method('dispatchTyped')->willReturnCallback(function ($event) use (&$types): void { + $this->assertInstanceOf(NavigationEntriesFilterEvent::class, $event); + $types[] = $event->getType(); + $entries = $event->getEntries(); + unset($entries['files']); + $event->setEntries($entries); + }); + + $this->assertEquals(['social'], array_keys($this->navigationManager->getAll())); + $this->assertEquals(['social', 'logout'], array_keys($this->navigationManager->getAll(INavigationManager::TYPE_ALL))); + $this->assertEquals([INavigationManager::TYPE_APPS, INavigationManager::TYPE_ALL], $types); + } + + public function testGetAllIgnoresEntriesAListenerAddsOrChanges(): void { + $this->navigationManager->add(['id' => 'files', 'name' => 'Files', 'order' => 1, 'href' => 'url']); + + $this->dispatcher->method('hasListeners')->willReturn(true); + $this->dispatcher->method('dispatchTyped')->willReturnCallback(function (NavigationEntriesFilterEvent $event): void { + $entries = $event->getEntries(); + $entries['files']['href'] = 'https://evil.example'; + $entries['extra'] = ['id' => 'extra', 'name' => 'Extra', 'order' => 0, 'href' => 'url', 'type' => 'link', 'active' => false]; + $event->setEntries($entries); + }); + + $all = $this->navigationManager->getAll(); + $this->assertEquals(['files'], array_keys($all)); + $this->assertEquals('url', $all['files']['href']); + } + + public function testGetAllDispatchesNoFilterEventWithoutListeners(): void { + $this->navigationManager->add(['id' => 'files', 'name' => 'Files', 'order' => 1, 'href' => 'url']); + + $this->dispatcher->method('hasListeners')->willReturn(false); + $this->dispatcher->expects($this->never())->method('dispatchTyped'); + + $this->assertEquals(['files'], array_keys($this->navigationManager->getAll())); + } + public function testAddArrayClearGetAll(): void { $entry = [ 'id' => 'entry id',