Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions core/AppInfo/ConfigLexicon.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ class ConfigLexicon implements ILexicon {

public const ON_DEMAND_PREVIEW_MIGRATION = 'on_demand_preview_migration';

public const APPSTORE_LINK_SHOWN = 'appstore_link_shown';

#[\Override]
public function getStrictness(): Strictness {
return Strictness::IGNORE;
Expand Down Expand Up @@ -101,6 +103,16 @@ public function getAppConfigs(): array {
defaultRaw: true,
definition: 'Whether on demand preview migration is enabled.'
),
new Entry(
key: self::APPSTORE_LINK_SHOWN,
type: ValueType::BOOL,
defaultRaw: fn (Preset $p): bool => match ($p) {
Preset::NONE, Preset::PRIVATE, Preset::FAMILY, Preset::CLUB => true,
default => false,
},
definition: 'Show the app store link in the app menu to accounts without admin rights',
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.',
),
];
}

Expand Down
14 changes: 11 additions & 3 deletions core/src/components/AppMenu.vue
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ export default defineComponent({
return {
appList,
settingsList,
// Fail closed: a missing state must not leak the link.
appStoreLinkShown: loadState<boolean>('core', 'appStoreLinkShown', false),
isAdmin: getCurrentUser()?.isAdmin ?? false,
// Roving tabindex: only this tile has tabindex=0; arrow keys move it.
focusedIndex: 0,
Expand Down Expand Up @@ -191,10 +193,16 @@ export default defineComponent({

// Stable-ordered list that focusedIndex indexes into. The trailing
// utility tile is "More apps" (local app management) for admins and
// "App store" (apps.nextcloud.com) for everyone else.
// "App store" (apps.nextcloud.com) for everyone else when
// appstore_link_shown allows it.
gridItems(): INavigationEntry[] {
const tail = this.isAdmin ? this.moreAppsEntry : this.appStoreEntry
return [...this.appList, tail]
const tail: INavigationEntry[] = []
if (this.isAdmin) {
tail.push(this.moreAppsEntry)
} else if (this.appStoreLinkShown) {
tail.push(this.appStoreEntry)
}
return [...this.appList, ...tail]
},
},

Expand Down
46 changes: 35 additions & 11 deletions core/src/tests/components/AppMenu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ function eightApps(activeIndex: number = -1): INavigationEntry[] {
}))
}

// AppMenu hides the app store tile when the state is absent, so a default
// instance has to supply it.
function stateFor(states: Record<string, unknown>) {
const all: Record<string, unknown> = { appStoreLinkShown: true, ...states }
return (_app: string, key: string, fallback: unknown) => key in all ? all[key] : fallback
}

// Import AFTER mocks are registered. Static `import` would hoist above
// vi.mock() and break the wiring; dynamic import in beforeAll/await is the
// idiomatic Vitest workaround when you need to control mock state per test.
Expand All @@ -86,7 +93,7 @@ beforeEach(async () => {
for (const k of Object.keys(eventBus.__handlers)) {
delete eventBus.__handlers[k]
}
initialState.loadState.mockImplementation((_app: string, key: string, fallback: unknown) => key === 'apps' ? fakeApps() : fallback)
initialState.loadState.mockImplementation(stateFor({ apps: fakeApps() }))
auth.getCurrentUser.mockReturnValue({ isAdmin: false })
AppMenu = (await import('../../components/AppMenu.vue')).default
})
Expand All @@ -98,6 +105,11 @@ afterEach(() => {
}
})

function gridLabels(): string[] {
return Array.from(document.querySelectorAll('.app-menu__grid [role="menuitem"]'))
.map((el) => el.querySelector('.app-item__label')?.textContent?.trim() ?? '')
}

// Click the waffle trigger and poll until the teleported menuitems are in the
// DOM. NcPopover teleports to <body> so wrapper.find() can't see them; vi.waitFor
// retries the DOM query rather than relying on flaky nextTick/setTimeout flushes.
Expand Down Expand Up @@ -137,8 +149,25 @@ describe('core: AppMenu', () => {
expect(moreApps).toBeTruthy()
})

it('omits the "App store" tile when the instance does not offer it', async () => {
initialState.loadState.mockImplementation(stateFor({ apps: fakeApps(), appStoreLinkShown: false }))
const wrapper = mount(AppMenu, { attachTo: document.body })
await openPopover(wrapper)

expect(gridLabels()).toEqual(['Files', 'Mail', 'Calendar'])
})

it('keeps the "More apps" tile for admins when the app store link is hidden', async () => {
initialState.loadState.mockImplementation(stateFor({ apps: fakeApps(), appStoreLinkShown: false }))
auth.getCurrentUser.mockReturnValue({ isAdmin: true })
const wrapper = mount(AppMenu, { attachTo: document.body })
await openPopover(wrapper)

expect(gridLabels()).toEqual(['Files', 'Mail', 'Calendar', 'More apps'])
})

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

Expand Down Expand Up @@ -221,15 +250,10 @@ describe('core: AppMenu', () => {
// "current section" even though it carries type=settings. NavigationManager
// today never marks it active, but a future regression shouldn't leak a
// "Log out" label into the header.
initialState.loadState.mockImplementation((_a: string, key: string, fallback: unknown) => {
if (key === 'apps') {
return [makeApp({ id: 'files', name: 'Files', active: false })]
}
if (key === 'settingsNavEntries') {
return { logout: makeApp({ id: 'logout', name: 'Log out', type: 'settings', href: '/logout', active: true }) }
}
return fallback
})
initialState.loadState.mockImplementation(stateFor({
apps: [makeApp({ id: 'files', name: 'Files', active: false })],
settingsNavEntries: { logout: makeApp({ id: 'logout', name: 'Log out', type: 'settings', href: '/logout', active: true }) },
}))
const wrapper = mount(AppMenu, { attachTo: document.body })
expect(wrapper.find('.app-menu__current-app').exists()).toBe(false)
})
Expand Down
4 changes: 2 additions & 2 deletions dist/core-main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/core-main.js.map

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1182,6 +1182,7 @@
'OC\\AppScriptSort' => $baseDir . '/lib/private/AppScriptSort.php',
'OC\\App\\AppManager' => $baseDir . '/lib/private/App/AppManager.php',
'OC\\App\\AppStore\\AppNotFoundException' => $baseDir . '/lib/private/App/AppStore/AppNotFoundException.php',
'OC\\App\\AppStore\\AppStoreLinkVisibility' => $baseDir . '/lib/private/App/AppStore/AppStoreLinkVisibility.php',
'OC\\App\\AppStore\\Bundles\\Bundle' => $baseDir . '/lib/private/App/AppStore/Bundles/Bundle.php',
'OC\\App\\AppStore\\Bundles\\BundleFetcher' => $baseDir . '/lib/private/App/AppStore/Bundles/BundleFetcher.php',
'OC\\App\\AppStore\\Bundles\\EducationBundle' => $baseDir . '/lib/private/App/AppStore/Bundles/EducationBundle.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1223,6 +1223,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\AppScriptSort' => __DIR__ . '/../../..' . '/lib/private/AppScriptSort.php',
'OC\\App\\AppManager' => __DIR__ . '/../../..' . '/lib/private/App/AppManager.php',
'OC\\App\\AppStore\\AppNotFoundException' => __DIR__ . '/../../..' . '/lib/private/App/AppStore/AppNotFoundException.php',
'OC\\App\\AppStore\\AppStoreLinkVisibility' => __DIR__ . '/../../..' . '/lib/private/App/AppStore/AppStoreLinkVisibility.php',
'OC\\App\\AppStore\\Bundles\\Bundle' => __DIR__ . '/../../..' . '/lib/private/App/AppStore/Bundles/Bundle.php',
'OC\\App\\AppStore\\Bundles\\BundleFetcher' => __DIR__ . '/../../..' . '/lib/private/App/AppStore/Bundles/BundleFetcher.php',
'OC\\App\\AppStore\\Bundles\\EducationBundle' => __DIR__ . '/../../..' . '/lib/private/App/AppStore/Bundles/EducationBundle.php',
Expand Down
45 changes: 45 additions & 0 deletions lib/private/App/AppStore/AppStoreLinkVisibility.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OC\App\AppStore;

use OC\Core\AppInfo\ConfigLexicon;
use OCP\IAppConfig;
use OCP\IConfig;
use OCP\Support\Subscription\IRegistry;

/**
* Decides whether the app store link is offered to accounts without admin rights.
*/
class AppStoreLinkVisibility {
public function __construct(
private readonly IConfig $config,
private readonly IAppConfig $appConfig,
private readonly IRegistry $registry,
) {
}

/**
* An explicitly set value wins. Without one, a disabled app store or an
* available subscription hides the link.
*/
public function isShownToUsers(): bool {
if (!$this->appConfig->hasKey('core', ConfigLexicon::APPSTORE_LINK_SHOWN)) {
if (!$this->config->getSystemValueBool('appstoreenabled', true)) {
return false;
}

if ($this->registry->delegateHasValidSubscription()) {
return false;
}
}

return $this->appConfig->getValueBool('core', ConfigLexicon::APPSTORE_LINK_SHOWN);
}
}
2 changes: 2 additions & 0 deletions lib/private/TemplateLayout.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
namespace OC;

use bantu\IniGetWrapper\IniGetWrapper;
use OC\App\AppStore\AppStoreLinkVisibility;
use OC\AppFramework\Http\Request;
use OC\Authentication\Token\IProvider;
use OC\Core\AppInfo\Application;
Expand Down Expand Up @@ -82,6 +83,7 @@ public function getPageTemplate(string $renderAs, string $appId): ITemplate {

$this->initialState->provideInitialState('core', 'active-app', $this->navigationManager->getActiveEntry());
$this->initialState->provideInitialState('core', 'apps', array_values($this->navigationManager->getAll()));
$this->initialState->provideInitialState('core', 'appStoreLinkShown', Server::get(AppStoreLinkVisibility::class)->isShownToUsers());

$this->initialState->provideInitialState('unified-search', 'min-search-length', $this->appConfig->getValueInt(Application::APP_ID, ConfigLexicon::UNIFIED_SEARCH_MIN_SEARCH_LENGTH));
if ($this->config->getSystemValueBool('unified_search.enabled', false) || !$this->config->getSystemValueBool('enable_non-accessible_features', true)) {
Expand Down
123 changes: 123 additions & 0 deletions tests/lib/App/AppStore/AppStoreLinkVisibilityTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace Test\App\AppStore;

use OC\App\AppStore\AppStoreLinkVisibility;
use OC\Core\AppInfo\ConfigLexicon;
use OCP\Config\Lexicon\Entry;
use OCP\Config\Lexicon\Preset;
use OCP\IAppConfig;
use OCP\IConfig;
use OCP\Support\Subscription\IRegistry;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;

class AppStoreLinkVisibilityTest extends TestCase {
private IConfig&MockObject $config;
private IAppConfig&MockObject $appConfig;
private IRegistry&MockObject $registry;
private AppStoreLinkVisibility $visibility;

#[\Override]
protected function setUp(): void {
parent::setUp();

$this->config = $this->createMock(IConfig::class);
$this->appConfig = $this->createMock(IAppConfig::class);
$this->registry = $this->createMock(IRegistry::class);

$this->visibility = new AppStoreLinkVisibility(
$this->config,
$this->appConfig,
$this->registry,
);
}

/**
* @param bool $stored whether an admin set the config key
* @param bool $value the stored value, or the lexicon default when $stored is false
*/
private function arrange(bool $stored, bool $value, bool $appStoreEnabled, bool $subscription): void {
$this->appConfig->method('hasKey')
->with('core', ConfigLexicon::APPSTORE_LINK_SHOWN)
->willReturn($stored);
$this->appConfig->method('getValueBool')
->with('core', ConfigLexicon::APPSTORE_LINK_SHOWN)
->willReturn($value);
$this->config->method('getSystemValueBool')
->with('appstoreenabled', true)
->willReturn($appStoreEnabled);
$this->registry->method('delegateHasValidSubscription')
->willReturn($subscription);
}

public static function dataBool(): array {
return [
'shown' => [true],
'hidden' => [false],
];
}

#[DataProvider('dataBool')]
public function testStoredValueWinsOverSubscriptionAndDisabledAppStore(bool $stored): void {
$this->arrange(stored: true, value: $stored, appStoreEnabled: false, subscription: true);

self::assertSame($stored, $this->visibility->isShownToUsers());
}

public function testHiddenWhenAppStoreIsDisabled(): void {
$this->arrange(stored: false, value: true, appStoreEnabled: false, subscription: false);

self::assertFalse($this->visibility->isShownToUsers());
}

public function testHiddenWhenSubscriptionIsAvailable(): void {
$this->arrange(stored: false, value: true, appStoreEnabled: true, subscription: true);

self::assertFalse($this->visibility->isShownToUsers());
}

#[DataProvider('dataBool')]
public function testUnstoredKeyReturnsTheLexiconDefault(bool $default): void {
$this->arrange(stored: false, value: $default, appStoreEnabled: true, subscription: false);

self::assertSame($default, $this->visibility->isShownToUsers());
}

public static function dataLexiconPreset(): array {
return [
// Existing instances run without a preset and must keep the link.
[Preset::NONE, '1'],
[Preset::FAMILY, '1'],
[Preset::UNIVERSITY, '0'],
];
}

/**
* The lexicon default {@see AppStoreLinkVisibility} falls back to is preset
* dependent. A fresh entry is built per case because {@see Entry::getDefault()}
* memoizes the first default it is asked for.
*/
#[DataProvider('dataLexiconPreset')]
public function testLexiconPresetDefault(Preset $preset, string $expected): void {
self::assertSame($expected, $this->lexiconEntry()->getDefault($preset));
}

private function lexiconEntry(): Entry {
foreach ((new ConfigLexicon())->getAppConfigs() as $entry) {
if ($entry->getKey() === ConfigLexicon::APPSTORE_LINK_SHOWN) {
return $entry;
}
}

self::fail('No lexicon entry for ' . ConfigLexicon::APPSTORE_LINK_SHOWN);
}
}
43 changes: 43 additions & 0 deletions tests/playwright/e2e/core/admin-settings-appstore-link.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

import { runOcc } from '@nextcloud/e2e-test-server'
import { expect } from '@playwright/test'
import { test as userTest } from '../../support/fixtures/random-user-session.ts'
import { NavigationHeaderPage } from '../../support/sections/NavigationHeaderPage.ts'

/**
* Set the app config key that decides whether the app store tile is offered.
*
* @param shown - Whether the tile should be offered
*/
async function setAppStoreLinkShown(shown: boolean): Promise<void> {
await runOcc(['config:app:set', 'core', 'appstore_link_shown', '--value', String(shown), '--type', 'boolean'])
}

// The `admin-settings-` prefix puts this in the serial project: it changes
// instance-wide config. Both states are set explicitly because the test server
// runs with `appstoreenabled` disabled, which hides the tile when the key is unset.
userTest.describe('core: app store link visibility', () => {
userTest.afterAll(async () => {
await runOcc(['config:app:delete', 'core', 'appstore_link_shown'])
})

userTest('offers the "App store" tile only while an admin allows it', async ({ page }) => {
const navigationHeader = new NavigationHeaderPage(page)
const appStoreTile = () => navigationHeader.navigationEntries().filter({ hasText: 'App store' })

await setAppStoreLinkShown(true)
await page.goto('/')
await navigationHeader.openMenu()
await expect(appStoreTile()).toBeVisible()

await setAppStoreLinkShown(false)
await page.goto('/')
await navigationHeader.openMenu()
await expect(navigationHeader.navigationEntries()).not.toHaveCount(0)
await expect(appStoreTile()).toHaveCount(0)
})
})
Loading