Skip to content

Commit

Permalink
chore: Use IAppConfig instead of IConfig->getAppValue
Browse files Browse the repository at this point in the history 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
  • Loading branch information
@susnux
susnux committed Sep 1, 2024
1 parent 6c5efcd commit 7df222c
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 94 deletions.
48 changes: 23 additions & 25 deletions apps/theming/lib/Controller/ThemingController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use OCP\AppFramework\Http\FileDisplayResponse;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\NotFoundResponse;
use OCP\AppFramework\Services\IAppConfig;
use OCP\Files\NotFoundException;
use OCP\Files\NotPermittedException;
use OCP\IConfig;
Expand All @@ -40,34 +41,19 @@
class ThemingController extends Controller {
public const VALID_UPLOAD_KEYS = ['header', 'logo', 'logoheader', 'background', 'favicon'];

private ThemingDefaults $themingDefaults;
private IL10N $l10n;
private IConfig $config;
private IURLGenerator $urlGenerator;
private IAppManager $appManager;
private ImageManager $imageManager;
private ThemesService $themesService;

public function __construct(
$appName,
IRequest $request,
IConfig $config,
ThemingDefaults $themingDefaults,
IL10N $l,
IURLGenerator $urlGenerator,
IAppManager $appManager,
ImageManager $imageManager,
ThemesService $themesService
private IConfig $config,
private IAppConfig $appConfig,
private ThemingDefaults $themingDefaults,
private IL10N $l10n,
private IURLGenerator $urlGenerator,
private IAppManager $appManager,
private ImageManager $imageManager,
private ThemesService $themesService,
) {
parent::__construct($appName, $request);

$this->themingDefaults = $themingDefaults;
$this->l10n = $l;
$this->config = $config;
$this->urlGenerator = $urlGenerator;
$this->appManager = $appManager;
$this->imageManager = $imageManager;
$this->themesService = $themesService;
}

/**
Expand All @@ -80,6 +66,7 @@ public function __construct(
public function updateStylesheet($setting, $value) {
$value = trim($value);
$error = null;
$saved = false;
switch ($setting) {
case 'name':
if (strlen($value) > 250) {
Expand Down Expand Up @@ -118,16 +105,25 @@ public function updateStylesheet($setting, $value) {
case 'primary_color':
if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $value)) {
$error = $this->l10n->t('The given color is invalid');
} else {
$this->appConfig->setAppValueString('primary_color', $value);
$saved = true;
}
break;
case 'background_color':
if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $value)) {
$error = $this->l10n->t('The given color is invalid');
} else {
$this->appConfig->setAppValueString('background_color', $value);
$saved = true;
}
break;
case 'disable-user-theming':
if ($value !== 'yes' && $value !== 'no') {
if (!in_array($value, ['yes', 'true', 'no', 'false'])) {
$error = $this->l10n->t('Disable-user-theming should be true or false');
} else {
$this->appConfig->setAppValueBool('disable-user-theming', $value === 'yes' || $value === 'true');
$saved = true;
}
break;
}
Expand All @@ -140,7 +136,9 @@ public function updateStylesheet($setting, $value) {
], Http::STATUS_BAD_REQUEST);
}

$this->themingDefaults->set($setting, $value);
if (!$saved) {
$this->themingDefaults->set($setting, $value);
}

return new DataResponse([
'data' => [
Expand Down
19 changes: 9 additions & 10 deletions apps/theming/lib/Migration/SeparatePrimaryColorAndBackground.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@
namespace OCA\Theming\Migration;

use OCA\Theming\AppInfo\Application;
use OCP\IConfig;
use OCP\IAppConfig;
use OCP\IDBConnection;
use OCP\Migration\IOutput;

class SeparatePrimaryColorAndBackground implements \OCP\Migration\IRepairStep {

public function __construct(
private IConfig $config,
private IAppConfig $appConfig,
private IDBConnection $connection,
) {
}
Expand All @@ -27,25 +27,24 @@ public function getName() {
}

public function run(IOutput $output) {

Check notice

Code scanning / Psalm

MissingReturnType Note

Method OCA\Theming\Migration\SeparatePrimaryColorAndBackground::run does not have a return type, expecting void
$defaultColor = $this->config->getAppValue(Application::APP_ID, 'color', '');
$defaultColor = $this->appConfig->getValueString(Application::APP_ID, 'color', '');
if ($defaultColor !== '') {
// Restore legacy value into new field
$this->config->setAppValue(Application::APP_ID, 'background_color', $defaultColor);
$this->config->setAppValue(Application::APP_ID, 'primary_color', $defaultColor);
$this->appConfig->setValueString(Application::APP_ID, 'background_color', $defaultColor);
$this->appConfig->setValueString(Application::APP_ID, 'primary_color', $defaultColor);
// Delete legacy field
$this->config->deleteAppValue(Application::APP_ID, 'color');
$this->appConfig->deleteKey(Application::APP_ID, 'color');
// give some feedback
$output->info('Global primary color restored');
}

// This can only be executed once because `background_color` is again used with Nextcloud 30,
// so this part only works when updating -> Nextcloud 29 -> 30
$migrated = $this->config->getAppValue('theming', 'nextcloud_30_migration', 'false') === 'true';
if ($migrated) {
if ($this->appConfig->getValueBool('theming', 'nextcloud_30_migration')) {
return;
}

$userThemingEnabled = $this->config->getAppValue('theming', 'disable-user-theming', 'no') !== 'yes';
$userThemingEnabled = $this->appConfig->getValueBool('theming', 'disable-user-theming');
if ($userThemingEnabled) {
$output->info('Restoring user primary color');
// For performance let the DB handle this
Expand All @@ -59,6 +58,6 @@ public function run(IOutput $output) {
$qb->executeStatement();
$output->info('Primary color of users restored');
}
$this->config->setAppValue('theming', 'nextcloud_30_migration', 'true');
$this->appConfig->setValueBool('theming', 'nextcloud_30_migration', true);
}
}
10 changes: 6 additions & 4 deletions apps/theming/lib/ThemingDefaults.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use OCP\App\IAppManager;
use OCP\Files\NotFoundException;
use OCP\Files\SimpleFS\ISimpleFile;
use OCP\IAppConfig;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IL10N;
Expand Down Expand Up @@ -39,6 +40,7 @@ class ThemingDefaults extends \OC_Defaults {
*/
public function __construct(
private IConfig $config,
private IAppConfig $appConfig,
private IL10N $l,
private IUserSession $userSession,
private IURLGenerator $urlGenerator,
Expand Down Expand Up @@ -221,7 +223,7 @@ public function getColorBackground(): string {
*/
public function getDefaultColorPrimary(): string {
// try admin color
$defaultColor = $this->config->getAppValue(Application::APP_ID, 'primary_color', '');
$defaultColor = $this->appConfig->getValueString(Application::APP_ID, 'primary_color', '');
if (preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $defaultColor)) {
return $defaultColor;
}
Expand All @@ -234,7 +236,7 @@ public function getDefaultColorPrimary(): string {
* Default background color only taking admin setting into account
*/
public function getDefaultColorBackground(): string {
$defaultColor = $this->config->getAppValue(Application::APP_ID, 'background_color', '');
$defaultColor = $this->appConfig->getValueString(Application::APP_ID, 'background_color');
if (preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $defaultColor)) {
return $defaultColor;
}
Expand Down Expand Up @@ -344,7 +346,7 @@ public function getScssVariables() {
$variables['image-login-background'] = "url('".$this->imageManager->getImageUrl('background')."')";
$variables['image-login-plain'] = 'false';

if ($this->config->getAppValue('theming', 'primary_color', '') !== '') {
if ($this->appConfig->getValueString(Application::APP_ID, 'primary_color', '') !== '') {
$variables['color-primary'] = $this->getColorPrimary();
$variables['color-primary-text'] = $this->getTextColorPrimary();
$variables['color-primary-element'] = $this->util->elementColor($this->getColorPrimary());
Expand Down Expand Up @@ -520,6 +522,6 @@ public function getDefaultTextColorPrimary() {
* Has the admin disabled user customization
*/
public function isUserThemingDisabled(): bool {
return $this->config->getAppValue('theming', 'disable-user-theming', 'no') === 'yes';
return $this->appConfig->getValueBool(Application::APP_ID, 'disable-user-theming');
}
}
33 changes: 15 additions & 18 deletions apps/theming/tests/Controller/ThemingControllerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use OCP\App\IAppManager;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Services\IAppConfig;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Files\NotFoundException;
use OCP\Files\SimpleFS\ISimpleFile;
Expand All @@ -24,28 +25,23 @@
use Test\TestCase;

class ThemingControllerTest extends TestCase {
/** @var IRequest|MockObject */
private $request;
/** @var IConfig|MockObject */
private $config;
/** @var ThemingDefaults|MockObject */
private $themingDefaults;
/** @var IL10N|MockObject */
private $l10n;
/** @var ThemingController */
private $themingController;
/** @var IAppManager|MockObject */
private $appManager;
/** @var ImageManager|MockObject */
private $imageManager;
/** @var IURLGenerator|MockObject */
private $urlGenerator;
/** @var ThemesService|MockObject */
private $themesService;

private IRequest&MockObject $request;
private IConfig&MockObject $config;
private IAppConfig&MockObject $appConfig;
private ThemingDefaults&MockObject $themingDefaults;
private IL10N&MockObject $l10n;
private IAppManager&MockObject $appManager;
private ImageManager&MockObject $imageManager;
private IURLGenerator&MockObject $urlGenerator;
private ThemesService&MockObject $themesService;

private ThemingController $themingController;

protected function setUp(): void {
$this->request = $this->createMock(IRequest::class);
$this->config = $this->createMock(IConfig::class);
$this->appConfig = $this->createMock(IAppConfig::class);
$this->themingDefaults = $this->createMock(ThemingDefaults::class);
$this->l10n = $this->createMock(L10N::class);
$this->appManager = $this->createMock(IAppManager::class);
Expand All @@ -64,6 +60,7 @@ protected function setUp(): void {
'theming',
$this->request,
$this->config,
$this->appConfig,
$this->themingDefaults,
$this->l10n,
$this->urlGenerator,
Expand Down
Loading

0 comments on commit 7df222c

Please sign in to comment.