fa/breadcrumb-the-shire
1
0
Fork 1
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

refactor(tenants): visibility form requires explicit appearance values

Browse Source 
Simplifies the tenant Visibility tab to match the tenant-only appearance
model established by the previous commits:

- primary_color: now required. Removed the "No brand color / Use system
  default (no brand accent)" toggle — every tenant carries a concrete
  primary color. Legacy NULL rows render the neutral app default (#2fa4a4)
  in the color picker and are converted to that explicit hex on save.
- default_theme: the select no longer offers a "Use system default" empty
  option. Legacy NULL rows resolve to 'light' at render time; saves always
  persist a valid theme via SettingsAppGateway::normalizeTheme().
- allow_user_theme: replaced the tri-state "Use system default (allowed) /
  Force allow / Force disallow" select with a single boolean switch
  ("Users may choose their own theme"). Legacy NULL rows load as checked.
  Saves persist 0/1 explicitly.

TenantService: sanitize no longer reads primary_color_use_default or
allow_user_theme_mode; it validates primary_color as a required hex and
treats allow_user_theme as a plain boolean. Both create and update paths
write concrete values only — no more NULL writes for these three fields.

DirectorySettingsGateway gains a normalizeTheme() delegate so TenantService
can route through the same gateway it uses for isAllowedTheme().

Removed now-unused app-color-default-toggle JS component + its runtime
registration + its architecture-test entry. i18n cleanup: "No brand
color", "Use system default (no brand accent)", "When enabled the tenant
renders without a brand accent color.", "Use system default (light)",
"Use system default (allowed)", "Force allow user theme", "Force disallow
user theme", "User theme policy is invalid" all removed. New copy: "Users
may choose their own theme" + helper text, plus a tightened tab blockquote.

Tests: TenantServiceTest validInput() updated to send concrete values;
settingsGateway mock gets normalizeTheme() + isAllowedTheme() defaults.
All 1985 tests pass; PHPStan level 5 clean; QG-006 clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Felix Schneider
2026-04-22 23:10:44 +02:00
parent c4c294edd7
commit 68453fd6ed
9 changed files with 69 additions and 174 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
core/Service/Directory/DirectorySettingsGateway.php
View File

@@ -32,4 +32,9 @@ class DirectorySettingsGateway
{
return $this->settingsAppGateway->isAllowedTheme($theme);
}
public function normalizeTheme(?string $theme): string
{
return $this->settingsAppGateway->normalizeTheme($theme);
}
}

31
core/Service/Tenant/TenantService.php
View File

