nextcloud
Use NavigationManager instead of AppManager to handle custom apps order
- Resolves: https://github.com/nextcloud/server/issues/47479
Summary
The custom defaults "apps" and order were using the apps and not navigation entries (see the linked issue for more details). The confusion lead to a few problems that can only be solved by only using the NavigationManager and not the AppManager. Most of the logic is the same, except for checking against available navigation entries and not apps.
I tried touching as little as possible, all the frontend code is still the same and uses the incorrect "app" over "navigation entry" concept. On the frontend it still makes a bit sense, since the concept is a lot easier to grasp for users. The config keys on the backend are kept the same to avoid migrations and deprecations.
In theory one could spend a lot more time on trying to fix this, but I already tried that a bit and it is a rabbit hole that is too deep for fixing and backporting.
The last commit is a fix that is useful even without all the other changes. In case an app is disabled while also being used for a custom order, the frontend would display "undefined" entries.
As for backporting, I kept the old broken interface by providing a proxy implementation. I hope this is good enough to be backported.
Checklist
- Code is properly formatted
- Sign-off message is added to all commits
- [ ] Tests (unit, integration, api and/or acceptance) are included
- [ ] Screenshots before/after for front-end changes
- [ ] Documentation (manuals or wiki) has been updated or is not required
- [ ] Backports requested where applicable (ex: critical bugfixes)
Is the comment in https://github.com/nextcloud/server/blob/558877c8422c1a7083d083b2fe0be553f3047ba0/config/config.sample.php#L1169-L1178 still true after your PR is merged?
What happens if you actually set an external site as default entry, you cannot use Nextcloud anymore? (login would redirect to external site, no?)
It still works, even if the user doesn't have access because all invalid entries are filtered out (e.g. because the external site is only allowed for a group the user is not part of). I think this might actually be broken on master, since only my last commit actually does this check.
still true after your PR is merged?
No, it has to be updated. I'm not sure thought how the admin could effectively get the navigation entry id without poking the API :thinking:
Cypress failure was because of a deeper issue in the backend side which I also fixed.
Fix will not be backported to any version due to the new interfaces and the close release of 30.
Cypress issues are resolved (caused by bugs in the backend). @susnux can you review again?
This actually broke the guests app, which is going nasty stuff: https://github.com/nextcloud/guests/blob/master/lib/FilteredNavigationManager.php
Can you fix it or raise a ticket there?
