From 68453fd6ed01838e7c5b8269ab9c8f7b8f07e741 Mon Sep 17 00:00:00 2001 From: fs Date: Wed, 22 Apr 2026 23:10:44 +0200 Subject: [PATCH] refactor(tenants): visibility form requires explicit appearance values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../Directory/DirectorySettingsGateway.php | 5 + core/Service/Tenant/TenantService.php | 31 +++--- i18n/default_de.json | 12 +-- i18n/default_en.json | 12 +-- pages/admin/tenants/_form.phtml | 99 ++++++++----------- .../FrontendRuntimeContractSupport.php | 1 - tests/Service/Tenant/TenantServiceTest.php | 10 +- web/js/app-init.js | 5 - web/js/components/app-color-default-toggle.js | 68 ------------- 9 files changed, 69 insertions(+), 174 deletions(-) delete mode 100644 web/js/components/app-color-default-toggle.js diff --git a/core/Service/Directory/DirectorySettingsGateway.php b/core/Service/Directory/DirectorySettingsGateway.php index f431939..09fc8b2 100644 --- a/core/Service/Directory/DirectorySettingsGateway.php +++ b/core/Service/Directory/DirectorySettingsGateway.php @@ -32,4 +32,9 @@ class DirectorySettingsGateway { return $this->settingsAppGateway->isAllowedTheme($theme); } + + public function normalizeTheme(?string $theme): string + { + return $this->settingsAppGateway->normalizeTheme($theme); + } } diff --git a/core/Service/Tenant/TenantService.php b/core/Service/Tenant/TenantService.php index 0b1f2ff..82eb8e3 100644 --- a/core/Service/Tenant/TenantService.php +++ b/core/Service/Tenant/TenantService.php @@ -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; } diff --git a/i18n/default_de.json b/i18n/default_de.json index b9875a9..58ce2e6 100644 --- a/i18n/default_de.json +++ b/i18n/default_de.json @@ -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", diff --git a/i18n/default_en.json b/i18n/default_en.json index 02c4bcc..85d72b3 100644 --- a/i18n/default_en.json +++ b/i18n/default_en.json @@ -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", diff --git a/pages/admin/tenants/_form.phtml b/pages/admin/tenants/_form.phtml index 920aef9..7edfd5c 100644 --- a/pages/admin/tenants/_form.phtml +++ b/pages/admin/tenants/_form.phtml @@ -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;
- +
-
-
- - - -
-
- - - - - - - -
-
+
+ + + +
> - - - - - + +
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', ]; } } diff --git a/web/js/app-init.js b/web/js/app-init.js index 0c21400..b30b10b 100644 --- a/web/js/app-init.js +++ b/web/js/app-init.js @@ -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', diff --git a/web/js/components/app-color-default-toggle.js b/web/js/components/app-color-default-toggle.js deleted file mode 100644 index aa40ef1..0000000 --- a/web/js/components/app-color-default-toggle.js +++ /dev/null @@ -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, -});