Skip to content

Commit 7f6df7e

Browse files
skjnldsvskjnldsv
authored
Merge pull request #61008 from nextcloud/revert-60961-backport/60458/stable34
[stable34] Revert "refactor(NavigationManager): move navigation definitions into apps"
2 parents ac8cbda + 677044d commit 7f6df7e

14 files changed

Lines changed: 324 additions & 248 deletions

File tree

‎apps/appstore/appinfo/info.xml‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,4 @@
1919
<dependencies>
2020
<nextcloud min-version="34" max-version="34"/>
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>
3922
</info>

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use OCP\AppFramework\Services\IInitialState;
2424
use OCP\IConfig;
2525
use OCP\IL10N;
26+
use OCP\INavigationManager;
2627
use OCP\IRequest;
2728
use OCP\IURLGenerator;
2829
use OCP\Server;
@@ -40,6 +41,7 @@ public function __construct(
4041
private readonly IURLGenerator $urlGenerator,
4142
private readonly IInitialState $initialState,
4243
private readonly BundleFetcher $bundleFetcher,
44+
private readonly INavigationManager $navigationManager,
4345
) {
4446
parent::__construct(Application::APP_ID, $request);
4547
}
@@ -49,6 +51,8 @@ public function __construct(
4951
#[FrontpageRoute(verb: 'GET', url: '/settings/apps/{category}', defaults: ['category' => ''], root: '')]
5052
#[FrontpageRoute(verb: 'GET', url: '/settings/apps/{category}/{id}', defaults: ['category' => '', 'id' => ''], root: '')]
5153
public function viewApps(): TemplateResponse {
54+
$this->navigationManager->setActiveEntry('core_apps');
55+
5256
$this->initialState->provideInitialState('appstoreEnabled', $this->config->getSystemValueBool('appstoreenabled', true));
5357
$this->initialState->provideInitialState('appstoreBundles', $this->getBundles());
5458
$this->initialState->provideInitialState('appstoreDeveloperDocs', $this->urlGenerator->linkToDocs('developer-manual'));

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use OCP\AppFramework\Services\IInitialState;
1717
use OCP\IConfig;
1818
use OCP\IL10N;
19+
use OCP\INavigationManager;
1920
use OCP\IRequest;
2021
use OCP\IURLGenerator;
2122
use PHPUnit\Framework\MockObject\MockObject;
@@ -29,6 +30,8 @@ final class PageControllerTest extends TestCase {
2930

3031
private IConfig&MockObject $config;
3132

33+
private INavigationManager&MockObject $navigationManager;
34+
3235
private IAppManager&MockObject $appManager;
3336

3437
private BundleFetcher&MockObject $bundleFetcher;
@@ -51,6 +54,7 @@ protected function setUp(): void {
5154
->method('t')
5255
->willReturnArgument(0);
5356
$this->config = $this->createMock(IConfig::class);
57+
$this->navigationManager = $this->createMock(INavigationManager::class);
5458
$this->appManager = $this->createMock(IAppManager::class);
5559
$this->bundleFetcher = $this->createMock(BundleFetcher::class);
5660
$this->installer = $this->createMock(Installer::class);
@@ -66,6 +70,7 @@ protected function setUp(): void {
6670
$this->urlGenerator,
6771
$this->initialState,
6872
$this->bundleFetcher,
73+
$this->navigationManager,
6974
);
7075
}
7176

@@ -79,6 +84,10 @@ public function testViewApps(): void {
7984
->method('getSystemValueBool')
8085
->with('appstoreenabled', true)
8186
->willReturn(true);
87+
$this->navigationManager
88+
->expects($this->once())
89+
->method('setActiveEntry')
90+
->with('core_apps');
8291

8392
$this->initialState
8493
->expects($this->exactly(4))
@@ -108,6 +117,10 @@ public function testViewAppsAppstoreNotEnabled(): void {
108117
->method('getSystemValueBool')
109118
->with('appstoreenabled', true)
110119
->willReturn(false);
120+
$this->navigationManager
121+
->expects($this->once())
122+
->method('setActiveEntry')
123+
->with('core_apps');
111124

112125
$this->initialState
113126
->expects($this->exactly(4))

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

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,6 @@
1616

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;
2419

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

3833
#[\Override]
3934
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-
]);
6835
}
6936
}

‎apps/settings/appinfo/info.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,5 +74,6 @@
7474
<provider>OCA\Settings\Activity\Provider</provider>
7575
<provider>OCA\Settings\Activity\SecurityProvider</provider>
7676
</providers>
77+
7778
</activity>
7879
</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' => 'overview'] , 'root' => ''],
36+
['name' => 'AdminSettings#index', 'url' => '/settings/admin/{section}', 'verb' => 'GET', 'defaults' => ['section' => 'server'] , '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: 0 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,8 @@
9090
use OCP\Group\Events\GroupDeletedEvent;
9191
use OCP\Group\Events\UserAddedEvent;
9292
use OCP\Group\Events\UserRemovedEvent;
93-
use OCP\Group\ISubAdmin;
9493
use OCP\IConfig;
95-
use OCP\IGroupManager;
96-
use OCP\INavigationManager;
9794
use OCP\IURLGenerator;
98-
use OCP\IUserSession;
9995
use OCP\L10N\IFactory;
10096
use OCP\Mail\IMailer;
10197
use OCP\Security\ICrypto;
@@ -230,93 +226,5 @@ public function register(IRegistrationContext $context): void {
230226

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

‎core/AppInfo/Application.php‎

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,6 @@
3232
use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
3333
use OCP\DB\Events\AddMissingIndicesEvent;
3434
use OCP\DB\Events\AddMissingPrimaryKeyEvent;
35-
use OCP\INavigationManager;
36-
use OCP\IURLGenerator;
37-
use OCP\IUserSession;
38-
use OCP\L10N\IFactory;
39-
use OCP\Server;
4035
use OCP\User\Events\BeforeUserDeletedEvent;
4136
use OCP\User\Events\PasswordUpdatedEvent;
4237
use OCP\User\Events\UserDeletedEvent;
@@ -98,36 +93,7 @@ public function register(IRegistrationContext $context): void {
9893

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

13399
}

‎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/server')
24+
cy.visit('/settings/admin')
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/server')
51+
cy.visit('/settings/admin')
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/server')
95+
cy.visit('/settings/admin')
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/, level: 2 })
157+
cy.findByRole('heading', { name: /^Theming/ })
158158
.should('exist')
159159
.scrollIntoView()
160160
})

0 commit comments

Comments
 (0)