fix(native-filters): persist created/pasted default values in select filter#40984
Conversation
dosubot
Bot
added
the
dashboard:native-filters
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #40984 +/- ##
=======================================
Coverage 64.55% 64.55%
=======================================
Files 2673 2673
Lines 147634 147647 +13
Branches 34090 34097 +7
=======================================
+ Hits 95307 95318 +11
- Misses 50595 50597 +2
Partials 1732 1732
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #5cfb3b
Actionable Suggestions - 1
-
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx - 1
- Paste handling inconsistency · Line 693-695
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx - 1
- CWE-710: Missing Hook Dependency · Line 344-344
Review Details
-
Files reviewed - 3 · Commit Range:
9667ae5..f03ff98- superset-frontend/packages/superset-ui-core/src/components/Select/Select.test.tsx
- superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
- superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #fc7932Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Code Review Agent Run #06e0bcActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
Thanks for the fix — the three-part approach (trim pasted tokens, surface typed values as creatable options for multi-select, and retain created values as chips) is solid, and reusing the existing Type coercion in the created-value dedupIn the new block in the if (creatable !== false && filterState.value) {
const existing = new Set(allOptions);
ensureIsArray(filterState.value)
.filter(v => v !== null && v !== undefined && !existing.has(v))
.forEach(v => {
baseOptions.push({ label: String(v), value: String(v), isNewOption: true });
existing.add(v);
});
}The dedup uses a strict
Suggestion: dedup with the same loose helper this file already uses for the typed-search case, and preserve the original value type: ensureIsArray(filterState.value)
.filter(v => v != null && !hasOption(v, baseOptions, true))
.forEach(v => {
baseOptions.push({ label: String(v), value: v, isNewOption: true });
});Minor / non-blockingIt'd be good to add plugin-level tests for the new behaviors — multi-select typed creatable option display, Otherwise looks good 👍 |
Code Review Agent Run #207094Actionable Suggestions - 0Additional Suggestions - 2
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
254cad0 to
2250cf9
Compare
Three bugs prevented default values typed/pasted into a creatable native filter from being stored and rendered correctly: 1. Paste tokenization did not trim whitespace, so "10107, 10121" split into ["10107", " 10121"] — the leading space caused label mismatch and stored wrong option values. 2. The options useMemo in SelectFilterPlugin guarded new-option injection with !multiSelect, so typed search terms never appeared as selectable options in multi-select mode. Also fixed the mutation bug (unshift on a memoized array). 3. uniqueOptions excluded values from filterState.value that are not in the dataset (i.e., user-created values). Added them back when creatable !== false so the Select can display them correctly. Adds a test for the whitespace-trimming fix.
The options useMemo was missing the `creatable !== false` check after the multi-select guard was removed, allowing new option entries to appear in the dropdown even when creatable=false. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ecks Auto-formatted by prettier: wrapped long lines in Select.tsx and SelectFilterPlugin.tsx to satisfy the prettier pre-commit hook.
- Add !searchAllOptions guard to options useMemo so new-option injection matches the allowNewOptions prop (consistent behavior in remote-search mode) - Apply paste trim/filter fix to AsyncSelect.tsx for consistency with the same fix already in Select.tsx Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…niqueOptions Per kgabryje's review: use hasOption() for loose matching instead of Set.has() to avoid string/number duplicate options, and preserve the original value type (value: v, not String(v)) so numeric filters don't flip type when creatable defaults are restored. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…syncSelect paste trim - SelectFilterPlugin: multi-select creatable shows create option, searchAllOptions suppresses it, filterState created values render as chips, no duplicate chip when value already in dataset - AsyncSelect: mirror Select.test.tsx whitespace-trim test for paste tokenizer Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2250cf9 to
943c0a1
Compare
Code Review Agent Run #4ce338Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
…etWrapper scope Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rusackas
left a comment
rusackas
left a comment
There was a problem hiding this comment.
LGTM — confirmed both plugin bugs on master (the unshift into a memoized array and the !multiSelect guard), and the fold-back of filterState.value dedupes correctly via hasOption's loose comparison, so dropdown selection is unaffected. Test coverage on the create/paste/searchAllOptions paths is solid.
Tiny note: the PR body still describes a Select.tsx change that's already on master, so the description is a touch stale... cosmetic only.
4 participants
Summary
Fixes three compounding bugs that prevented native filter default values from persisting when typed or pasted (rather than selected from the dropdown) in a creatable multi-select filter.
Bug: A Value (
filter_select) native filter with Allow creation of new values enabled silently dropped default values that were typed or pasted. The chips appeared in the config modal but after clicking Save the filter rendered empty on the dashboard, and reopening the config showed no default value.Fix 1 —
Select.tsx: trim whitespace from pasted tokens (primary fix)"10107, 10121".split(",")previously produced["10107", " 10121"]— the leading space caused label-match failure against the existing option{value: 10107, label: "10107"}, resulting in the value being stored as a new string literal" 10121"rather than the correct matched option value.Fix 2 —
SelectFilterPlugin.tsx: multi-select typed values in options (secondary fix)!multiSelectguard prevented typed search terms from appearing as selectable new options in multi-select filters. Also fixed a mutation bug whereunshiftwas called on the memoizeduniqueOptionsarray.Fix 3 —
SelectFilterPlugin.tsx: include created values inuniqueOptions(tertiary fix)uniqueOptionswas built only from DB data. WhenfilterState.valuecontained created values not in the dataset, they were absent fromoptionsand relied on the Select component's internalfullSelectOptionsfallback which could lose them across re-renders.Test plan
order_numbercolumn — paste10107, 10121, 10134, 10145, 10159, 10168, 10180, 10188, 10201, 10211as default value, Save, verify filter chips appear on dashboardjest Select.test.tsx— new test'trims whitespace from pasted comma-separated values'should pass🤖 Generated with Claude Code