@@ -68,12 +68,12 @@ class TenantService
'website' => $form['website'],
'privacy_url' => $form['privacy_url'],
'imprint_url' => $form['imprint_url'],
// null means "inherit from global settings" for color, theme, and theme-toggle policy.
'primary_color' => $form['primary_color_use_default']
? null
: ($form['primary_color'] !== '' ? $form['primary_color'] : null),
'default_theme' => $form['default_theme'] !== '' ? $form['default_theme'] : null,
'allow_user_theme' => $form['allow_user_theme_mode'] === '' ? null : (int) $form['allow_user_theme_mode'],
// Appearance is tenant-scoped with explicit, required values. Every
// tenant has a primary color, a default theme, and an explicit
// allow-user-theme flag — there is no global fallback to inherit.
'primary_color' => strtolower($form['primary_color']),
'default_theme' => $this->settingsGateway->normalizeTheme($form['default_theme']),
'allow_user_theme' => $form['allow_user_theme'] ? 1 : 0,
'status' => $form['status'],
'status_changed_at' => $statusChangedAt,
'status_changed_by' => $currentUserId > 0 ? $currentUserId : null,
@@ -135,11 +135,9 @@ class TenantService
'website' => $form['website'],
'privacy_url' => $form['privacy_url'],
'imprint_url' => $form['imprint_url'],
'primary_color' => $form['primary_color_use_default']
? null
: ($form['primary_color'] !== '' ? $form['primary_color'] : null),
'default_theme' => $form['default_theme'] !== '' ? $form['default_theme'] : null,
'allow_user_theme' => $form['allow_user_theme_mode'] === '' ? null : (int) $form['allow_user_theme_mode'],
'primary_color' => strtolower($form['primary_color']),
'default_theme' => $this->settingsGateway->normalizeTheme($form['default_theme']),
'allow_user_theme' => $form['allow_user_theme'] ? 1 : 0,
'status' => $form['status'],
'modified_by' => $currentUserId > 0 ? $currentUserId : null,
];
@@ -223,9 +221,8 @@ class TenantService
'privacy_url' => trim((string) ($input['privacy_url'] ?? '')),
'imprint_url' => trim((string) ($input['imprint_url'] ?? '')),
'primary_color' => trim((string) ($input['primary_color'] ?? '')),
'primary_color_use_default' => !empty($input['primary_color_use_default']),
'default_theme' => strtolower(trim((string) ($input['default_theme'] ?? ''))),
'allow_user_theme_mode' => trim((string) ($input['allow_user_theme_mode'] ?? '')),
'allow_user_theme' => isset($input['allow_user_theme']) ? (bool) $input['allow_user_theme'] : true,
'status' => trim((string) ($input['status'] ?? '')),
];
}
@@ -240,18 +237,14 @@ class TenantService
$errors[] = t('Status is invalid');
}
if (
!$form['primary_color_use_default']
&& $form['primary_color'] !== ''
&& !preg_match('/^#([0-9a-f]{3}|[0-9a-f]{6})$/i', $form['primary_color'])
$form['primary_color'] === ''
|| !preg_match('/^#([0-9a-f]{3}|[0-9a-f]{6})$/i', $form['primary_color'])
) {
$errors[] = t('Primary color is invalid');
}
if ($form['default_theme'] !== '' && !$this->settingsGateway->isAllowedTheme((string) $form['default_theme'])) {
$errors[] = t('Default theme is invalid');
}
if (!in_array($form['allow_user_theme_mode'], ['', '0', '1'], true)) {
$errors[] = t('User theme policy is invalid');
}
return $errors;
}

12
i18n/default_de.json
View File

@@ -425,13 +425,10 @@
"Appearance": "Darstellung",
"API docs": "API-Doku",
"Can view API documentation": "Kann API-Dokumentation anzeigen",
"Appearance is controlled per tenant. If a field is left at the system default, the app renders in light theme without a brand accent.": "Die Darstellung wird pro Mandant gesteuert. Wenn ein Feld auf Systemstandard bleibt, rendert die App im hellen Theme ohne Markenakzent.",
"No brand color": "Keine Markenfarbe",
"Use system default (no brand accent)": "Systemstandard verwenden (kein Markenakzent)",
"When enabled the tenant renders without a brand accent color.": "Wenn aktiv, rendert der Mandant ohne Markenakzent-Farbe.",
"Use system default (light)": "Systemstandard verwenden (hell)",
"Appearance is controlled per tenant. Every tenant carries its own primary color and default theme.": "Die Darstellung wird pro Mandant gesteuert. Jeder Mandant hat eine eigene Primärfarbe und ein eigenes Standard-Theme.",
"The theme used for users in this tenant who have no personal preference.": "Das Theme für Nutzer dieses Mandanten, die keine persönliche Präferenz gesetzt haben.",
"Use system default (allowed)": "Systemstandard verwenden (erlaubt)",
"Users may choose their own theme": "Benutzer dürfen ihr eigenes Theme wählen",
"Controls whether users in this tenant can override the tenant default with a personal theme preference.": "Steuert, ob Benutzer dieses Mandanten den Mandanten-Standard durch eine persönliche Theme-Präferenz überschreiben dürfen.",
"Inactive tenants are excluded from user access and tenant-scoped data.": "Inaktive Mandanten werden vom Benutzerzugriff und mandantenspezifischen Daten ausgeschlossen.",
"Status & meta": "Status & Meta",
"Audit": "Audit",
@@ -759,10 +756,7 @@
"Filterable fields are available in address book filters.": "Filterbare Felder sind im Adressbuch als Filter verfügbar.",
"Inactive fields are hidden in forms and filters.": "Inaktive Felder sind in Formularen und Filtern ausgeblendet.",
"Default theme is invalid": "Standard-Theme ist ungültig",
"User theme policy is invalid": "Benutzer-Theme-Richtlinie ist ungültig",
"Theme change is disabled for this tenant": "Theme-Wechsel ist für diesen Mandanten deaktiviert",
"Force allow user theme": "Benutzer-Theme erlauben (erzwingen)",
"Force disallow user theme": "Benutzer-Theme verbieten (erzwingen)",
"Controls whether users in this tenant can choose their own theme.": "Steuert, ob Benutzer in diesem Mandanten ihr eigenes Theme wählen können.",
"Tenants can override color and theme behavior in their appearance settings.": "Mandanten können Farbe und Theme-Verhalten in ihren Darstellungs-Einstellungen überschreiben.",
"User theme policy": "Benutzer-Theme-Richtlinie",

