Skip to content

Commit 50b67c1

Browse files
sorbaughsorbaugh
authored
Merge pull request #60458 from nextcloud/refactor/navigation-manager
refactor(NavigationManager): move navigation definitions into apps
2 parents 0c3f22e + 0abaf60 commit 50b67c1

14 files changed

Lines changed: 248 additions & 324 deletions

File tree

‎apps/appstore/appinfo/info.xml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,21 @@
1919
<dependencies>
2020
<nextcloud min-version="35" max-version="35"/>
2121
</dependencies>
22+
23+
<navigations>
24+
<navigation role="admin">
25+
<name>Appstore</name>
26+
<route>appstore.page.viewApps</route>
27+
<icon>app.svg</icon>
28+
<order>99</order>
29+
</navigation>
30+
31+
<navigation role="admin">
32+
<name>Apps</name>
33+
<route>appstore.page.viewApps</route>
34+
<icon>app.svg</icon>
35+
<order>5</order>
36+
<type>settings</type>
37+
</navigation>
38+
</navigations>
2239
</info>

‎apps/appstore/lib/Controller/PageController.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
use OCP\AppFramework\Services\IInitialState;
2424
use OCP\IConfig;
2525
use OCP\IL10N;
26-
use OCP\INavigationManager;
2726
use OCP\IRequest;
2827
use OCP\IURLGenerator;
2928
use OCP\Server;
@@ -41,7 +40,6 @@ public function __construct(
4140
private readonly IURLGenerator $urlGenerator,
4241
private readonly IInitialState $initialState,
4342
private readonly BundleFetcher $bundleFetcher,
44-
private readonly INavigationManager $navigationManager,
4543
) {
4644
parent::__construct(Application::APP_ID, $request);
4745
}
@@ -51,8 +49,6 @@ public function __construct(
5149
#[FrontpageRoute(verb: 'GET', url: '/settings/apps/{category}', defaults: ['category' => ''], root: '')]
5250
#[FrontpageRoute(verb: 'GET', url: '/settings/apps/{category}/{id}', defaults: ['category' => '', 'id' => ''], root: '')]
5351
public function viewApps(): TemplateResponse {
54-
$this->navigationManager->setActiveEntry('core_apps');
55-
5652
$this->initialState->provideInitialState('appstoreEnabled', $this->config->getSystemValueBool('appstoreenabled', true));
5753
$this->initialState->provideInitialState('appstoreBundles', $this->getBundles());
5854
$this->initialState->provideInitialState('appstoreDeveloperDocs', $this->urlGenerator->linkToDocs('developer-manual'));

‎apps/appstore/tests/Controller/PageControllerTest.php‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
use OCP\AppFramework\Services\IInitialState;
1818
use OCP\IConfig;
1919
use OCP\IL10N;
20-
use OCP\INavigationManager;
2120
use OCP\IRequest;
2221
use OCP\IURLGenerator;
2322
use PHPUnit\Framework\MockObject\MockObject;
@@ -31,8 +30,6 @@ final class PageControllerTest extends TestCase {
3130

3231
private IConfig&MockObject $config;
3332

34-
private INavigationManager&MockObject $navigationManager;
35-
3633
private IAppManager&MockObject $appManager;
3734

3835
private BundleFetcher&MockObject $bundleFetcher;
@@ -55,7 +52,6 @@ protected function setUp(): void {
5552
->method('t')
5653
->willReturnArgument(0);
5754
$this->config = $this->createMock(IConfig::class);
58-
$this->navigationManager = $this->createMock(INavigationManager::class);
5955
$this->appManager = $this->createMock(IAppManager::class);
6056
$this->bundleFetcher = $this->createMock(BundleFetcher::class);
6157
$this->installer = $this->createMock(Installer::class);
@@ -71,7 +67,6 @@ protected function setUp(): void {
7167
$this->urlGenerator,
7268
$this->initialState,
7369
$this->bundleFetcher,
74-
$this->navigationManager,
7570
);
7671
}
7772

@@ -85,10 +80,6 @@ public function testViewApps(): void {
8580
->method('getSystemValueBool')
8681
->with('appstoreenabled', true)
8782
->willReturn(true);
88-
$this->navigationManager
89-
->expects($this->once())
90-
->method('setActiveEntry')
91-
->with('core_apps');
9283

9384
$this->initialState
9485
->expects($this->exactly(4))
@@ -118,10 +109,6 @@ public function testViewAppsAppstoreNotEnabled(): void {
118109
->method('getSystemValueBool')
119110
->with('appstoreenabled', true)
120111
->willReturn(false);
121-
$this->navigationManager
122-
->expects($this->once())
123-
->method('setActiveEntry')
124-
->with('core_apps');
125112

126113
$this->initialState
127114
->expects($this->exactly(4))

‎apps/profile/lib/AppInfo/Application.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616
use OCP\AppFramework\Bootstrap\IBootstrap;
1717
use OCP\AppFramework\Bootstrap\IRegistrationContext;
1818
use OCP\Collaboration\Reference\RenderReferenceEvent;
19+
use OCP\INavigationManager;
20+
use OCP\IURLGenerator;
21+
use OCP\IUserSession;
22+
use OCP\L10N\IFactory;
23+
use OCP\Server;
1924

2025
class Application extends App implements IBootstrap {
2126
public const APP_ID = 'profile';
@@ -32,5 +37,33 @@ public function register(IRegistrationContext $context): void {
3237

3338
#[\Override]
3439
public function boot(IBootContext $context): void {
40+
$context->injectFn($this->registerNavigationEntry(...));
41+
}
42+
43+
/**
44+
* Registers the navigation entry for the profile app in the user settings.
45+
* Needed as the href is dynamic and thus we cannot use the appinfo/info.xml
46+
*/
47+
public function registerNavigationEntry(
48+
INavigationManager $navigationManager,
49+
IUserSession $userSession,
50+
IURLGenerator $urlGenerator,
51+
): void {
52+
if (!$userSession->isLoggedIn()) {
53+
return;
54+
}
55+
56+
$l = Server::get(IFactory::class)->get('profile');
57+
// Profile
58+
$navigationManager->add([
59+
'type' => 'settings',
60+
'id' => 'profile',
61+
'order' => 1,
62+
'href' => $urlGenerator->linkToRoute(
63+
'profile.ProfilePage.index',
64+
['targetUserId' => $userSession->getUser()->getUID()],
65+
),
66+
'name' => $l->t('View profile'),
67+
]);
3568
}
3669
}

‎apps/settings/appinfo/info.xml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,5 @@
7474
<provider>OCA\Settings\Activity\Provider</provider>
7575
<provider>OCA\Settings\Activity\SecurityProvider</provider>
7676
</providers>
77-
7877
</activity>
7978
</info>

‎apps/settings/appinfo/routes.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
['name' => 'CheckSetup#getFailedIntegrityCheckFiles', 'url' => '/settings/integrity/failed', 'verb' => 'GET' , 'root' => ''],
3434
['name' => 'CheckSetup#rescanFailedIntegrityCheck', 'url' => '/settings/integrity/rescan', 'verb' => 'GET' , 'root' => ''],
3535
['name' => 'PersonalSettings#index', 'url' => '/settings/user/{section}', 'verb' => 'GET', 'defaults' => ['section' => 'personal-info'] , 'root' => ''],
36-
['name' => 'AdminSettings#index', 'url' => '/settings/admin/{section}', 'verb' => 'GET', 'defaults' => ['section' => 'server'] , 'root' => ''],
36+
['name' => 'AdminSettings#index', 'url' => '/settings/admin/{section}', 'verb' => 'GET', 'defaults' => ['section' => 'overview'] , 'root' => ''],
3737
['name' => 'AdminSettings#form', 'url' => '/settings/admin/{section}', 'verb' => 'GET' , 'root' => ''],
3838
['name' => 'ChangePassword#changePersonalPassword', 'url' => '/settings/personal/changepassword', 'verb' => 'POST' , 'root' => ''],
3939
['name' => 'ChangePassword#changeUserPassword', 'url' => '/settings/users/changepassword', 'verb' => 'POST' , 'root' => ''],

‎apps/settings/lib/AppInfo/Application.php‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,12 @@
9191
use OCP\Group\Events\GroupDeletedEvent;
9292
use OCP\Group\Events\UserAddedEvent;
9393
use OCP\Group\Events\UserRemovedEvent;
94+
use OCP\Group\ISubAdmin;
9495
use OCP\IConfig;
96+
use OCP\IGroupManager;
97+
use OCP\INavigationManager;
9598
use OCP\IURLGenerator;
99+
use OCP\IUserSession;
96100
use OCP\L10N\IFactory;
97101
use OCP\Mail\IMailer;
98102
use OCP\Security\ICrypto;
@@ -227,5 +231,93 @@ public function register(IRegistrationContext $context): void {
227231

228232
#[\Override]
229233
public function boot(IBootContext $context): void {
234+
$context->injectFn($this->registerNavigationEntries(...));
235+
}
236+
237+
/**
238+
* Registers the navigation entries for the user settings.
239+
* Needed as some entries are dynamic and thus we cannot use the appinfo/info.xml
240+
*
241+
* Registers the following entries:
242+
* - Appearance and accessibility
243+
* - Personal settings (named "Settings" for non-admins)
244+
* - Accounts (only for subadmins)
245+
* - Help & privacy (conditionally enabled based on config)
246+
*/
247+
public function registerNavigationEntries(
248+
INavigationManager $navigationManager,
249+
IURLGenerator $urlGenerator,
250+
IUserSession $userSession,
251+
IConfig $config,
252+
): void {
253+
if ($userSession->getUser() === null) {
254+
return;
255+
}
256+
257+
$l = Server::get(IFactory::class)
258+
->get('settings');
259+
$groupManager = Server::get(IGroupManager::class);
260+
$subAdmin = Server::get(ISubAdmin::class);
261+
$isAdmin = $groupManager->isAdmin($userSession->getUser()->getUID());
262+
$isSubAdmin = $subAdmin->isSubAdmin($userSession->getUser());
263+
264+
// Accessibility settings - the URL is dynamic (route parameters) which is currently not supported by appinfo.xml
265+
$navigationManager->add([
266+
'type' => 'settings',
267+
'id' => 'accessibility_settings',
268+
'order' => 2,
269+
'href' => $urlGenerator->linkToRoute('settings.PersonalSettings.index', ['section' => 'theming']),
270+
'name' => $l->t('Appearance and accessibility'),
271+
'icon' => $urlGenerator->imagePath('theming', 'accessibility-dark.svg'),
272+
]);
273+
274+
// Personal settings - this entry is dynamic so we cannot use appinfo
275+
$navigationManager->add([
276+
'type' => 'settings',
277+
'id' => 'settings_personal',
278+
'order' => 3,
279+
'href' => $urlGenerator->linkToRoute('settings.PersonalSettings.index'),
280+
'name' => $isAdmin
281+
? $l->t('Personal settings')
282+
: $l->t('Settings'),
283+
'icon' => $isAdmin
284+
? $urlGenerator->imagePath('settings', 'personal.svg')
285+
: $urlGenerator->imagePath('settings', 'admin.svg'),
286+
]);
287+
288+
if ($isAdmin) {
289+
$navigationManager->add([
290+
'type' => 'settings',
291+
'id' => 'settings_administration',
292+
'order' => 4,
293+
'href' => $urlGenerator->linkToRoute('settings.adminSettings.index'),
294+
'name' => $l->t('Administration settings'),
295+
'icon' => $urlGenerator->imagePath('settings', 'admin.svg'),
296+
]);
297+
}
298+
299+
// User management is conditionally enabled for subadmins, but appinfo currently only supports full admins
300+
if ($isSubAdmin) {
301+
$navigationManager->add([
302+
'type' => 'settings',
303+
'id' => 'core_users',
304+
'order' => 6,
305+
'href' => $urlGenerator->linkToRoute('settings.Users.usersList'),
306+
'name' => $l->t('Accounts'),
307+
'icon' => $urlGenerator->imagePath('settings', 'users.svg'),
308+
]);
309+
}
310+
311+
// conditionally enabled navigation entry
312+
if ($config->getSystemValueBool('knowledgebaseenabled', true)) {
313+
$navigationManager->add([
314+
'type' => 'settings',
315+
'id' => 'help',
316+
'order' => 99998,
317+
'href' => $urlGenerator->linkToRoute('settings.Help.help'),
318+
'name' => $l->t('Help & privacy'),
319+
'icon' => $urlGenerator->imagePath('settings', 'help.svg'),
320+
]);
321+
}
230322
}
231323
}

‎core/AppInfo/Application.php‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@
3434
use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
3535
use OCP\DB\Events\AddMissingIndicesEvent;
3636
use OCP\DB\Events\AddMissingPrimaryKeyEvent;
37+
use OCP\INavigationManager;
38+
use OCP\IURLGenerator;
39+
use OCP\IUserSession;
40+
use OCP\L10N\IFactory;
41+
use OCP\Server;
3742
use OCP\User\Events\BeforeUserDeletedEvent;
3843
use OCP\User\Events\PasswordUpdatedEvent;
3944
use OCP\User\Events\UserDeletedEvent;
@@ -96,7 +101,36 @@ public function register(IRegistrationContext $context): void {
96101

97102
#[\Override]
98103
public function boot(IBootContext $context): void {
99-
// ...
104+
$context->injectFn($this->registerNavigationEntries(...));
105+
}
106+
107+
/**
108+
* Registers the navigation entries for the core app:
109+
* - The logout button in the settings menu
110+
*/
111+
public function registerNavigationEntries(
112+
INavigationManager $navigationManager,
113+
IUserSession $userSession,
114+
IURLGenerator $urlGenerator,
115+
): void {
116+
if (!$userSession->isLoggedIn()) {
117+
return;
118+
}
119+
120+
$l = Server::get(IFactory::class)->get('core');
121+
122+
// Register the logout button in the user settings
123+
$logoutUrl = \OC_User::getLogoutUrl($urlGenerator);
124+
if ($logoutUrl !== '') {
125+
$navigationManager->add([
126+
'type' => 'settings',
127+
'id' => 'logout',
128+
'order' => 99999,
129+
'href' => $logoutUrl,
130+
'name' => $l->t('Log out'),
131+
'icon' => $urlGenerator->imagePath('core', 'actions/logout.svg'),
132+
]);
133+
}
100134
}
101135

102136
}

‎cypress/e2e/systemtags/admin-settings.cy.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ describe('Create system tags', () => {
2121

2222
// login as admin and go to admin settings
2323
cy.login(admin)
24-
cy.visit('/settings/admin')
24+
cy.visit('/settings/admin/server')
2525
})
2626

2727
it('Can create a tag', () => {
@@ -48,7 +48,7 @@ describe('Create system tags', () => {
4848
describe('Update system tags', { testIsolation: false }, () => {
4949
before(() => {
5050
cy.login(admin)
51-
cy.visit('/settings/admin')
51+
cy.visit('/settings/admin/server')
5252
})
5353

5454
it('select the tag', () => {
@@ -92,7 +92,7 @@ describe('Update system tags', { testIsolation: false }, () => {
9292
describe('Delete system tags', { testIsolation: false }, () => {
9393
before(() => {
9494
cy.login(admin)
95-
cy.visit('/settings/admin')
95+
cy.visit('/settings/admin/server')
9696
})
9797

9898
it('select the tag', () => {

‎cypress/e2e/theming/admin-settings_branding.cy.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ describe('Admin theming: Change the login fields then reset them', function() {
154154

155155
it('See the admin theming section', function() {
156156
cy.visit('/settings/admin/theming')
157-
cy.findByRole('heading', { name: /^Theming/ })
157+
cy.findByRole('heading', { name: /^Theming/, level: 2 })
158158
.should('exist')
159159
.scrollIntoView()
160160
})

0 commit comments

Comments
 (0)