Merged PR 2007: fix(filter-service, number-range-filter-input): resolve NumberRange state man...

fix(filter-service, number-range-filter-input): resolve NumberRange state management and reference issues

**Root Cause:**
Filter service was experiencing reference sharing between current state and
committed state due to shallow copying in commit(), causing filters to
incorrectly appear as "changed" when mixing NumberRange with other filter
types like Checkbox.

**Changes Made:**

1. **State Management (filter.service.ts):**
   - Use structuredClone() in commit() for deep copies to prevent reference sharing
   - Update clear() to preserve structural properties (options array references)
   - Refactor resetInput() to selectively copy only mutable properties while
     preserving structural ones for isEqual() comparisons
   - Simplify selectedFilterCount to use isDefaultFilterInput() consistently

2. **Default Values (number-range-filter-input.mapping.ts):**
   - Parse minValue/maxValue from config (e.g., "1-" → 1) as defaults
   - Use parsed defaults as initial min/max when no explicit value provided

**Impact:**
- NumberRange filters correctly display default values in UI
- Filters no longer incorrectly show as "changed" after multiple commits
- "Standardeinstellungen" works correctly when mixing NumberRange with other types
- selectedFilterCount accurately reflects changed filters including NumberRange

Ref: #5402
This commit is contained in:
Nino Righi
2025-11-05 20:17:52 +00:00
committed by Lorenz Hilpert
parent b8e2d3f87b
commit a6f0aaf1cc
2 changed files with 95 additions and 32 deletions

View File