12
i18n/default_en.json
View File

@@ -425,13 +425,10 @@
"Appearance": "Appearance",
"API docs": "API docs",
"Can view API documentation": "Can view API documentation",
"Appearance is controlled per tenant. If a field is left at the system default, the app renders in light theme without a brand accent.": "Appearance is controlled per tenant. If a field is left at the system default, the app renders in light theme without a brand accent.",
"No brand color": "No brand color",
"Use system default (no brand accent)": "Use system default (no brand accent)",
"When enabled the tenant renders without a brand accent color.": "When enabled the tenant renders without a brand accent color.",
"Use system default (light)": "Use system default (light)",
"Appearance is controlled per tenant. Every tenant carries its own primary color and default theme.": "Appearance is controlled per tenant. Every tenant carries its own primary color and default theme.",
"Users may choose their own theme": "Users may choose their own theme",
"Controls whether users in this tenant can override the tenant default with a personal theme preference.": "Controls whether users in this tenant can override the tenant default with a personal theme preference.",
"The theme used for users in this tenant who have no personal preference.": "The theme used for users in this tenant who have no personal preference.",
"Use system default (allowed)": "Use system default (allowed)",
"Inactive tenants are excluded from user access and tenant-scoped data.": "Inactive tenants are excluded from user access and tenant-scoped data.",
"Status & meta": "Status & meta",
"Audit": "Audit",
@@ -759,10 +756,7 @@
"Filterable fields are available in address book filters.": "Filterable fields are available in address book filters.",
"Inactive fields are hidden in forms and filters.": "Inactive fields are hidden in forms and filters.",
"Default theme is invalid": "Default theme is invalid",
"User theme policy is invalid": "User theme policy is invalid",
"Theme change is disabled for this tenant": "Theme change is disabled for this tenant",
"Force allow user theme": "Force allow user theme",
"Force disallow user theme": "Force disallow user theme",
"Controls whether users in this tenant can choose their own theme.": "Controls whether users in this tenant can choose their own theme.",
"Tenants can override color and theme behavior in their appearance settings.": "Tenants can override color and theme behavior in their appearance settings.",
"User theme policy": "User theme policy",

99
pages/admin/tenants/_form.phtml
View File

