Skip to content

fix(NavigationManager): only resolve navigations of booted apps - #61120

Merged
CarlSchwan merged 3 commits into
masterfrom
fix/navigation-manager
Jul 23, 2026
Merged

fix(NavigationManager): only resolve navigations of booted apps#61120
CarlSchwan merged 3 commits into
masterfrom
fix/navigation-manager

Conversation

@susnux

@susnux susnux commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Prevent log flooded with "route not found"

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@susnux
susnux requested a review from come-nc June 9, 2026 16:07
@susnux
susnux requested a review from a team as a code owner June 9, 2026 16:07
@susnux
susnux requested review from leftybournes, provokateurin and salmart-dev and removed request for a team June 9, 2026 16:07
@susnux susnux added bug 3. to review Waiting for reviews labels Jun 9, 2026
@susnux susnux added this to the Nextcloud 35 milestone Jun 9, 2026
@come-nc

This comment was marked as resolved.

Comment thread lib/private/NavigationManager.php Outdated
@susnux
susnux marked this pull request as draft June 10, 2026 15:42
@susnux

susnux commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

We discovered more issues, will work on it.

@susnux
susnux force-pushed the fix/navigation-manager branch 4 times, most recently from 281ccd1 to e870c6f Compare June 22, 2026 18:37
Comment thread apps/profile/lib/Listener/LoadAdditionalEntriesListener.php Outdated
Comment thread apps/profile/lib/Listener/LoadAdditionalEntriesListener.php Outdated
Comment thread apps/profile/lib/Listener/LoadAdditionalEntriesListener.php Outdated
Comment thread apps/settings/lib/Listener/LoadAdditionalEntriesListener.php Outdated
Comment thread apps/settings/lib/Listener/LoadAdditionalEntriesListener.php Outdated
Comment thread apps/settings/lib/Listener/LoadAdditionalEntriesListener.php Outdated
Comment thread core/Listener/LoadAdditionalEntriesListener.php Outdated
Comment thread core/Listener/LoadAdditionalEntriesListener.php
Comment thread core/Listener/LoadAdditionalEntriesListener.php Outdated
Comment thread lib/private/NavigationManager.php Outdated
Comment thread lib/private/NavigationManager.php Outdated
@susnux susnux added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jun 27, 2026
@susnux
susnux force-pushed the fix/navigation-manager branch 2 times, most recently from b1d022e to 2cee396 Compare July 19, 2026 11:43
Comment thread lib/private/NavigationManager.php
@susnux
susnux marked this pull request as ready for review July 21, 2026 13:10
@susnux
susnux force-pushed the fix/navigation-manager branch from 2cee396 to f509595 Compare July 21, 2026 13:10
@susnux
susnux requested a review from blizzz July 21, 2026 13:10
@susnux susnux added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 21, 2026
susnux added 3 commits July 22, 2026 20:32
…ps loaded

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
…tion entries

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
susnux force-pushed the fix/navigation-manager branch from f509595 to b47622e Compare July 22, 2026 18:34
@susnux

susnux commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

(rebased for psalm issue)

@CarlSchwan
CarlSchwan enabled auto-merge July 23, 2026 11:16
@CarlSchwan
CarlSchwan merged commit 182e403 into master Jul 23, 2026
221 of 233 checks passed
@CarlSchwan
CarlSchwan deleted the fix/navigation-manager branch July 23, 2026 11:24
@blizzz

blizzz commented Jul 23, 2026

Copy link
Copy Markdown
Member

This causes a regression in tables (e.g. failing tests here and here)

Mentioned yesterday in internal chat:

but… now our Tables Applications are not listed in the Waffle menu anymore. 🎭

we listen to BeforeTemplateRenderedEvent and add our items then to the navigation, cf https://github.com/nextcloud/tables/blob/main/lib/Listener/BeforeTemplateRenderedListener.php

Is this expected @susnux, and if so what to do about it?

cc / fyi @enjeck

@susnux

susnux commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

@blizzz / @enjeck

No this is not expected I will fix this. But also its not correct to register in BeforeTemplateRenderedEvent thats far too late and will also cause other problems when apps need to know which entries are available before that.
You should migrate to OCP\Navigation\Events\LoadAdditionalEntriesEvent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Settings menu broken for LDAP users (on current master only)

4 participants