Skip to content
Prev Previous commit
Next Next commit
fix(theming): Use NavigationManager to handle default entries
Signed-off-by: provokateurin <[email protected]>
  • Loading branch information
provokateurin committed Sep 9, 2024
commit 0a3093d05da92036684afb5814a4925b443468f7
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
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
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