@@ -34,22 +34,27 @@ $customFieldDefinitions = is_array($customFieldDefinitions ?? null) ? $customFie
$readonlyAttr = $isReadOnly ? 'readonly' : '';
$disabledAttr = $isReadOnly ? 'disabled' : '';
$themes = appThemes();
// Primary color is required per tenant — appearance is tenant-scoped and
// there is no "no brand color" opt-out. Legacy rows (NULL) are rendered
// with the neutral app default; saving converts NULL to that explicit hex.
$primaryColor = (string) ($values['primary_color'] ?? '');
// No global fallback — appearance is tenant-scoped. The color picker's
// "use default" preview uses a neutral hex so the UI is never empty.
$defaultPrimaryColor = '#2fa4a4';
$useDefaultPrimaryColor = $primaryColor === '';
$tenantDefaultTheme = strtolower(trim((string) ($values['default_theme'] ?? '')));
if ($tenantDefaultTheme !== '' && !isset($themes[$tenantDefaultTheme])) {
$tenantDefaultTheme = '';
if ($primaryColor === '' || !preg_match('/^#([0-9a-f]{3}|[0-9a-f]{6})$/i', $primaryColor)) {
$primaryColor = '#2fa4a4';
}
$rawAllowUserThemeMode = $values['allow_user_theme_mode'] ?? ($values['allow_user_theme'] ?? null);
if ($rawAllowUserThemeMode === null || $rawAllowUserThemeMode === '') {
$tenantAllowUserThemeMode = '';
} elseif (in_array(strtolower((string) $rawAllowUserThemeMode), ['1', 'true', 'yes', 'on'], true)) {
$tenantAllowUserThemeMode = '1';
// Resolve tenant's default theme against the catalog; fall back to 'light'
// for legacy rows or unknown values. The select never renders an empty
// option — appearance is tenant-scoped, no global setting to inherit.
$tenantDefaultTheme = strtolower(trim((string) ($values['default_theme'] ?? '')));
if ($tenantDefaultTheme === '' || !isset($themes[$tenantDefaultTheme])) {
$tenantDefaultTheme = isset($themes['light']) ? 'light' : (array_key_first($themes) ?: 'light');
}
// allow_user_theme: boolean — true when set or unset (legacy NULL), false
// only when explicitly disabled. Eliminates the old tri-state select.
$rawAllowUserTheme = $values['allow_user_theme'] ?? null;
if ($rawAllowUserTheme === null || $rawAllowUserTheme === '') {
$tenantAllowUserTheme = true;
} else {
$tenantAllowUserThemeMode = '0';
$tenantAllowUserTheme = in_array(strtolower((string) $rawAllowUserTheme), ['1', 'true', 'yes', 'on'], true);
}
$microsoftEnabledRaw = $values['microsoft_enabled'] ?? false;
$microsoftEnabled = in_array(strtolower(trim((string) $microsoftEnabledRaw)), ['1', 'true', 'yes', 'on'], true);
@@ -933,45 +938,25 @@ $openOverrideCard = $detailsOpenAll;
<div data-tab-panel="visibility">
<blockquote data-variant="info">
<?php e(t('Appearance is controlled per tenant. If a field is left at the system default, the app renders in light theme without a brand accent.')); ?>
<?php e(t('Appearance is controlled per tenant. Every tenant carries its own primary color and default theme.')); ?>
</blockquote>
<div class="grid">
<fieldset>
<legend><small><?php e(t('Primary color')); ?></small></legend>
<label for="primary_color">
<input type="color" name="primary_color" id="primary_color"
value="<?php e($primaryColor !== '' ? $primaryColor : $defaultPrimaryColor); ?>"
<?php e($readonlyAttr); ?> />
<div>
<?php e(t('Choose color')); ?>
</div>
</label>
<small class="muted"><?php e(t('The primary color affects buttons and accent elements across the app.')); ?></small>
</fieldset>
<fieldset>
<legend>
<small>
<?php e(t('No brand color')); ?>
</small>
</legend>
<label class="app-field">
<input type="hidden" name="primary_color_use_default" value="0">
<input type="checkbox" role="switch" name="primary_color_use_default" value="1"
data-color-default-toggle
data-color-target="#primary_color"
data-color-default="<?php e($defaultPrimaryColor); ?>"
<?php e($useDefaultPrimaryColor ? 'checked' : ''); ?>
<?php e($readonlyAttr); ?>>
<span><?php e(t('Use system default (no brand accent)')); ?></span>
</label>
<small class="muted"><?php e(t('When enabled the tenant renders without a brand accent color.')); ?></small>
</fieldset>
</div>
<fieldset>
<legend><small><?php e(t('Primary color')); ?></small></legend>
<label for="primary_color">
<input type="color" name="primary_color" id="primary_color"
value="<?php e($primaryColor); ?>"
required
<?php e($readonlyAttr); ?> />
<div>
<?php e(t('Choose color')); ?>
</div>
</label>
<small class="muted"><?php e(t('The primary color affects buttons and accent elements across the app.')); ?></small>
</fieldset>
<div class="grid">
<fieldset>
<legend><small><?php e(t('Default theme')); ?></small></legend>
<select name="default_theme" <?php e($disabledAttr); ?>>
<option value=""><?php e(t('Use system default (light)')); ?></option>
<?php foreach ($themes as $themeKey => $themeLabel): ?>
<option value="<?php e($themeKey); ?>" <?php e($tenantDefaultTheme === $themeKey ? 'selected' : ''); ?>>
<?php e(t($themeLabel)); ?>
@@ -982,18 +967,14 @@ $openOverrideCard = $detailsOpenAll;
</fieldset>
<fieldset>
<legend><small><?php e(t('User theme policy')); ?></small></legend>
<select name="allow_user_theme_mode" <?php e($disabledAttr); ?>>
<option value="" <?php e($tenantAllowUserThemeMode === '' ? 'selected' : ''); ?>>
<?php e(t('Use system default (allowed)')); ?>
</option>
<option value="1" <?php e($tenantAllowUserThemeMode === '1' ? 'selected' : ''); ?>>
<?php e(t('Force allow user theme')); ?>
</option>
<option value="0" <?php e($tenantAllowUserThemeMode === '0' ? 'selected' : ''); ?>>
<?php e(t('Force disallow user theme')); ?>
</option>
</select>
<small class="muted"><?php e(t('Controls whether users in this tenant can choose their own theme.')); ?></small>
<label class="app-field">
<input type="hidden" name="allow_user_theme" value="0">
<input type="checkbox" role="switch" name="allow_user_theme" value="1"
<?php e($tenantAllowUserTheme ? 'checked' : ''); ?>
<?php e($readonlyAttr); ?>>
<span><?php e(t('Users may choose their own theme')); ?></span>
</label>
<small class="muted"><?php e(t('Controls whether users in this tenant can override the tenant default with a personal theme preference.')); ?></small>
</fieldset>
</div>
<?php

1
tests/Architecture/FrontendRuntimeContractSupport.php
View File

@@ -24,7 +24,6 @@ trait FrontendRuntimeContractSupport
'web/js/components/app-confirm-actions.js',
'web/js/components/app-details-state.js',
'web/js/components/app-tenant-switcher.js',
'web/js/components/app-color-default-toggle.js',
'web/js/components/app-custom-field-options-toggle.js',
'web/js/components/app-tenant-sso-toggle.js',
'web/js/components/app-settings-telemetry.js',

10
tests/Service/Tenant/TenantServiceTest.php
View File

@@ -23,6 +23,9 @@ class TenantServiceTest extends TestCase
$this->tenantRepository = $this->createMock(TenantRepositoryInterface::class);
$this->departmentRepository = $this->createMock(DepartmentRepositoryInterface::class);
$this->settingsGateway = $this->createMock(DirectorySettingsGateway::class);
$this->settingsGateway->method('isAllowedTheme')->willReturn(true);
$this->settingsGateway->method('normalizeTheme')
->willReturnCallback(static fn (?string $t): string => $t ?? 'light');
$this->systemAuditService = $this->createMock(AuditRecorderInterface::class);
$this->service = new TenantService(
@@ -196,10 +199,9 @@ class TenantServiceTest extends TestCase
'website' => '',
'privacy_url' => '',
'imprint_url' => '',
'primary_color' => '',
'primary_color_use_default' => true,
'default_theme' => '',
'allow_user_theme_mode' => '',
'primary_color' => '#2fa4a4',
'default_theme' => 'light',
'allow_user_theme' => '1',
];
}
}

