From 99f8b55d49a5fe5c92b56956c57cfe2d0ec9018d Mon Sep 17 00:00:00 2001 From: fs Date: Mon, 13 Apr 2026 22:22:15 +0200 Subject: [PATCH] refactor: consolidate JS toggle components, grid query parsers, and add tests JS components: migrate app-custom-field-options-toggle, app-color-default-toggle, and app-settings-telemetry to shared conditional-controls factory/utility, removing duplicated syncControlState logic and init boilerplate. grid.php: extract gridQueryCsvInput() to DRY up 3 CSV parsers, simplify multi_csv handler via gridNormalizeLabelList, delegate order type to gridQueryEnum. Tests: add 23 SearchQueryNormalizer tests (LIKE escaping, wildcards, Unicode) and 7 additional Crypto edge-case tests (empty input, missing fields, special chars, binary content, truncation). Co-Authored-By: Claude Opus 4.6 --- lib/Support/helpers/grid.php | 77 ++++------ tests/Unit/Support/CryptoTest.php | 54 +++++++ .../Support/SearchQueryNormalizerTest.php | 137 ++++++++++++++++++ web/js/components/app-color-default-toggle.js | 78 ++++------ .../app-custom-field-options-toggle.js | 120 ++++++--------- web/js/components/app-settings-telemetry.js | 11 +- 6 files changed, 297 insertions(+), 180 deletions(-) create mode 100644 tests/Unit/Support/SearchQueryNormalizerTest.php diff --git a/lib/Support/helpers/grid.php b/lib/Support/helpers/grid.php index ca3a561..8ee090d 100644 --- a/lib/Support/helpers/grid.php +++ b/lib/Support/helpers/grid.php @@ -269,26 +269,7 @@ function gridBuildToolbarFilterState(array $toolbarSchema, array $filterState): $value = array_key_exists($key, $filterState) ? $filterState[$key] : $default; if ($type === 'multi_csv') { - if (is_array($value)) { - $items = array_values(array_filter( - array_map(static fn (mixed $item): string => trim((string) $item), $value), - static fn (string $item): bool => $item !== '' - )); - $state[$key] = array_values(array_unique($items)); - continue; - } - - $text = trim((string) $value); - if ($text === '') { - $state[$key] = []; - continue; - } - - $items = array_values(array_filter( - array_map(static fn (string $item): string => trim($item), explode(',', $text)), - static fn (string $item): bool => $item !== '' - )); - $state[$key] = array_values(array_unique($items)); + $state[$key] = array_values(array_unique(gridNormalizeLabelList($value, ','))); continue; } @@ -614,9 +595,7 @@ function gridParseFilters(array $query, array $schema): array (array) ($rule['allowed'] ?? []) ), static fn (string $item): bool => $item !== '')); $default = (string) ($rule['default'] ?? ($allowed[0] ?? '')); - $parsed[$key] = in_array(trim((string) ($query[$source] ?? $default)), $allowed, true) - ? trim((string) ($query[$source] ?? $default)) - : $default; + $parsed[$key] = gridQueryEnum($query, $source, $allowed, $default); continue; } @@ -719,6 +698,28 @@ function gridQueryEnum(array $query, string $key, array $allowed, string $defaul return in_array($value, $allowed, true) ? $value : $default; } +/** + * Extract raw items from a comma-separated or array query parameter. + * + * @return list + */ +function gridQueryCsvInput(array $query, string $key): array +{ + $value = $query[$key] ?? ''; + if (is_string($value)) { + return $value === '' ? [] : explode(',', $value); + } + if (is_array($value)) { + $items = []; + array_walk_recursive($value, static function (mixed $item) use (&$items): void { + $items[] = $item; + }); + return $items; + } + + return []; +} + /** * Normalize comma-separated positive IDs from query params. * @@ -730,15 +731,7 @@ function gridQueryCsvIds(array $query, string $key, int $max = 200): array return []; } - $value = $query[$key] ?? ''; - $items = []; - if (is_string($value)) { - $items = $value === '' ? [] : explode(',', $value); - } elseif (is_array($value)) { - array_walk_recursive($value, static function (mixed $item) use (&$items): void { - $items[] = $item; - }); - } + $items = gridQueryCsvInput($query, $key); $ids = []; $seen = []; @@ -768,15 +761,7 @@ function gridQueryCsvUuids(array $query, string $key, int $max = 200): array return []; } - $value = $query[$key] ?? ''; - $items = []; - if (is_string($value)) { - $items = $value === '' ? [] : explode(',', $value); - } elseif (is_array($value)) { - array_walk_recursive($value, static function (mixed $item) use (&$items): void { - $items[] = $item; - }); - } + $items = gridQueryCsvInput($query, $key); $uuids = []; $seen = []; @@ -806,15 +791,7 @@ function gridQueryCsvStrings(array $query, string $key, int $max = 200, ?callabl return []; } - $value = $query[$key] ?? ''; - $items = []; - if (is_string($value)) { - $items = $value === '' ? [] : explode(',', $value); - } elseif (is_array($value)) { - array_walk_recursive($value, static function (mixed $item) use (&$items): void { - $items[] = $item; - }); - } + $items = gridQueryCsvInput($query, $key); $result = []; $seen = []; diff --git a/tests/Unit/Support/CryptoTest.php b/tests/Unit/Support/CryptoTest.php index e2708cb..9579499 100644 --- a/tests/Unit/Support/CryptoTest.php +++ b/tests/Unit/Support/CryptoTest.php @@ -67,4 +67,58 @@ final class CryptoTest extends TestCase $this->expectExceptionMessage('malformed'); Crypto::decryptString($encrypted); } + + public function testDecryptThrowsOnEmptyString(): void + { + $this->expectException(\RuntimeException::class); + Crypto::decryptString(''); + } + + public function testDecryptThrowsOnMissingPayloadFields(): void + { + // Valid base64 wrapping valid JSON, but missing required fields. + $payload = base64_encode(json_encode(['v' => 1])); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('malformed'); + Crypto::decryptString($payload); + } + + public function testRoundTripWithSpecialCharacters(): void + { + $values = [ + 'unicode: Ünterström ñ 日本語', + "newlines:\nand\ttabs", + 'json-like: {"key": "value"}', + str_repeat('x', 10000), + ]; + + foreach ($values as $plaintext) { + $encrypted = Crypto::encryptString($plaintext); + self::assertSame($plaintext, Crypto::decryptString($encrypted), "Round-trip failed for: " . substr($plaintext, 0, 40)); + } + } + + public function testRoundTripWithBinaryLikeContent(): void + { + $plaintext = "null-bytes:\x00\x01\x02 and high-bytes:\xFE\xFF"; + $encrypted = Crypto::encryptString($plaintext); + + self::assertSame($plaintext, Crypto::decryptString($encrypted)); + } + + public function testDecryptThrowsOnNonBase64Input(): void + { + $this->expectException(\RuntimeException::class); + Crypto::decryptString('not!valid@base64###'); + } + + public function testDecryptThrowsOnTruncatedPayload(): void + { + $encrypted = Crypto::encryptString('secret'); + $truncated = substr($encrypted, 0, (int) (strlen($encrypted) / 2)); + + $this->expectException(\RuntimeException::class); + Crypto::decryptString($truncated); + } } diff --git a/tests/Unit/Support/SearchQueryNormalizerTest.php b/tests/Unit/Support/SearchQueryNormalizerTest.php new file mode 100644 index 0000000..60d2e52 --- /dev/null +++ b/tests/Unit/Support/SearchQueryNormalizerTest.php @@ -0,0 +1,137 @@ + { const targetSelector = toggle.dataset.colorTarget || ''; @@ -29,54 +30,39 @@ const syncToggle = (toggle, scope) => { } }; -export function initColorDefaultToggle(root = document, options = {}) { - const host = resolveHost(root); - const selector = String(options.selector || '[data-color-default-toggle]').trim() || '[data-color-default-toggle]'; - const toggles = Array.from(host.querySelectorAll(selector)).filter( - (toggle) => toggle instanceof HTMLInputElement - ); - if (!toggles.length) { - return { destroy: () => {} }; +const initRoot = (toggle) => { + if (!(toggle instanceof HTMLInputElement)) { + return null; } - const cleanupFns = []; - toggles.forEach((toggle) => { - if (toggle.dataset.colorDefaultToggleBound === '1') { - return; - } - toggle.dataset.colorDefaultToggleBound = '1'; + 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 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)]; - const onToggleChange = () => syncToggle(toggle, scope); - toggle.addEventListener('change', onToggleChange); - cleanupFns.push(() => 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)); + } - if (target instanceof HTMLInputElement) { - const onTargetInput = () => { - if (toggle.checked) { - toggle.checked = false; - target.removeAttribute('disabled'); - } - }; - target.addEventListener('input', onTargetInput); - cleanupFns.push(() => target.removeEventListener('input', onTargetInput)); - } + syncToggle(toggle, scope); + return () => cleanups.forEach((fn) => fn()); +}; - syncToggle(toggle, scope); - cleanupFns.push(() => { - delete toggle.dataset.colorDefaultToggleBound; - }); - }); - - const destroy = () => { - cleanupFns.forEach((cleanup) => cleanup()); - }; - - return { destroy }; -} +export const initColorDefaultToggle = createConditionalToggleInit({ + rootSelector: '[data-color-default-toggle]', + boundKey: 'colorDefaultToggleBound', + initRoot, +}); diff --git a/web/js/components/app-custom-field-options-toggle.js b/web/js/components/app-custom-field-options-toggle.js index e4dbdf3..e12575a 100644 --- a/web/js/components/app-custom-field-options-toggle.js +++ b/web/js/components/app-custom-field-options-toggle.js @@ -1,35 +1,12 @@ /** * Shows/hides custom field option list based on field type selection. */ -import { warnOnce, resolveHost } from '../core/app-dom.js'; +import { warnOnce } from '../core/app-dom.js'; +import { syncControlState, createConditionalToggleInit } from '../core/app-conditional-controls.js'; const TYPES_WITH_OPTIONS = new Set(['select', 'multiselect']); const FILTERABLE_TYPES = new Set(['select', 'multiselect', 'boolean', 'date']); -const syncOptionsVisibility = (source, target) => { - const selectedType = String(source.value || '').trim().toLowerCase(); - const showOptions = TYPES_WITH_OPTIONS.has(selectedType); - - target.hidden = !showOptions; - target.querySelectorAll('textarea, input, select').forEach((control) => { - if (!control.dataset.initialDisabled) { - control.dataset.initialDisabled = control.hasAttribute('disabled') ? '1' : '0'; - } - - if (!showOptions) { - control.setAttribute('disabled', 'disabled'); - return; - } - - if (control.dataset.initialDisabled === '1') { - control.setAttribute('disabled', 'disabled'); - return; - } - - control.removeAttribute('disabled'); - }); -}; - const syncFilterableAvailability = (source, target) => { const selectedType = String(source.value || '').trim().toLowerCase(); const filterableAllowed = FILTERABLE_TYPES.has(selectedType); @@ -62,63 +39,52 @@ const syncFilterableAvailability = (source, target) => { checkbox.removeAttribute('disabled'); }; -export function initCustomFieldOptionsToggle(root = document, options = {}) { - const host = resolveHost(root); - const sourceSelector = String(options.sourceSelector || '[data-custom-field-type-source]').trim() || '[data-custom-field-type-source]'; - const sources = Array.from(host.querySelectorAll(sourceSelector)); - if (!sources.length) { - return { destroy: () => {} }; +const initRoot = (source) => { + if (!(source instanceof HTMLSelectElement)) { + return null; } - const cleanupFns = []; - sources.forEach((source) => { - if (!(source instanceof HTMLSelectElement) || source.dataset.customFieldOptionsBound === '1') { - return; - } - source.dataset.customFieldOptionsBound = '1'; + const group = String(source.dataset.customFieldGroup || '').trim(); + if (!group) { + warnOnce('UI_EL_MISSING', 'Missing data-custom-field-group', { module: 'custom-field-options-toggle' }); + return null; + } - const group = String(source.dataset.customFieldGroup || '').trim(); - if (!group) { - warnOnce('UI_EL_MISSING', 'Missing data-custom-field-group', { module: 'custom-field-options-toggle' }); - return; - } - - const target = document.querySelector( - `[data-custom-field-options-target][data-custom-field-group="${group}"]` - ); - if (!(target instanceof HTMLElement)) { - warnOnce('UI_EL_MISSING', `Missing custom field options target for group: ${group}`, { - module: 'custom-field-options-toggle', - }); - return; - } - - const filterableTarget = document.querySelector( - `[data-custom-field-filterable-target][data-custom-field-group="${group}"]` - ); - if (!(filterableTarget instanceof HTMLElement)) { - warnOnce('UI_EL_MISSING', `Missing custom field filterable target for group: ${group}`, { - module: 'custom-field-options-toggle', - }); - return; - } - - const onChange = () => { - syncOptionsVisibility(source, target); - syncFilterableAvailability(source, filterableTarget); - }; - source.addEventListener('change', onChange); - cleanupFns.push(() => { - source.removeEventListener('change', onChange); - delete source.dataset.customFieldOptionsBound; + const target = document.querySelector( + `[data-custom-field-options-target][data-custom-field-group="${group}"]` + ); + if (!(target instanceof HTMLElement)) { + warnOnce('UI_EL_MISSING', `Missing custom field options target for group: ${group}`, { + module: 'custom-field-options-toggle', }); + return null; + } - onChange(); - }); + const filterableTarget = document.querySelector( + `[data-custom-field-filterable-target][data-custom-field-group="${group}"]` + ); + if (!(filterableTarget instanceof HTMLElement)) { + warnOnce('UI_EL_MISSING', `Missing custom field filterable target for group: ${group}`, { + module: 'custom-field-options-toggle', + }); + return null; + } - const destroy = () => { - cleanupFns.forEach((cleanup) => cleanup()); + const onChange = () => { + const selectedType = String(source.value || '').trim().toLowerCase(); + syncControlState(target, TYPES_WITH_OPTIONS.has(selectedType)); + syncFilterableAvailability(source, filterableTarget); }; + source.addEventListener('change', onChange); + onChange(); - return { destroy }; -} + return () => { + source.removeEventListener('change', onChange); + }; +}; + +export const initCustomFieldOptionsToggle = createConditionalToggleInit({ + rootSelector: '[data-custom-field-type-source]', + boundKey: 'customFieldOptionsBound', + initRoot, +}); diff --git a/web/js/components/app-settings-telemetry.js b/web/js/components/app-settings-telemetry.js index 6d9a4d4..3186f1a 100644 --- a/web/js/components/app-settings-telemetry.js +++ b/web/js/components/app-settings-telemetry.js @@ -2,17 +2,17 @@ * Tracks settings page interactions for telemetry. */ import { resolveHost } from '../core/app-dom.js'; +import { syncControlState } from '../core/app-conditional-controls.js'; + export const initTelemetrySettings = (root = document, options = {}) => { const host = resolveHost(root); const toggleSelector = String(options.toggleSelector || '[data-telemetry-enabled-toggle]').trim() || '[data-telemetry-enabled-toggle]'; const samplingFieldsetSelector = String(options.samplingFieldsetSelector || '[data-telemetry-sampling-fieldset]').trim() || '[data-telemetry-sampling-fieldset]'; const samplingRowSelector = String(options.samplingRowSelector || '[data-telemetry-sampling-row]').trim() || '[data-telemetry-sampling-row]'; - const samplingSelectSelector = String(options.samplingSelectSelector || '[data-telemetry-sampling-select]').trim() || '[data-telemetry-sampling-select]'; const toggle = host.querySelector(toggleSelector); const samplingFieldset = host.querySelector(samplingFieldsetSelector); const samplingRow = host.querySelector(samplingRowSelector); - const samplingSelect = host.querySelector(samplingSelectSelector); if (!toggle || !samplingFieldset) { return { destroy: () => {} }; @@ -20,13 +20,10 @@ export const initTelemetrySettings = (root = document, options = {}) => { const syncSamplingVisibility = () => { const active = Boolean(toggle.checked); - samplingFieldset.hidden = !active; - if (samplingRow) { + syncControlState(samplingFieldset, active); + if (samplingRow instanceof HTMLElement) { samplingRow.hidden = !active; } - if (samplingSelect) { - samplingSelect.disabled = !active || toggle.disabled; - } }; toggle.addEventListener('change', syncSamplingVisibility);