Skip to content

Commit 33f1803

Browse files
authored
Merge pull request #63860 from nextcloud/fix/60495/appstore-link-visibility
fix(core): make the app store link in the app menu configurable
2 parents 31b2c78 + 74dbbbc commit 33f1803

9 files changed

Lines changed: 283 additions & 47 deletions

File tree

core/AppInfo/ConfigLexicon.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ class ConfigLexicon implements ILexicon {
4040

4141
public const ON_DEMAND_PREVIEW_MIGRATION = 'on_demand_preview_migration';
4242

43+
public const APPSTORE_LINK_SHOWN = 'appstore_link_shown';
44+
4345
#[\Override]
4446
public function getStrictness(): Strictness {
4547
return Strictness::IGNORE;
@@ -102,6 +104,16 @@ public function getAppConfigs(): array {
102104
defaultRaw: true,
103105
definition: 'Whether on demand preview migration is enabled.'
104106
),
107+
new Entry(
108+
key: self::APPSTORE_LINK_SHOWN,
109+
type: ValueType::BOOL,
110+
defaultRaw: fn (Preset $p): bool => match ($p) {
111+
Preset::NONE, Preset::PRIVATE, Preset::FAMILY, Preset::CLUB => true,
112+
default => false,
113+
},
114+
definition: 'Show the app store link in the app menu to accounts without admin rights',
115+
note: 'When this key is not set, the link is also hidden while a valid subscription is available or while "appstoreenabled" is disabled. Setting this key explicitly takes precedence over both.',
116+
),
105117
new Entry(
106118
key: self::DAV_REPAIR_REMOVED_BROKEN_PROPERTIES,
107119
type: ValueType::BOOL,

core/src/components/AppMenu.vue

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ export default defineComponent({
152152
appList,
153153
navigationActions,
154154
settingsList,
155+
// Fail closed: a missing state must not leak the link.
156+
appStoreLinkShown: loadState<boolean>('core', 'appStoreLinkShown', false),
155157
isAdmin: getCurrentUser()?.isAdmin ?? false,
156158
// Roving tabindex: only this tile has tabindex=0; arrow keys move it.
157159
focusedIndex: 0,
@@ -252,12 +254,16 @@ export default defineComponent({
252254
253255
// Stable-ordered list that focusedIndex indexes into. The trailing
254256
// utility tile is "More apps" (local app management) for admins and
255-
// "App store" (apps.nextcloud.com) for everyone else.
257+
// "App store" (apps.nextcloud.com) for everyone else when
258+
// appstore_link_shown allows it.
256259
gridItems(): INavigationEntry[] {
257-
const tail = this.isAdmin
258-
? { ...this.moreAppsEntry, active: this.currentApp?.id === APP_MANAGEMENT_ID }
259-
: this.appStoreEntry
260-
return [...this.appList, tail]
260+
const tail: INavigationEntry[] = []
261+
if (this.isAdmin) {
262+
tail.push({ ...this.moreAppsEntry, active: this.currentApp?.id === APP_MANAGEMENT_ID })
263+
} else if (this.appStoreLinkShown) {
264+
tail.push(this.appStoreEntry)
265+
}
266+
return [...this.appList, ...tail]
261267
},
262268
},
263269

core/src/tests/components/AppMenu.spec.ts

Lines changed: 45 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,10 @@ function fakeApps(): INavigationEntry[] {
7070
// ships getAll('settings') keyed by entry id.
7171
function mockActiveSettingsEntry(overrides: Partial<INavigationEntry>): void {
7272
const entry = makeApp({ type: 'settings', active: true, ...overrides })
73-
initialState.loadState.mockImplementation((_a: string, key: string, fallback: unknown) => {
74-
if (key === 'apps') {
75-
return [makeApp({ id: 'files', name: 'Files', active: false })]
76-
}
77-
if (key === 'settingsNavEntries') {
78-
return { [entry.id]: entry }
79-
}
80-
return fallback
81-
})
73+
initialState.loadState.mockImplementation(stateFor({
74+
apps: [makeApp({ id: 'files', name: 'Files', active: false })],
75+
settingsNavEntries: { [entry.id]: entry },
76+
}))
8277
}
8378

8479
// Navigation actions (INavigationManager::TYPE_ACTION). Without an `href` the
@@ -98,17 +93,16 @@ function fakeActions(count: number): INavigationEntry[] {
9893
return ids.slice(0, count).map((id) => makeAction(id))
9994
}
10095

96+
// AppMenu hides the app store tile when the state is absent, so a default
97+
// instance has to supply it.
98+
function stateFor(states: Record<string, unknown>) {
99+
const all: Record<string, unknown> = { appStoreLinkShown: true, ...states }
100+
return (_app: string, key: string, fallback: unknown) => key in all ? all[key] : fallback
101+
}
102+
101103
// loadState implementation serving both apps and navigation actions.
102104
function stateWith(apps: INavigationEntry[], actions: INavigationEntry[]) {
103-
return (_app: string, key: string, fallback: unknown) => {
104-
if (key === 'apps') {
105-
return apps
106-
}
107-
if (key === 'navigationActions') {
108-
return actions
109-
}
110-
return fallback
111-
}
105+
return stateFor({ apps, navigationActions: actions })
112106
}
113107

114108
function eightApps(activeIndex: number = -1): INavigationEntry[] {
@@ -132,7 +126,7 @@ beforeEach(async () => {
132126
for (const k of Object.keys(eventBus.__handlers)) {
133127
delete eventBus.__handlers[k]
134128
}
135-
initialState.loadState.mockImplementation((_app: string, key: string, fallback: unknown) => key === 'apps' ? fakeApps() : fallback)
129+
initialState.loadState.mockImplementation(stateFor({ apps: fakeApps() }))
136130
auth.getCurrentUser.mockReturnValue({ isAdmin: false })
137131
AppMenu = (await import('../../components/AppMenu.vue')).default
138132
})
@@ -144,6 +138,11 @@ afterEach(() => {
144138
}
145139
})
146140

141+
function gridLabels(): string[] {
142+
return Array.from(document.querySelectorAll('.app-menu__grid [role="menuitem"]'))
143+
.map((el) => el.querySelector('.app-item__label')?.textContent?.trim() ?? '')
144+
}
145+
147146
// Click the waffle trigger and poll until the teleported menuitems are in the
148147
// DOM. NcPopover teleports to <body> so wrapper.find() can't see them; vi.waitFor
149148
// retries the DOM query rather than relying on flaky nextTick/setTimeout flushes.
@@ -166,10 +165,24 @@ describe('core: AppMenu', () => {
166165
const wrapper = mount(AppMenu, { attachTo: document.body })
167166
await openPopover(wrapper)
168167

169-
const items = document.querySelectorAll('.app-menu__grid [role="menuitem"]')
170-
expect(items).toHaveLength(4)
171-
const labels = Array.from(items).map((el) => el.querySelector('.app-item__label')?.textContent?.trim() ?? '')
172-
expect(labels).toEqual(['Files', 'Mail', 'Calendar', 'App store'])
168+
expect(gridLabels()).toEqual(['Files', 'Mail', 'Calendar', 'App store'])
169+
})
170+
171+
it('omits the "App store" tile when the instance does not offer it', async () => {
172+
initialState.loadState.mockImplementation(stateFor({ apps: fakeApps(), appStoreLinkShown: false }))
173+
const wrapper = mount(AppMenu, { attachTo: document.body })
174+
await openPopover(wrapper)
175+
176+
expect(gridLabels()).toEqual(['Files', 'Mail', 'Calendar'])
177+
})
178+
179+
it('keeps the "More apps" tile for admins when the app store link is hidden', async () => {
180+
initialState.loadState.mockImplementation(stateFor({ apps: fakeApps(), appStoreLinkShown: false }))
181+
auth.getCurrentUser.mockReturnValue({ isAdmin: true })
182+
const wrapper = mount(AppMenu, { attachTo: document.body })
183+
await openPopover(wrapper)
184+
185+
expect(gridLabels()).toEqual(['Files', 'Mail', 'Calendar', 'More apps'])
173186
})
174187

175188
it('renders the "More apps" tile when the current user is an admin', async () => {
@@ -196,7 +209,7 @@ describe('core: AppMenu', () => {
196209
})
197210

198211
it('ArrowRight moves the roving stop from index 0 to index 1 and focuses it', async () => {
199-
initialState.loadState.mockImplementation((_a: string, key: string, fallback: unknown) => key === 'apps' ? eightApps() : fallback)
212+
initialState.loadState.mockImplementation(stateFor({ apps: eightApps() }))
200213
const wrapper = mount(AppMenu, { attachTo: document.body })
201214
await openPopover(wrapper)
202215

@@ -274,15 +287,10 @@ describe('core: AppMenu', () => {
274287
})
275288

276289
it('prefers the active app over a settings entry when both are marked active', () => {
277-
initialState.loadState.mockImplementation((_a: string, key: string, fallback: unknown) => {
278-
if (key === 'apps') {
279-
return [makeApp({ id: 'files', name: 'Files', active: true })]
280-
}
281-
if (key === 'settingsNavEntries') {
282-
return { settings_administration: makeApp({ id: 'settings_administration', name: 'Administration settings', type: 'settings', active: true }) }
283-
}
284-
return fallback
285-
})
290+
initialState.loadState.mockImplementation(stateFor({
291+
apps: [makeApp({ id: 'files', name: 'Files', active: true })],
292+
settingsNavEntries: { settings_administration: makeApp({ id: 'settings_administration', name: 'Administration settings', type: 'settings', active: true }) },
293+
}))
286294
const wrapper = mount(AppMenu, { attachTo: document.body })
287295
expect(wrapper.find('.app-menu__current-app-name').text()).toBe('Files')
288296
})
@@ -292,15 +300,10 @@ describe('core: AppMenu', () => {
292300
// "current section" even though it carries type=settings. NavigationManager
293301
// today never marks it active, but a future regression shouldn't leak a
294302
// "Log out" label into the header.
295-
initialState.loadState.mockImplementation((_a: string, key: string, fallback: unknown) => {
296-
if (key === 'apps') {
297-
return [makeApp({ id: 'files', name: 'Files', active: false })]
298-
}
299-
if (key === 'settingsNavEntries') {
300-
return { logout: makeApp({ id: 'logout', name: 'Log out', type: 'settings', href: '/logout', active: true }) }
301-
}
302-
return fallback
303-
})
303+
initialState.loadState.mockImplementation(stateFor({
304+
apps: [makeApp({ id: 'files', name: 'Files', active: false })],
305+
settingsNavEntries: { logout: makeApp({ id: 'logout', name: 'Log out', type: 'settings', href: '/logout', active: true }) },
306+
}))
304307
const wrapper = mount(AppMenu, { attachTo: document.body })
305308
expect(wrapper.find('.app-menu__current-app').exists()).toBe(false)
306309
})

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,6 +1256,7 @@
12561256
'OC\\AppScriptSort' => $baseDir . '/lib/private/AppScriptSort.php',
12571257
'OC\\App\\AppManager' => $baseDir . '/lib/private/App/AppManager.php',
12581258
'OC\\App\\AppStore\\AppNotFoundException' => $baseDir . '/lib/private/App/AppStore/AppNotFoundException.php',
1259+
'OC\\App\\AppStore\\AppStoreLinkVisibility' => $baseDir . '/lib/private/App/AppStore/AppStoreLinkVisibility.php',
12591260
'OC\\App\\AppStore\\Bundles\\Bundle' => $baseDir . '/lib/private/App/AppStore/Bundles/Bundle.php',
12601261
'OC\\App\\AppStore\\Bundles\\BundleFetcher' => $baseDir . '/lib/private/App/AppStore/Bundles/BundleFetcher.php',
12611262
'OC\\App\\AppStore\\Bundles\\EducationBundle' => $baseDir . '/lib/private/App/AppStore/Bundles/EducationBundle.php',

lib/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1297,6 +1297,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
12971297
'OC\\AppScriptSort' => __DIR__ . '/../../..' . '/lib/private/AppScriptSort.php',
12981298
'OC\\App\\AppManager' => __DIR__ . '/../../..' . '/lib/private/App/AppManager.php',
12991299
'OC\\App\\AppStore\\AppNotFoundException' => __DIR__ . '/../../..' . '/lib/private/App/AppStore/AppNotFoundException.php',
1300+
'OC\\App\\AppStore\\AppStoreLinkVisibility' => __DIR__ . '/../../..' . '/lib/private/App/AppStore/AppStoreLinkVisibility.php',
13001301
'OC\\App\\AppStore\\Bundles\\Bundle' => __DIR__ . '/../../..' . '/lib/private/App/AppStore/Bundles/Bundle.php',
13011302
'OC\\App\\AppStore\\Bundles\\BundleFetcher' => __DIR__ . '/../../..' . '/lib/private/App/AppStore/Bundles/BundleFetcher.php',
13021303
'OC\\App\\AppStore\\Bundles\\EducationBundle' => __DIR__ . '/../../..' . '/lib/private/App/AppStore/Bundles/EducationBundle.php',
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OC\App\AppStore;
11+
12+
use OC\Core\AppInfo\ConfigLexicon;
13+
use OCP\IAppConfig;
14+
use OCP\IConfig;
15+
use OCP\Support\Subscription\IRegistry;
16+
17+
/**
18+
* Decides whether the app store link is offered to accounts without admin rights.
19+
*/
20+
class AppStoreLinkVisibility {
21+
public function __construct(
22+
private readonly IConfig $config,
23+
private readonly IAppConfig $appConfig,
24+
private readonly IRegistry $registry,
25+
) {
26+
}
27+
28+
/**
29+
* An explicitly set value wins. Without one, a disabled app store or an
30+
* available subscription hides the link.
31+
*/
32+
public function isShownToUsers(): bool {
33+
if (!$this->appConfig->hasKey('core', ConfigLexicon::APPSTORE_LINK_SHOWN)) {
34+
if (!$this->config->getSystemValueBool('appstoreenabled', true)) {
35+
return false;
36+
}
37+
38+
if ($this->registry->delegateHasValidSubscription()) {
39+
return false;
40+
}
41+
}
42+
43+
return $this->appConfig->getValueBool('core', ConfigLexicon::APPSTORE_LINK_SHOWN);
44+
}
45+
}

lib/private/TemplateLayout.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
namespace OC;
1212

1313
use bantu\IniGetWrapper\IniGetWrapper;
14+
use OC\App\AppStore\AppStoreLinkVisibility;
1415
use OC\AppFramework\Http\Request;
1516
use OC\Authentication\Token\IProvider;
1617
use OC\Core\AppInfo\Application;
@@ -83,6 +84,7 @@ public function getPageTemplate(string $renderAs, string $appId): ITemplate {
8384
$this->initialState->provideInitialState('core', 'active-app', $this->navigationManager->getActiveEntry());
8485
$this->initialState->provideInitialState('core', 'apps', array_values($this->navigationManager->getAll()));
8586
$this->initialState->provideInitialState('core', 'navigationActions', array_values($this->navigationManager->getAll(INavigationManager::TYPE_ACTION)));
87+
$this->initialState->provideInitialState('core', 'appStoreLinkShown', Server::get(AppStoreLinkVisibility::class)->isShownToUsers());
8688

8789
$this->initialState->provideInitialState('unified-search', 'min-search-length', $this->appConfig->getValueInt(Application::APP_ID, ConfigLexicon::UNIFIED_SEARCH_MIN_SEARCH_LENGTH));
8890
if ($this->config->getSystemValueBool('unified_search.enabled', false) || !$this->config->getSystemValueBool('enable_non-accessible_features', true)) {

0 commit comments

Comments
 (0)