Skip to content

Keep commas in search filter values - #4496

Open
justadityaraj wants to merge 2 commits into
umami-software:devfrom
justadityaraj:fix/4489-preserve-regex-commas
Open

Keep commas in search filter values#4496
justadityaraj wants to merge 2 commits into
umami-software:devfrom
justadityaraj:fix/4489-preserve-regex-commas

Conversation

@justadityaraj

@justadityaraj justadityaraj commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Problem

Editing a search filter initialized every string value by splitting on commas. Regex quantifiers such as {21,22} were therefore truncated in the input even though the URL parser had preserved the full expression.

Fix

Keep search-operator values as one string when initializing FilterRecord; retain comma splitting for the existing multi-value operators. Add a component regression using the reported regex shape.

User impact

Regex, contains, and other free-text filter values containing commas remain fully visible and editable instead of showing only the prefix before the first comma.

Verification

  • Focused regressions: 2 passed, including an operator transition that preserves the full comma-bearing value.
  • TypeScript: tsc --noEmit passed after generating the Prisma client with a placeholder local database URL.
  • Biome check on both changed files passed.
  • Full Vitest run: 93 files and 732 tests passed; 4 unrelated suites could not load the repository's PostCSS plugin on this Windows environment.
  • Full repository lint reaches existing diagnostics outside this diff; both changed files are clean.
  • git diff --check passed.

Fixes #4489


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@vercel

vercel Bot commented Aug 28, 2026

Copy link
Copy Markdown

@justadityaraj is attempting to deploy a commit to the Umami Software Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR preserves commas in scalar search-filter values and adds a regression test for regex quantifiers. The initialization fix is incomplete when an already-mounted filter changes operators.

  • Wraps scalar search-operator values in a single-element selection instead of splitting on commas
  • Retains comma splitting for multi-value operators
  • Adds a component regression covering ^[a-zA-Z0-9]{21,22}$

Confidence Score: 4/5

The operator-transition truncation should be fixed before merging because users can still lose the comma-containing portion of an edited filter.

FilterRecord derives its local selection from the operator only during initial mounting, while its parent preserves the component instance when operators change, leaving the reported truncation reachable through the editor.

Files Needing Attention: src/components/common/FilterRecord.tsx

Important Files Changed

Filename Overview
src/components/common/FilterRecord.tsx Preserves comma-containing search values on initial mount, but local selection state is not recalculated when the operator changes.
src/components/common/FilterRecord.test.tsx Adds focused coverage for initial rendering of a regex containing a comma, but does not cover an in-place operator transition.

Reviews (1): Last reviewed commit: "fix(filters): preserve commas in search ..." | Re-trigger Greptile

Comment thread src/components/common/FilterRecord.tsx Outdated
Comment on lines 46 to 53
const initValues = Array.isArray(value)
? value
: value
? isSearch
? [value]
: value.split(',')
: [];
const [selected, setSelected] = useState<string[]>(initValues);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Operator changes retain split state

When a comma-containing multi-value filter is changed to a search operator, the stable component instance retains its initially split selected state, so the search field displays only the prefix before the comma and subsequent editing can overwrite the complete value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8928c5f. The selected state now resynchronizes from the full value whenever the operator changes; the regression covers eq to re with a comma-bearing quantifier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant