Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use NavigationManager instead of AppManager to handle custom apps order #47523

Merged
merged 7 commits into from
Sep 9, 2024
4 changes: 3 additions & 1 deletion apps/files/lib/App.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use OCP\IUserSession;
use OCP\L10N\IFactory;
use OCP\Server;
use Psr\Log\LoggerInterface;

class App {
private static ?INavigationManager $navigationManager = null;
Expand All @@ -32,7 +33,8 @@ public static function getNavigationManager(): INavigationManager {
Server::get(IFactory::class),
Server::get(IUserSession::class),
Server::get(IGroupManager::class),
Server::get(IConfig::class)
Server::get(IConfig::class),
Server::get(LoggerInterface::class),
);
self::$navigationManager->clear(false);
}
Expand Down
10 changes: 7 additions & 3 deletions apps/theming/lib/Controller/ThemingController.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use OCP\Files\NotPermittedException;
use OCP\IConfig;
use OCP\IL10N;
use OCP\INavigationManager;
use OCP\IRequest;
use OCP\IURLGenerator;
use ScssPhp\ScssPhp\Compiler;
Expand All @@ -47,6 +48,7 @@ class ThemingController extends Controller {
private IAppManager $appManager;
private ImageManager $imageManager;
private ThemesService $themesService;
private INavigationManager $navigationManager;

public function __construct(
$appName,
Expand All @@ -57,7 +59,8 @@ public function __construct(
IURLGenerator $urlGenerator,
IAppManager $appManager,
ImageManager $imageManager,
ThemesService $themesService
ThemesService $themesService,
INavigationManager $navigationManager,
) {
parent::__construct($appName, $request);

Expand All @@ -68,6 +71,7 @@ public function __construct(
$this->appManager = $appManager;
$this->imageManager = $imageManager;
$this->themesService = $themesService;
$this->navigationManager = $navigationManager;
}

/**
Expand Down Expand Up @@ -163,7 +167,7 @@ public function updateAppMenu($setting, $value) {
case 'defaultApps':
if (is_array($value)) {
try {
$this->appManager->setDefaultApps($value);
$this->navigationManager->setDefaultEntryIds($value);
} catch (InvalidArgumentException $e) {
$error = $this->l10n->t('Invalid app given');
}
Expand Down Expand Up @@ -310,7 +314,7 @@ public function undo(string $setting): DataResponse {
#[AuthorizedAdminSetting(settings: Admin::class)]
public function undoAll(): DataResponse {
$this->themingDefaults->undoAll();
$this->appManager->setDefaultApps([]);
$this->navigationManager->setDefaultEntryIds([]);

return new DataResponse(
[
Expand Down
4 changes: 3 additions & 1 deletion apps/theming/lib/Settings/Admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use OCP\AppFramework\Services\IInitialState;
use OCP\IConfig;
use OCP\IL10N;
use OCP\INavigationManager;
use OCP\IURLGenerator;
use OCP\Settings\IDelegatedSettings;
use OCP\Util;
Expand All @@ -28,6 +29,7 @@ public function __construct(
private IInitialState $initialState,
private IURLGenerator $urlGenerator,
private ImageManager $imageManager,
private INavigationManager $navigationManager,
) {
}

Expand Down Expand Up @@ -70,7 +72,7 @@ public function getForm(): TemplateResponse {
'docUrlIcons' => $this->urlGenerator->linkToDocs('admin-theming-icons'),
'canThemeIcons' => $this->imageManager->shouldReplaceIcons(),
'userThemingDisabled' => $this->themingDefaults->isUserThemingDisabled(),
'defaultApps' => array_filter(explode(',', $this->config->getSystemValueString('defaultapp', ''))),
'defaultApps' => $this->navigationManager->getDefaultEntryIds(),
]);

Util::addScript($this->appName, 'admin-theming');
Expand Down
10 changes: 5 additions & 5 deletions apps/theming/lib/Settings/Personal.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
use OCA\Theming\Service\BackgroundService;
use OCA\Theming\Service\ThemesService;
use OCA\Theming\ThemingDefaults;
use OCP\App\IAppManager;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Services\IInitialState;
use OCP\IConfig;
use OCP\INavigationManager;
use OCP\Settings\ISettings;
use OCP\Util;

Expand All @@ -25,7 +25,7 @@ public function __construct(
private ThemesService $themesService,
private IInitialState $initialStateService,
private ThemingDefaults $themingDefaults,
private IAppManager $appManager,
private INavigationManager $navigationManager,
) {
}

Expand All @@ -49,8 +49,8 @@ public function getForm(): TemplateResponse {
});
}

// Get the default app enforced by admin
$forcedDefaultApp = $this->appManager->getDefaultAppForUser(null, false);
// Get the default entry enforced by admin
$forcedDefaultEntry = $this->navigationManager->getDefaultEntryIdForUser(null, false);

/** List of all shipped backgrounds */
$this->initialStateService->provideInitialState('shippedBackgrounds', BackgroundService::SHIPPED_BACKGROUNDS);
Expand Down Expand Up @@ -78,7 +78,7 @@ public function getForm(): TemplateResponse {
$this->initialStateService->provideInitialState('enableBlurFilter', $this->config->getUserValue($this->userId, 'theming', 'force_enable_blur_filter', ''));
$this->initialStateService->provideInitialState('navigationBar', [
'userAppOrder' => json_decode($this->config->getUserValue($this->userId, 'core', 'apporder', '[]'), true, flags:JSON_THROW_ON_ERROR),
'enforcedDefaultApp' => $forcedDefaultApp
'enforcedDefaultApp' => $forcedDefaultEntry
]);

Util::addScript($this->appName, 'personal-theming');
Expand Down
2 changes: 1 addition & 1 deletion apps/theming/src/components/UserAppMenuSection.vue
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export default defineComponent({
*/
const initialAppOrder = loadState<INavigationEntry[]>('core', 'apps')
.filter(({ type }) => type === 'link')
.map((app) => ({ ...app, label: app.name, default: app.default && app.app === enforcedDefaultApp }))
.map((app) => ({ ...app, label: app.name, default: app.default && app.id === enforcedDefaultApp }))

/**
* Check if a custom app order is used or the default is shown
Expand Down
5 changes: 5 additions & 0 deletions apps/theming/tests/Controller/ThemingControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use OCP\Files\SimpleFS\ISimpleFile;
use OCP\IConfig;
use OCP\IL10N;
use OCP\INavigationManager;
use OCP\IRequest;
use OCP\IURLGenerator;
use PHPUnit\Framework\MockObject\MockObject;
Expand All @@ -42,6 +43,8 @@ class ThemingControllerTest extends TestCase {
private $urlGenerator;
/** @var ThemesService|MockObject */
private $themesService;
/** @var INavigationManager|MockObject */
private $navigationManager;

protected function setUp(): void {
$this->request = $this->createMock(IRequest::class);
Expand All @@ -52,6 +55,7 @@ protected function setUp(): void {
$this->urlGenerator = $this->createMock(IURLGenerator::class);
$this->imageManager = $this->createMock(ImageManager::class);
$this->themesService = $this->createMock(ThemesService::class);
$this->navigationManager = $this->createMock(INavigationManager::class);

$timeFactory = $this->createMock(ITimeFactory::class);
$timeFactory->expects($this->any())
Expand All @@ -70,6 +74,7 @@ protected function setUp(): void {
$this->appManager,
$this->imageManager,
$this->themesService,
$this->navigationManager,
);

parent::setUp();
Expand Down
6 changes: 5 additions & 1 deletion apps/theming/tests/Settings/AdminTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use OCP\AppFramework\Services\IInitialState;
use OCP\IConfig;
use OCP\IL10N;
use OCP\INavigationManager;
use OCP\IURLGenerator;
use Test\TestCase;

Expand All @@ -24,6 +25,7 @@ class AdminTest extends TestCase {
private IURLGenerator $urlGenerator;
private ImageManager $imageManager;
private IL10N $l10n;
private INavigationManager $navigationManager;

protected function setUp(): void {
parent::setUp();
Expand All @@ -33,6 +35,7 @@ protected function setUp(): void {
$this->initialState = $this->createMock(IInitialState::class);
$this->urlGenerator = $this->createMock(IURLGenerator::class);
$this->imageManager = $this->createMock(ImageManager::class);
$this->navigationManager = $this->createMock(INavigationManager::class);

$this->admin = new Admin(
Application::APP_ID,
Expand All @@ -41,7 +44,8 @@ protected function setUp(): void {
$this->themingDefaults,
$this->initialState,
$this->urlGenerator,
$this->imageManager
$this->imageManager,
$this->navigationManager,
);
}

Expand Down
15 changes: 8 additions & 7 deletions apps/theming/tests/Settings/PersonalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use OCP\AppFramework\Services\IInitialState;
use OCP\IConfig;
use OCP\IL10N;
use OCP\INavigationManager;
use OCP\IURLGenerator;
use OCP\IUserSession;
use PHPUnit\Framework\MockObject\MockObject;
Expand All @@ -34,7 +35,7 @@ class PersonalTest extends TestCase {
private ThemesService&MockObject $themesService;
private IInitialState&MockObject $initialStateService;
private ThemingDefaults&MockObject $themingDefaults;
private IAppManager&MockObject $appManager;
private INavigationManager&MockObject $navigationManager;
private Personal $admin;

/** @var ITheme[] */
Expand All @@ -46,7 +47,7 @@ protected function setUp(): void {
$this->themesService = $this->createMock(ThemesService::class);
$this->initialStateService = $this->createMock(IInitialState::class);
$this->themingDefaults = $this->createMock(ThemingDefaults::class);
$this->appManager = $this->createMock(IAppManager::class);
$this->navigationManager = $this->createMock(INavigationManager::class);

$this->initThemes();

Expand All @@ -62,7 +63,7 @@ protected function setUp(): void {
$this->themesService,
$this->initialStateService,
$this->themingDefaults,
$this->appManager,
$this->navigationManager,
);
}

Expand Down Expand Up @@ -103,9 +104,9 @@ public function testGetForm(string $enforcedTheme, $themesState) {
['admin', 'theming', 'background_image', BackgroundService::BACKGROUND_DEFAULT],
]);

$this->appManager->expects($this->once())
->method('getDefaultAppForUser')
->willReturn('forcedapp');
$this->navigationManager->expects($this->once())
->method('getDefaultEntryIdForUser')
->willReturn('forced_id');

$this->initialStateService->expects($this->exactly(8))
->method('provideInitialState')
Expand All @@ -117,7 +118,7 @@ public function testGetForm(string $enforcedTheme, $themesState) {
['themes', $themesState],
['enforceTheme', $enforcedTheme],
['isUserThemingDisabled', false],
['navigationBar', ['userAppOrder' => [], 'enforcedDefaultApp' => 'forcedapp']],
['navigationBar', ['userAppOrder' => [], 'enforcedDefaultApp' => 'forced_id']],
]);

$expected = new TemplateResponse('theming', 'settings-personal');
Expand Down
20 changes: 10 additions & 10 deletions config/config.sample.php
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@

/**
* Enable SMTP class debugging.
* NOTE: ``loglevel`` will likely need to be adjusted too. See docs:
* NOTE: ``loglevel`` will likely need to be adjusted too. See docs:
* https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/email_configuration.html#enabling-debug-mode
*
* Defaults to ``false``
Expand Down Expand Up @@ -1167,9 +1167,9 @@
*/

/**
* Set the default app to open on login. Use the app names as they appear in the
* URL after clicking them in the Apps menu, such as documents, calendar, and
* gallery. You can use a comma-separated list of app names, so if the first
* Set the default app to open on login. The entry IDs can be retrieved from
* the Navigations OCS API endpoint: https://docs.nextcloud.com/server/latest/develper_manual/_static/openapi.html#/operations/core-navigation-get-apps-navigation.
* You can use a comma-separated list of app names, so if the first
* app is not enabled for a user then Nextcloud will try the second one, and so
* on. If no enabled apps are found it defaults to the dashboard app.
*
Expand Down Expand Up @@ -1307,18 +1307,18 @@
/**
* custom path for ffmpeg binary
*
* Defaults to ``null`` and falls back to searching ``avconv`` and ``ffmpeg``
* Defaults to ``null`` and falls back to searching ``avconv`` and ``ffmpeg``
* in the configured ``PATH`` environment
*/
'preview_ffmpeg_path' => '/usr/bin/ffmpeg',

/**
* Set the URL of the Imaginary service to send image previews to.
* Also requires the ``OC\Preview\Imaginary`` provider to be enabled in the
* ``enabledPreviewProviders`` array, to create previews for these mimetypes: bmp,
* Also requires the ``OC\Preview\Imaginary`` provider to be enabled in the
* ``enabledPreviewProviders`` array, to create previews for these mimetypes: bmp,
* x-bitmap, png, jpeg, gif, heic, heif, svg+xml, tiff, webp and illustrator.
*
* If you want Imaginary to also create preview images from PDF Documents, you
* If you want Imaginary to also create preview images from PDF Documents, you
* have to add the ``OC\Preview\ImaginaryPDF`` provider as well.
*
* See https://github.com/h2non/imaginary
Expand Down Expand Up @@ -2050,9 +2050,9 @@
/**
* Deny extensions from being used for filenames.
* Matching existing files can no longer be updated and in matching folders no files can be created anymore.
*
*
* The '.part' extension is always forbidden, as this is used internally by Nextcloud.
*
*
* Defaults to ``array('.filepart', '.part')``
*/
'forbidden_filename_extensions' => ['.part', '.filepart'],
Expand Down
4 changes: 2 additions & 2 deletions dist/theming-personal-theming.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/theming-personal-theming.js.map

Large diffs are not rendered by default.

Loading
Loading