5
web/js/app-init.js
View File

@@ -15,7 +15,6 @@ import { initContrastToggle } from './components/app-contrast-toggle.js';
import { initConfirmActions } from './components/app-confirm-actions.js';
import { initDetailsState } from './components/app-details-state.js';
import { initTenantSwitcher } from './components/app-tenant-switcher.js';
import { initColorDefaultToggle } from './components/app-color-default-toggle.js';
import { initCustomFieldOptionsToggle } from './components/app-custom-field-options-toggle.js';
import { initTenantSsoToggle } from './components/app-tenant-sso-toggle.js';
import { initTenantLdapToggle } from './components/app-tenant-ldap-toggle.js';
@@ -233,10 +232,6 @@ const bootRuntime = async () => {
selector: '[data-app-component="tenant-switcher"]',
configPath: 'components.tenantSwitcher',
});
runtime.register('color-default-toggle', initColorDefaultToggle, {
scope: 'global',
configPath: 'components.colorDefaultToggle',
});
runtime.register('custom-field-options-toggle', initCustomFieldOptionsToggle, {
scope: 'global',
configPath: 'components.customFieldOptionsToggle',

68
web/js/components/app-color-default-toggle.js
View File

@@ -1,68 +0,0 @@
/**
* Toggles color input between custom value and default/inherited color.
*/
import { warnOnce } from '../core/app-dom.js';
import { createConditionalToggleInit } from '../core/app-conditional-controls.js';
const syncToggle = (toggle, scope) => {
const targetSelector = toggle.dataset.colorTarget || '';
if (!targetSelector) {
warnOnce('UI_EL_MISSING', 'Missing data-color-target', { module: 'color-default-toggle' });
return;
}
const target = scope.querySelector(targetSelector);
if (!(target instanceof HTMLInputElement)) {
warnOnce('UI_EL_MISSING', `Missing color target: ${targetSelector}`, { module: 'color-default-toggle' });
return;
}
const defaultValue = toggle.dataset.colorDefault || '#2fa4a4';
if (toggle.checked) {
if (!target.dataset.customValue) {
target.dataset.customValue = target.value;
}
target.value = defaultValue;
target.setAttribute('disabled', 'disabled');
} else {
target.removeAttribute('disabled');
if (target.dataset.customValue) {
target.value = target.dataset.customValue;
}
}
};
const initRoot = (toggle) => {
if (!(toggle instanceof HTMLInputElement)) {
return null;
}
const scope = document;
const targetSelector = toggle.dataset.colorTarget || '';
const target = targetSelector ? scope.querySelector(targetSelector) : null;
if (targetSelector && !target) {
warnOnce('UI_EL_MISSING', `Missing color target: ${targetSelector}`, { module: 'color-default-toggle' });
}
const onToggleChange = () => syncToggle(toggle, scope);
toggle.addEventListener('change', onToggleChange);
const cleanups = [() => toggle.removeEventListener('change', onToggleChange)];
if (target instanceof HTMLInputElement) {
const onTargetInput = () => {
if (toggle.checked) {
toggle.checked = false;
target.removeAttribute('disabled');
}
};
target.addEventListener('input', onTargetInput);
cleanups.push(() => target.removeEventListener('input', onTargetInput));
}
syncToggle(toggle, scope);
return () => cleanups.forEach((fn) => fn());
};
export const initColorDefaultToggle = createConditionalToggleInit({
rootSelector: '[data-color-default-toggle]',
boundKey: 'colorDefaultToggleBound',
initRoot,
});