Skip to content

Commit e889c72

Browse files
authored
Merge pull request #61406 from nextcloud/fix/settings-nav-active-entry-mismatch
fix(settings): show app menu current-app button on settings pages
2 parents ec557c1 + 869c294 commit e889c72

3 files changed

Lines changed: 140 additions & 2 deletions

File tree

apps/settings/lib/Controller/CommonSettingsTrait.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,14 +152,14 @@ private function getIndexResponse(string $type, string $section): TemplateRespon
152152
if ($section === 'theming') {
153153
$this->navigationManager->setActiveEntry('accessibility_settings');
154154
} else {
155-
$this->navigationManager->setActiveEntry('settings');
155+
$this->navigationManager->setActiveEntry('settings_personal');
156156
}
157157
} elseif ($type === 'admin') {
158158
$settings = array_values($this->settingsManager->getAllowedAdminSettings($section, $user));
159159
if (empty($settings) && empty($declarativeSettings)) {
160160
throw new NotAdminException('Logged in user does not have permission to access these settings.');
161161
}
162-
$this->navigationManager->setActiveEntry('admin_settings');
162+
$this->navigationManager->setActiveEntry('settings_administration');
163163
} else {
164164
throw new InvalidArgumentException('$type must be either "admin" or "personal"');
165165
}

apps/settings/tests/Controller/AdminSettingsControllerTest.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,13 @@ public function testIndex(): void {
123123
->with($user, 'admin', 'test')
124124
->willReturn([]);
125125

126+
// The active entry must match the id the admin nav entry is registered
127+
// under in Application.php, otherwise the header current-app button is hidden.
128+
$this->navigationManager
129+
->expects($this->once())
130+
->method('setActiveEntry')
131+
->with('settings_administration');
132+
126133
$initialState = [];
127134
$this->initialState->expects(self::atLeastOnce())
128135
->method('provideInitialState')
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
8+
namespace OCA\Settings\Tests\Controller;
9+
10+
use OCA\Settings\Controller\PersonalSettingsController;
11+
use OCA\Settings\Settings\Personal\ServerDevNotice;
12+
use OCP\AppFramework\Http\TemplateResponse;
13+
use OCP\AppFramework\Services\IInitialState;
14+
use OCP\Group\ISubAdmin;
15+
use OCP\IGroupManager;
16+
use OCP\INavigationManager;
17+
use OCP\IRequest;
18+
use OCP\IUser;
19+
use OCP\IUserManager;
20+
use OCP\IUserSession;
21+
use OCP\Server;
22+
use OCP\Settings\IDeclarativeManager;
23+
use OCP\Settings\IManager;
24+
use PHPUnit\Framework\MockObject\MockObject;
25+
use Test\TestCase;
26+
27+
/**
28+
* @package Tests\Settings\Controller
29+
*/
30+
#[\PHPUnit\Framework\Attributes\Group(name: 'DB')]
31+
class PersonalSettingsControllerTest extends TestCase {
32+
33+
private IRequest&MockObject $request;
34+
private INavigationManager&MockObject $navigationManager;
35+
private IManager&MockObject $settingsManager;
36+
private IUserSession&MockObject $userSession;
37+
private IGroupManager&MockObject $groupManager;
38+
private ISubAdmin&MockObject $subAdmin;
39+
private IDeclarativeManager&MockObject $declarativeSettingsManager;
40+
private IInitialState&MockObject $initialState;
41+
42+
private string $uid = 'personalsettingsuser';
43+
private PersonalSettingsController $personalSettingsController;
44+
45+
protected function setUp(): void {
46+
parent::setUp();
47+
48+
$this->request = $this->createMock(IRequest::class);
49+
$this->navigationManager = $this->createMock(INavigationManager::class);
50+
$this->settingsManager = $this->createMock(IManager::class);
51+
$this->userSession = $this->createMock(IUserSession::class);
52+
$this->groupManager = $this->createMock(IGroupManager::class);
53+
$this->subAdmin = $this->createMock(ISubAdmin::class);
54+
$this->declarativeSettingsManager = $this->createMock(IDeclarativeManager::class);
55+
$this->initialState = $this->createMock(IInitialState::class);
56+
57+
$this->personalSettingsController = new PersonalSettingsController(
58+
'settings',
59+
$this->request,
60+
$this->navigationManager,
61+
$this->settingsManager,
62+
$this->userSession,
63+
$this->groupManager,
64+
$this->subAdmin,
65+
$this->declarativeSettingsManager,
66+
$this->initialState,
67+
);
68+
69+
$user = Server::get(IUserManager::class)->createUser($this->uid, 'mylongrandompassword');
70+
\OC_User::setUserId($user->getUID());
71+
}
72+
73+
protected function tearDown(): void {
74+
Server::get(IUserManager::class)
75+
->get($this->uid)
76+
->delete();
77+
\OC_User::setUserId(null);
78+
Server::get(IUserSession::class)->setUser(null);
79+
80+
parent::tearDown();
81+
}
82+
83+
/**
84+
* Marks the section we are about to render so the rest of getIndexResponse()
85+
* has something to format. The actual content is irrelevant to these tests.
86+
*/
87+
private function stubSettingsFor(string $section): void {
88+
$user = $this->createMock(IUser::class);
89+
$user->method('getUID')->willReturn('user123');
90+
$this->userSession->method('getUser')->willReturn($user);
91+
92+
$form = new TemplateResponse('settings', 'settings/empty');
93+
$setting = $this->createMock(ServerDevNotice::class);
94+
$setting->method('getForm')->willReturn($form);
95+
96+
$this->settingsManager
97+
->method('getPersonalSettings')
98+
->with($section)
99+
->willReturn([5 => [$setting]]);
100+
$this->settingsManager->method('getPersonalSections')->willReturn([]);
101+
$this->settingsManager->method('getAdminSections')->willReturn([]);
102+
$this->declarativeSettingsManager->method('getFormIDs')->willReturn([]);
103+
$this->declarativeSettingsManager->method('getFormsWithValues')->willReturn([]);
104+
}
105+
106+
public function testIndexActivatesPersonalNavEntry(): void {
107+
$this->stubSettingsFor('additional');
108+
109+
// Must match the id the personal nav entry is registered under in
110+
// Application.php ('settings_personal'), otherwise the header
111+
// current-app button is hidden on personal settings pages.
112+
$this->navigationManager
113+
->expects($this->once())
114+
->method('setActiveEntry')
115+
->with('settings_personal');
116+
117+
$this->personalSettingsController->index('additional');
118+
}
119+
120+
public function testThemingSectionActivatesAccessibilityNavEntry(): void {
121+
$this->stubSettingsFor('theming');
122+
123+
// The appearance/accessibility section keeps its own nav entry.
124+
$this->navigationManager
125+
->expects($this->once())
126+
->method('setActiveEntry')
127+
->with('accessibility_settings');
128+
129+
$this->personalSettingsController->index('theming');
130+
}
131+
}

0 commit comments

Comments
 (0)