@@ -480,9 +480,8 @@ export class FilterService {
/**
* Computes the number of filter inputs that are currently selected (i.e., not in their default state).
*
* - For checkbox inputs, increments the count if any values are selected.
* - For date range inputs, increments the count if either start or stop is set.
* - Inputs in their default state are not counted.
* - For all input types, increments the count if the input differs from its default state.
* - Uses isDefaultFilterInput for accurate comparison including default values.
*
* @remarks
* This property is a computed signal and updates automatically when the filter state changes.
@@ -492,18 +491,11 @@ export class FilterService {
selectedFilterCount = computed<number>(() => {
const currentState = getState(this.#state);
return currentState.inputs.reduce((count, input) => {
if (this.isDefaultFilterInput(input)) {
return count;
}
if (input.type === InputType.Checkbox && input.selected?.length) {
// Simply check if input differs from default state
// This handles all cases including NumberRange with default values
if (!this.isDefaultFilterInput(input)) {
return count + 1;
}
if (input.type === InputType.DateRange && (input.start || input.stop)) {
return count + 1;
}
return count;
}, 0);
});
@@ -651,13 +643,18 @@ export class FilterService {
* Commits the current state by capturing a snapshot of the internal state.
* This method updates the private `#commitdState` property with the current state
* and triggers any registered commit callbacks.
*
* @remarks
* Uses structuredClone to create a deep copy of the state, preventing reference sharing
* between the current state and the committed state.
*/
commit(): void {
const currentState = getState(this.#state);
const committedState = this.#commitdState();
if (!isEqual(currentState, committedState)) {
this.#commitdState.set(currentState);
// Deep clone to prevent reference sharing
this.#commitdState.set(structuredClone(currentState));
this.#logger.debug('Filter state committed', () => ({
changes: true,
}));
@@ -675,7 +672,7 @@ export class FilterService {
/**
* Clears all filter values without resetting to default values.
* This sets text inputs to undefined, checkbox selections to empty arrays,
* and date ranges to undefined for both start and stop.
* date ranges to undefined for both start and stop, and number ranges to undefined for both min and max.
*
* @param options - Optional parameters
* @param options.commit - If true, commits the changes immediately after clearing
@@ -695,6 +692,10 @@ export class FilterService {
return !!input.start || !!input.stop;
}
if (input.type === InputType.NumberRange) {
return input.min != null || input.max != null;
}
return false;
});
@@ -707,18 +708,31 @@ export class FilterService {
}
const inputs = this.#state.inputs().map((input) => {
// Get the default input to preserve structural properties (like options array reference)
const defaultInput = this.defaultState.inputs.find(
(i) => i.key === input.key,
);
if (input.type === InputType.Text) {
return { ...input, value: undefined };
}
if (input.type === InputType.Checkbox) {
return { ...input, selected: [] };
if (
input.type === InputType.Checkbox &&
defaultInput?.type === InputType.Checkbox
) {
// Preserve the options array reference from defaultState
return { ...input, options: defaultInput.options, selected: [] };
}
if (input.type === InputType.DateRange) {
return { ...input, start: undefined, stop: undefined };
}
if (input.type === InputType.NumberRange) {
return { ...input, min: undefined, max: undefined };
}
return input;
});
@@ -755,6 +769,8 @@ export class FilterService {
* @remarks
* - If no input is found with the specified key, a warning is logged for that key.
* - The method updates the state by replacing the input(s) with their default configuration.
* - For checkbox inputs, keeps the original options array to maintain reference equality.
* - For other input types, creates a shallow copy with reset values.
*
* @example
* ```typescript
@@ -766,24 +782,27 @@ export class FilterService {
* ```
*/
resetInput(keys: string[], options?: { commit: boolean }): void {
// Use a more lightweight approach than structuredClone
const defaultFilter = { ...this.defaultState };
// Find all default inputs that match the provided keys
const inputsToReset = keys
.map((key) => {
const inputToReset = defaultFilter.inputs.find((i) => i.key === key);
const inputToReset = this.defaultState.inputs.find(
(i) => i.key === key,
);
if (!inputToReset) {
this.#logger.warn(`No input found with key`, () => ({
key,
method: 'resetInput',
}));
return undefined;
}
return { key, defaultInput: inputToReset };
})
.filter((item) => item.defaultInput !== undefined);
.filter(
(item): item is { key: string; defaultInput: FilterInput } =>
item !== undefined,
);
if (inputsToReset.length === 0) {
return;
@@ -797,8 +816,40 @@ export class FilterService {
return input;
}
const resetItem = inputsToReset.find((item) => item.key === input.key);
return resetItem?.defaultInput ?? input;
const defaultInput = inputsToReset.find(
(item) => item.key === input.key,
)?.defaultInput;
if (!defaultInput) {
return input;
}
// Reset only the mutable state while preserving structural properties (like options array reference)
// This ensures reference equality for isEqual comparisons in isDefaultFilterInput
switch (input.type) {
case InputType.Checkbox:
return defaultInput.type === InputType.Checkbox
? { ...input, selected: [...defaultInput.selected] }
: input;
case InputType.DateRange:
return defaultInput.type === InputType.DateRange
? { ...input, start: defaultInput.start, stop: defaultInput.stop }
: input;
case InputType.NumberRange:
return defaultInput.type === InputType.NumberRange
? { ...input, min: defaultInput.min, max: defaultInput.max }
: input;
case InputType.Text:
return defaultInput.type === InputType.Text
? { ...input, value: defaultInput.value }
: input;
default:
return input;
}
});
patchState(this.#state, {

View File

@@ -8,6 +8,20 @@ export function numberRangeFilterInputMapping(
group: string,
input: Input,
): NumberRangeFilterInput {
// Extract minValue and maxValue, removing trailing dashes
const minValueRaw = input.options?.values?.[0]?.minValue;
const maxValueRaw = input.options?.values?.[0]?.maxValue;
// Parse minValue: "1-" becomes 1, undefined stays undefined
const minDefault = minValueRaw
? Number(minValueRaw.replace(/-/g, ''))
: undefined;
// Parse maxValue: "-100" becomes 100, undefined stays undefined
const maxDefault = maxValueRaw
? Number(maxValueRaw.replace(/-/g, ''))
: undefined;
return NumberRangeFilterInputSchema.parse({
group,
key: input.key,
@@ -15,18 +29,16 @@ export function numberRangeFilterInputMapping(
description: input.description,
type: InputType.NumberRange,
target: input.target,
// Use value if present (for pre-filled values), otherwise use minValue as default
min: input.options?.values?.[0]?.value
? Number(input.options?.values?.[0]?.value)
: undefined,
: minDefault,
max: input.options?.values?.[1]?.value
? Number(input.options?.values?.[1]?.value)
: undefined,
minValue: input.options?.values?.[0]?.minValue
? Number(input.options?.values?.[0]?.minValue.replace('-', ''))
: undefined,
maxValue: input.options?.values?.[0]?.maxValue
? Number(input.options?.values?.[0]?.maxValue.replace('-', ''))
: undefined,
: maxDefault,
// Store the constraints for validation
minValue: minDefault,
maxValue: maxDefault,
minLabel: input.options?.values?.[0]?.label || 'Min',
maxLabel: input.options?.values?.[1]?.label || 'Max',
});