Conversation
WalkthroughAdds a multi-row filter editor, new contains-operator encoding, expanded filter keys and panel order, backend endpoints/services to fetch browser/OS version options, a medium modal size, translations, and minor UI styling tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Browser UI
participant Frontend as Web App
participant API as Frontend API wrapper
participant Controller as Analytics Controller
participant Service as Analytics Service
participant DB as ClickHouse
Client->>Frontend: Open SearchFilters modal / request versions for column
Frontend->>API: getVersionFilters(pid, type, column, password?)
API->>Controller: GET /analytics/filters/versions (validated DTO)
Controller->>Service: analyticsService.getVersionFilters(pid, type, column)
Service->>DB: SELECT DISTINCT name, version FROM <table> WHERE pid = $pid AND column IS NOT NULL AND version IS NOT NULL
DB-->>Service: rows {name, version}
Service-->>Controller: [{name, version}, ...]
Controller-->>API: 200 OK with version list
API-->>Frontend: version list (frontend builds "Name|||Version" entries)
Frontend-->>Client: show combined version suggestions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
web/app/ui/VirtualizedSelect.tsx (2)
166-169: Index passed to extractors refers tofilteredItems, not originalitems.When search filtering is active,
virtualItem.indexis the index withinfilteredItems, not the originalitemsarray. If extractors depend on the original item index, this could cause inconsistencies.If this is intentional (extractors typically use the item value, not index), consider documenting this behavior in the interface comments. Otherwise, you may need to track original indices.
122-136: Consider addingaria-labelto the search input for accessibility.The search input has a placeholder but lacks an explicit
aria-label, which would improve screen reader experience.<input type='text' className='w-full rounded-md border border-gray-300 bg-white px-3 py-1.5 text-sm focus:border-indigo-500 focus:ring-1 focus:ring-indigo-500 focus:outline-hidden dark:border-gray-700 dark:bg-slate-700 dark:text-gray-50' placeholder={t('common.search') || 'Search...'} + aria-label={t('common.search') || 'Search'} value={query} onChange={(e) => setQuery(e.target.value)}web/app/pages/Project/View/components/SearchFilters.tsx (2)
168-177: Country/OS/browser/device icon logic is solid; minor duplication with existingdeviceIconMappinghelperThe per‑column icon handling in
getIconForItemis well‑structured and handles:
- Countries →
Flag+ i18n name- Browsers/OS → logo maps with dark theme fallback
- Devices → local
deviceIconMappingwith a question‑mark fallbackFunctionally this looks good.
There is a small duplication with
deviceIconMappingalready defined inViewProject.helpers.tsx(same keys, different size classes). If you want to reduce drift, consider centralizing the mapping and passing aclassNameorsizeprop from here instead of redefining it.Also applies to: 179-275
460-485: Filter values prefetching is robust; small risk of double‑fetch and some avoidable effect churnThe caching + loading‑set approach is solid overall:
filterValuesCache[column]andloadingColumnsprevent most duplicate fetches.- Pre‑fetch on modal open plus targeted fetch on column change is a good UX optimization.
- State updates use functional setters, which avoids stale closure issues.
Two minor points you may want to tighten:
Potential double fetch for the same column
Because React state updates are async, two quick successive calls tofetchFilterValues(column)(e.g. from the prefetch effect and a row’supdateFilterRow) can both seeloadingColumns.has(column) === falseand both start a request. Not critical, but you can harden this by tracking an “in‑flight” set in a ref or by narrowing call sites (e.g., skipping prefetch for the just‑changed column inupdateFilterRow).Effect dependency noise on
panelOptionsand caches
panelOptionsis recomputed on every render and then used directly as a dependency, which forces the prefetch effect to re‑run each render (even though theshowModalguard prevents extra work). Wrapping it inuseMemowould make the effect dependencies cleaner:
- const panelOptions = type === 'traffic' ? FILTERS_PANELS_ORDER : ERRORS_FILTERS_PANELS_ORDER
- const panelOptions = useMemo(
- () => (type === 'traffic' ? FILTERS_PANELS_ORDER : ERRORS_FILTERS_PANELS_ORDER),
- [type],
- )
These are non‑blocking, but worth considering.
Also applies to: 487-498, 524-538
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
web/app/lib/constants/index.ts(1 hunks)web/app/pages/Project/View/components/SearchFilters.tsx(4 hunks)web/app/pages/Project/View/utils/filters.tsx(1 hunks)web/app/ui/Modal.tsx(2 hunks)web/app/ui/VirtualizedSelect.tsx(1 hunks)web/public/locales/en.json(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
web/app/pages/Project/View/components/SearchFilters.tsx (3)
web/app/pages/Project/View/ViewProject.helpers.tsx (1)
deviceIconMapping(1646-1654)web/app/lib/constants/index.ts (5)
BROWSER_LOGO_MAP(929-976)OS_LOGO_MAP(978-1004)OS_LOGO_MAP_DARK(1006-1011)FILTERS_PANELS_ORDER(253-273)ERRORS_FILTERS_PANELS_ORDER(274-274)web/app/api/index.ts (2)
getErrorsFilters(1422-1432)getFilters(1410-1420)
🪛 GitHub Actions: Pull Request Validation Workflow
web/app/ui/VirtualizedSelect.tsx
[error] 1-1: Unused files detected by knip: 'app/ui/VirtualizedSelect.tsx'. Process exited with code 1.
🔇 Additional comments (9)
web/app/ui/Modal.tsx (1)
28-28: LGTM!The addition of the
'medium'size option is consistent with the existing size patterns. The CSS classes'w-[90vw] max-w-2xl md:w-full'appropriately position it between'regular'(max-w-lg) and'large'(max-w-5xl).Also applies to: 69-69
web/app/pages/Project/View/utils/filters.tsx (1)
37-38: LGTM!The addition of
'te'(UTM Term) and'co'(UTM Content) tovalidFiltersis consistent with the updatedFILTERS_PANELS_ORDERin constants and the mapping translations inen.json.web/public/locales/en.json (1)
144-144: LGTM!The new translation keys are well-structured and use correct interpolation syntax (
{{value}}). They align properly with the enhanced filter UI components.Also applies to: 943-949
web/app/lib/constants/index.ts (1)
253-274: The omission of 'refn' from FILTERS_PANELS_ORDER is intentional and correct.'refn' is a valid filter used internally in the system (backend special handling in analytics services, conditional filtering logic in ViewProject.tsx), but it is not exposed as a user-selectable filter panel option. The UI uses 'ref' instead in the traffic sources tabs, and 'refn' is only applied programmatically when transitioning between grouped and filtered views. This is a deliberate design choice, not an oversight.
web/app/pages/Project/View/components/SearchFilters.tsx (5)
87-137: URL map key now depends onisContains— behavior looks correct but needs consistency check with other filter buildersYou now include
isContainsin both the dedupe map key and the URL key prefix (~vs^) which is necessary to distinguish “contains” vs “not contains” filters:
- Map key:
${f.isExclusive ? '!' : ''}${f.isContains ? '~' : ''}${f.column}=${f.filter}- URL key: plain /
!/~/^depending onisExclusive/isContains.This is logically consistent and should avoid collisions between
is,isNot,contains, andnotContainsfilters, and the preservation loop correctly strips all!/~/^prefixes beforeisFilterValidchecks.Please double‑check that:
FilterTypealways setsisContains(including in places where filters are parsed from URL/search params), and- Any other code that constructs filter URL params uses the same prefix scheme.
285-291: Virtualized dropdown implementation looks correct and efficientThe
useVirtualizersetup and rendering are wired properly:
countderives fromfilteredItems.length.getScrollElementtargets the scrollable container vialistRef.- Each option uses
virtualItem.startandvirtualItem.sizefor absolute positioning and height.getLabelForItem/getIconForItemare reused for display.This should scale well for large lists without layout glitches.
Also applies to: 387-427
552-580: Filter row →FilterTypeconversion andisContainsmapping look correctThe
onSubmitpipeline:
- Filters out incomplete rows (
row.column && row.value).- Uses
operatorToFilterto derive{ isExclusive, isContains }from the operator.- Normalizes country values to alpha‑2 codes when possible.
- Calls
getFiltersUrlParamswithoverride=true, replacing existing URL filters atomically.This is coherent with the new operator model and seems to produce the expected
FilterTypeobjects for downstream use.
580-589: Submit enablement and Clear‑all logic are good, but consider reusinghasValidFiltersafterValueInputfix
hasValidFilterscorrectly disables submit unless at least one row has both a column and a (non‑empty) value, and the Clear‑all button is only shown when it’s meaningful.Once you sync
ValueInputchanges torow.value, these controls will accurately track what the user sees. No change required beyond theValueInputfix, just calling out that the wiring is otherwise sound.Also applies to: 745-765
767-809: Applied filters summary is well‑designed and leverages the new operator semanticsThe applied‑filters chips:
- Localize column names via
tnMapping/project.mapping.- Show country names for
ccviacountries.getName.- Compute operator labels directly from
isExclusive/isContains.- Use different color accents for each operator combination.
This provides a clear, compact summary of active filters and correctly reflects the expanded operator set.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
web/app/pages/Project/View/components/SearchFilters.tsx (3)
330-333:ValueInputdoes not sync user edits back torow.value, causing stale filter values to be applied
handleInputChangeonly updates the localinputValuestate; it never callsonChange. As a result,row.valueremains whatever was last committed (via option click / Enter / "Use this value" button). This creates UX bugs:
- If a user clears an existing value and applies filters without hitting Enter or selecting a suggestion, the UI shows an empty field but the old filter still gets submitted.
submitDisabledcan remaintruebecauserow.valueis non‑empty even though the visible input is empty, or vice versa.Apply this diff to keep parent state in sync:
const handleInputChange = (e: React.ChangeEvent<HTMLInputElement>) => { const newValue = e.target.value setInputValue(newValue) + onChange(newValue) }
789-812: FixListboxOptionclassName prop to useactiveinstead offocusHeadless UI's
ListboxOptionrender prop exposesactive(andselected,disabled), notfocus. Usingfocusmeans the active background style never applies.Apply this diff:
<ListboxOption key={option} value={option} - className={({ focus }) => + className={({ active }) => cx('relative cursor-pointer py-2 pr-4 pl-3 select-none', { - 'bg-gray-100 dark:bg-slate-700': focus, - 'text-gray-700 dark:text-gray-50': !focus, + 'bg-gray-100 dark:bg-slate-700': active, + 'text-gray-700 dark:text-gray-50': !active, }) } >
847-869: FixListboxOptionclassName prop to useactiveinstead offocusSame issue as the column select: Headless UI's
ListboxOptionrender prop exposesactive, notfocus. The hover/active styling won't work correctly.Apply this diff:
<ListboxOption key={op.value} value={op.value} - className={({ focus }) => + className={({ active }) => cx('relative cursor-pointer py-2 pr-4 pl-8 select-none', { - 'bg-gray-100 dark:bg-slate-700': focus, - 'text-gray-700 dark:text-gray-50': !focus, + 'bg-gray-100 dark:bg-slate-700': active, + 'text-gray-700 dark:text-gray-50': !active, }) } >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/app/pages/Project/View/components/RefreshStatsButton.tsx(1 hunks)web/app/pages/Project/View/components/SearchFilters.tsx(4 hunks)
🔇 Additional comments (1)
web/app/pages/Project/View/components/RefreshStatsButton.tsx (1)
75-80: Button hover/dark-mode styling change looks consistentThe updated class list keeps focus/disabled behavior intact while making the button visually quieter by default and only emphasizing it on hover (including dark mode via
hover:dark:border-slate-700/80anddark:hover:bg-slate-800). This aligns with the rest of the UI; no issues from a code perspective.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
web/app/pages/Project/View/components/SearchFilters.tsx (2)
330-333: User input is not persisted until explicitly committed—verify this is the intended UX.
handleInputChangeonly updates localinputValuestate without callingonChange. The value is only committed when the user:
- Selects an item from the dropdown
- Presses Enter
- Clicks "Use this value" button
This means if a user types a value and clicks elsewhere (including the Submit button), their typed text is lost. Since
hasValidFilters(line 717) checksrow.value, the Submit button may be disabled even though text appears in the input.If this is intentional (requiring explicit commitment), consider adding visual feedback (e.g., a subtle "Press Enter to confirm" hint). Otherwise, sync on blur:
+ const handleBlur = () => { + if (inputValue.trim() && inputValue !== displayValue) { + // Commit the typed value + if (column === 'cc') { + const alpha2 = countries.getAlpha2Code(inputValue, language) + onChange(alpha2 || inputValue.trim()) + } else { + onChange(inputValue.trim()) + } + } + setIsOpen(false) + }
768-776:ListboxOptionrender prop should useactiveinstead offocus.Headless UI's
ListboxOptionexposes{ active, selected, disabled }in its render function, notfocus. Usingfocusmeans it's alwaysundefined, so the hover/keyboard-focus styling never applies.- className={({ focus }) => - cx('relative cursor-pointer py-2 pr-4 pl-3 select-none', { - 'bg-gray-100 dark:bg-slate-700': focus, - 'text-gray-700 dark:text-gray-50': !focus, - }) - } + className={({ active }) => + cx('relative cursor-pointer py-2 pr-4 pl-3 select-none', { + 'bg-gray-100 dark:bg-slate-700': active, + 'text-gray-700 dark:text-gray-50': !active, + }) + }Same fix needed at lines 829-834.
🧹 Nitpick comments (6)
backend/apps/cloud/src/analytics/analytics.service.ts (1)
2007-2033: Version filters query is consistent and safely scopedThe new
getVersionFiltershelper correctly scopes bypid, restricts table/column to a small union, and returns a simple{ name, version }shape that matches the UI usage. You might optionally add anORDER BY ${column}, ${safeVersionCol}for deterministic ordering, but the current implementation is sound.backend/apps/community/src/analytics/dto/get-version-filters.dto.ts (1)
5-37: Community DTO mirrors cloud DTO, avoiding driftThis community
GetVersionFiltersDtois identical in structure and validation to the cloud one, which is good for keeping behavior aligned across editions. If duplication grows, consider extracting a shared DTO base, but it's fine as-is.web/app/pages/Project/View/components/SearchFilters.tsx (4)
526-563: Dependency array may cause stale closure or unnecessary re-creations.
fetchFilterValuesincludesfilterValuesCacheandloadingColumnsin dependencies. Since these are state values that change when the function itself modifies them, this could cause subtle issues:
- The function gets recreated on every state update
- The early-return check
filterValuesCache[column]uses the closure value, which should be fineConsider using refs for the early-return checks if performance becomes an issue, but functionally this should work correctly.
Minor: The error case silently sets an empty array. Consider logging or showing a toast so users know why no suggestions appear:
} catch (error) { console.error('Failed to fetch filter values:', error) setFilterValuesCache((prev) => ({ ...prev, [column]: [] })) + // Optional: Show user-facing error }
565-575: Pre-fetching all filter columns at modal open could cause burst of API calls.When the modal opens, this effect iterates through all
panelOptions(up to 20 columns) and triggers a fetch for each. This could result in:
- 20+ concurrent API requests
- Backend load spikes
- Potential rate limiting
Consider:
- Lazy loading: fetch only when user selects a column
- Batching: create a single endpoint that returns all filter options
- Progressive loading: prioritize commonly-used filters first
For now, the UX benefit of instant suggestions may outweigh the cost, but monitor backend performance.
577-585: Effect dependency could be simplified.Including
filterRowsin the dependency array means this effect runs on every row change, even though it only acts whenfilterRowsis empty. This is functionally correct but slightly wasteful.A minor optimization:
useEffect(() => { if (showModal) { if (_isEmpty(filterRows)) { setFilterRows([{ id: crypto.randomUUID(), column: '', operator: 'is', value: '' }]) } } - }, [showModal, filterRows]) + }, [showModal]) // Only run when modal visibility changesHowever, removing
filterRowswould violate the exhaustive-deps rule, so this is fine as-is.
496-500: Consider edge case: version strings containing the separator.If
VERSION_SEPARATOR(|||) ever appears in a browser/OS name or version (unlikely but possible with custom user agents),splitwill produce more than 2 parts, and only the first two are captured.For robustness:
const parseVersionValue = (value: string): { parent: string; version: string } | null => { if (!value.includes(VERSION_SEPARATOR)) return null - const [parent, version] = value.split(VERSION_SEPARATOR) + const idx = value.indexOf(VERSION_SEPARATOR) + const parent = value.slice(0, idx) + const version = value.slice(idx + VERSION_SEPARATOR.length) return { parent, version } }This ensures only the first separator is used as the split point.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/apps/cloud/src/analytics/analytics.controller.ts(2 hunks)backend/apps/cloud/src/analytics/analytics.service.ts(1 hunks)backend/apps/cloud/src/analytics/dto/get-version-filters.dto.ts(1 hunks)backend/apps/community/src/analytics/analytics.controller.ts(2 hunks)backend/apps/community/src/analytics/analytics.service.ts(1 hunks)backend/apps/community/src/analytics/dto/get-version-filters.dto.ts(1 hunks)web/app/api/index.ts(1 hunks)web/app/pages/Project/View/components/SearchFilters.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
backend/apps/community/src/analytics/dto/get-version-filters.dto.ts (1)
backend/apps/cloud/src/analytics/dto/get-version-filters.dto.ts (1)
GetVersionFiltersDto(5-38)
backend/apps/community/src/analytics/analytics.controller.ts (4)
backend/apps/cloud/src/auth/decorators/auth.decorator.ts (1)
Auth(10-17)backend/apps/community/src/auth/decorators/auth.decorator.ts (1)
Auth(9-16)backend/apps/cloud/src/analytics/dto/get-version-filters.dto.ts (1)
GetVersionFiltersDto(5-38)backend/apps/community/src/analytics/dto/get-version-filters.dto.ts (1)
GetVersionFiltersDto(5-38)
web/app/api/index.ts (4)
backend/apps/cloud/src/analytics/analytics.controller.ts (1)
getVersionFilters(531-547)backend/apps/cloud/src/analytics/analytics.service.ts (1)
getVersionFilters(2007-2033)backend/apps/community/src/analytics/analytics.controller.ts (1)
getVersionFilters(1406-1420)backend/apps/community/src/analytics/analytics.service.ts (1)
getVersionFilters(490-516)
backend/apps/cloud/src/analytics/dto/get-version-filters.dto.ts (1)
backend/apps/community/src/analytics/dto/get-version-filters.dto.ts (1)
GetVersionFiltersDto(5-38)
backend/apps/cloud/src/analytics/analytics.controller.ts (2)
backend/apps/cloud/src/auth/decorators/auth.decorator.ts (1)
Auth(10-17)backend/apps/cloud/src/analytics/dto/get-version-filters.dto.ts (1)
GetVersionFiltersDto(5-38)
web/app/pages/Project/View/components/SearchFilters.tsx (4)
web/app/pages/Project/View/ViewProject.helpers.tsx (1)
deviceIconMapping(1646-1654)web/app/lib/constants/index.ts (5)
BROWSER_LOGO_MAP(929-976)OS_LOGO_MAP(978-1004)OS_LOGO_MAP_DARK(1006-1011)FILTERS_PANELS_ORDER(253-273)ERRORS_FILTERS_PANELS_ORDER(274-274)web/app/providers/ThemeProvider.tsx (1)
useTheme(29-37)web/app/api/index.ts (3)
getVersionFilters(1434-1444)getErrorsFilters(1422-1432)getFilters(1410-1420)
🔇 Additional comments (12)
backend/apps/cloud/src/analytics/dto/get-version-filters.dto.ts (1)
5-37: DTO definition for version filters is clear and aligns with usageThe
GetVersionFiltersDtoshape (pid/type/column) and validators match the controller and service expectations, and mirror the community edition DTO, which keeps both stacks consistent. No changes needed.web/app/api/index.ts (1)
1434-1444: Frontend wrapper for version filters matches backend contract
getVersionFiltersuses the same URL pattern and password header style asgetFilters/getErrorsFilters, and the return type{ name, version }[]matches the service implementation. Throwing the raw error is consistent with neighboring helpers here.backend/apps/cloud/src/analytics/analytics.controller.ts (2)
68-68: Import wiring for GetVersionFiltersDto is correctThe DTO import path is consistent with other analytics DTO imports and matches the file location.
529-547: Cloud version-filters endpoint is correctly guarded and delegated
GET log/filters/versionsreuses the existing access model (@Auth,checkProjectAccess,checkBillingAccess) and simply delegates toanalyticsService.getVersionFilters, which keeps controller logic thin and consistent with other filter endpoints.backend/apps/community/src/analytics/analytics.controller.ts (2)
44-44: DTO import for version filters is consistent with other DTOsImporting
GetVersionFiltersDtoalongsideGetFiltersDtokeeps the controller’s DTO usage organized and clear.
1404-1420: Community version-filters endpoint matches cloud behavior minus billingThe community
GET log/filters/versionsendpoint mirrors the cloud one (minus billing checks, as expected here), using the DTO for validation and delegating straight toanalyticsService.getVersionFilters. Route path and auth usage are consistent with existing filters endpoints.backend/apps/community/src/analytics/analytics.service.ts (1)
490-516: LGTM! Clean implementation of the version filters endpoint.The method correctly:
- Sanitizes table and column names by mapping to known safe values (no SQL injection risk)
- Uses parameterized queries for the
pidvalue- Applies NULL checks on both columns
- Returns a properly typed result
web/app/pages/Project/View/components/SearchFilters.tsx (5)
62-85: Good organization of filter category icons.The icon mapping is well-structured and covers all filter panel types. Using
ReactNodeas the value type allows flexibility for different icon components.
139-166: Well-structured operator definitions.The
operatorToFilterfunction cleanly maps the UI operator values to the internal filter representation. The exhaustive switch covers all cases.
640-664: Browser/OS version filtering creates paired filters—this is intentional but worth documenting.When a user selects "Chrome 120.5" from the browser version dropdown, the code correctly splits this into two filters:
br=Chrome(browser name)brv=120.5(version number)This ensures precise filtering (Chrome version 120.5, not Firefox version 120.5). Good design choice.
717-717: Clear validation logic for submit button.The
hasValidFilterscheck correctly ensures at least one filter row has both a column and value selected before enabling submission.
129-134: I'll help you verify the prefix logic for filter keys. Let me search for the URL parsing logic that handles these prefixes to ensure they're consistently interpreted.
<function_calls>
#!/bin/bashSearch for logic that parses URL params and reconstructs filters
Looking for where ^ prefix is handled
First, search for parseUrlParams or similar parsing functions
rg -n "parseUrlParams|parseFilter|fromUrl" --type ts --type tsx -C 3
Also search for handling of ^ prefix specifically
rg -n "^|startsWith.*'\^'" --type ts --type tsx -C 3
</function_calls>Looking for the file to understand the full context of filter handling:
<function_calls>
#!/bin/bashGet more context from the SearchFilters.tsx file
First check file size
wc -l web/app/pages/Project/View/components/SearchFilters.tsx
Look for all function definitions and filter-related logic
rg -n "function|const.=.(|export" web/app/pages/Project/View/components/SearchFilters.tsx | head -30
</function_calls><function_calls>
#!/bin/bashRead the relevant sections of SearchFilters.tsx
Show lines around the encoding logic and find any parsing logic
sed -n '1,50p' web/app/pages/Project/View/components/SearchFilters.tsx
echo "---"
sed -n '87,140p' web/app/pages/Project/View/components/SearchFilters.tsx
</function_calls><function_calls>
#!/bin/bashContinue reading the file to find parsing logic
sed -n '140,250p' web/app/pages/Project/View/components/SearchFilters.tsx
</function_calls><function_calls>
#!/bin/bashSearch in the entire file for where filters are reconstructed from URL
Look for logic that reads these prefixes back
rg -n "isExclusive|isContains" web/app/pages/Project/View/components/SearchFilters.tsx -C 2
</function_calls><function_calls>
#!/bin/bashSearch more broadly for filter reconstruction or parsing from URL
Look in the entire codebase for functions that handle URLSearchParams with these prefixes
rg -n "URLSearchParams|searchParams" web/app/pages/Project/View/components/SearchFilters.tsx -C 5 | head -50
</function_calls><function_calls>
#!/bin/bashLet's search for the complementary function - if there's getFiltersUrlParams,
there should be a function to parse URL params back to filters
rg -n "getFiltersUrlParams|parseFiltersFromUrl|filtersFromUrl" --type ts --type tsx -C 3
</function_calls><function_calls>
#!/bin/bashSearch for where filters are read from URL (setSearchParams or similar)
and look for the decoding logic
rg -n "setSearchParams|new URLSearchParams|location.search" --type ts --type tsx web/app/pages/Project/View/components/SearchFilters.tsx -C 5
</function_calls><function_calls>
#!/bin/bashLet's read the entire SearchFilters.tsx file to understand the full context
cat -n web/app/pages/Project/View/components/SearchFilters.tsx
</function_calls><function_calls>
#!/bin/bashSearch for where the URL parameters are being interpreted
Look for any place where '!' or '~' or '^' prefixes are stripped/parsed
rg -n "startsWith.['"]!" --type ts --type tsx -C 3
rg -n "startsWith.['"]~" --type ts --type tsx -C 3
rg -n "substring|slice(" web/app/pages/Project/View/components/SearchFilters.tsx -C 2 | head -40
</function_calls><function_calls>
#!/bin/bashSearch for the actual parsing/reconstruction of filters from URL
Look for the useEffect or initialization that reads from URL
rg -n "useEffect|useSearchParams|location|searchParams" web/app/pages/Project/View/components/SearchFilters.tsx -C 5
</function_calls><function_calls>
#!/bin/bashLet's check if there's a utility file that handles filter parsing
Search for files that might contain the parsing logic
fd -name ".ts" -o -name ".tsx" web/app/pages/Project/View/ | xargs rg -l "getFiltersUrlParams"
</function_calls><function_calls>
#!/bin/bashSearch for all files that reference getFiltersUrlParams or similar functions
rg -r "import.*getFiltersUrlParams|export.*getFiltersUrlParams" --type ts --type tsx
</function_calls><function_calls>
#!/bin/bashLet's look at the entire SearchFilters.tsx more carefully
and focus on finding all the pieces of the puzzle
wc -l web/app/pages/Project/View/components/SearchFilters.tsx
</function_calls><function_calls>
#!/bin/bashGet the full file with line numbers
head -n 500 web/app/pages/Project/View/components/SearchFilters.tsx | cat -n
</function_calls><function_calls>
#!/bin/bashLook for the section where filters are being applied or read
Search for the part that handles the URL parameters in reverse
rg -n "Array.from|getAll|forEach" web/app/pages/Project/View/components/SearchFilters.tsx -C 3
</function_calls><function_calls>
#!/bin/bashLet me search for where the prefixes are actually parsed from keys
This should show us where ! ~ or ^ are stripped from the filter key names
rg -n 'replace.*[!~^]|substring|startsWith|slice' web/app/pages/Project/View/components/SearchFilters.tsx -C 2
</function_calls><function_calls>
#!/bin/bashTry a different approach - look at lines 87-200 in detail to understand the full flow
sed -n '87,200p' web/app/pages/Project/View/components/SearchFilters.tsx | cat -n
</function_calls><function_calls>
#!/bin/bashCheck if there's a reverse parsing function (from URL back to filters)
Search across the codebase for functions that might reconstruct filters
rg -n "setFilters|filters.*=" web/app/pages/Project/View/components/SearchFilters.tsx -C 3 | head -50
</function_calls><function_calls>
#!/bin/bashLet's search for where the URL is initially parsed when the component loads
Look for useEffect with dependency on searchParams or location
ast-grep --pattern 'useEffect(() => {
$$$
}, [$_])'
</function_calls><function_calls>
#!/bin/bashSearch for any function that processes the URL search params
and converts them back to filter objects
rg -n "filter.*=|setFilter" web/app/pages/Project/View/components/SearchFilters.tsx | head -30
</function_calls><function_calls>
#!/bin/bashLet me look at the getFiltersUrlParams implementation more carefully
Search for where it's defined
rg -n "function getFiltersUrlParams|const getFiltersUrlParams|export.*getFiltersUrlParams" --type ts --type tsx -A 50 | head -80
</function_calls><function_calls>
#!/bin/bashLook at lines 100-140 more carefully to understand what's happening
sed -n '100,140p' web/app/pages/Project/View/components/SearchFilters.tsx | cat -n
</function_calls><function_calls>
#!/bin/bashNow let's search for the mirror/complementary logic that parses these parameters
Search in the entire web/app directory for parsing logic
rg -n "startsWith.*'!'" web/app/ --type ts --type tsx -C 3
</function_calls><function_calls>
#!/bin/bashSearch for where the filter key prefixes are interpreted
Look for conditions checking for these special characters
rg -n "[0].*[=!~^]|\.substring(0|charAt(0" web/app/pages/Project/View/components/ --type ts --type tsx -C 3
</function_calls><function_calls>
#!/bin/bashCheck if there's a separate utility file for filter handling
fd -name ".ts" -o -name ".tsx" web/app/ | xargs rg -l "isExclusive.*isContains|filter.*prefix|parseFilter"
</function_calls><function_calls>
#!/bin/bashLet me look for any comment or documentation about the prefix scheme
rg -n "exclusive|contains|prefix|mapKey" web/app/pages/Project/View/components/SearchFilters.tsx -C 2
</function_calls><function_calls>
#!/bin/bashSearch for the complete getFiltersUrlParams function
sed -n '87,137p' web/app/pages/Project/View/components/SearchFilters.tsx
</function_calls><function_calls>
#!/bin/bashSearch for where these URL params are actually read and used
Look for the initialization or effects that consume the URL params
rg -n "getFiltersUrlParams|" web/app/ --type ts --type tsx -B 5 -A 10
</function_calls><function_calls>
#!/bin/bashLet me search in a much simpler way - look at what functions call getFiltersUrlParams
rg -n "getFiltersUrlParams" web/app/ --type ts --type tsx
</function_calls>Based on the review comment structure and my search attempts, let me look directly at the file content to understand the full context:
<function_calls>
#!/bin/bashJust show me the entire SearchFilters.tsx file with line numbers
cat web/app/pages/Project/View/components/SearchFilters.tsx
</function_calls>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
web/app/pages/Project/View/components/SearchFilters.tsx (2)
191-375: ValueInput still allows stalerow.valueto be submitted when the visible input is cleared or edited
ValueInputkeepsinputValuelocal and never callsonChangefromhandleInputChange, sorow.valuecan remain non-empty while the user clears the field or types a different value. In that case,hasValidFiltersandonSubmitstill use the oldrow.value, and the user can apply filters that don't match what they see (e.g. clear the field → button remains enabled → old filter is submitted).To keep the parent state as the single source of truth, wire typing directly into
onChange:const handleInputChange = (e: React.ChangeEvent<HTMLInputElement>) => { const newValue = e.target.value setInputValue(newValue) + // Keep parent row.value in sync with what the user sees + onChange(newValue) + // Optionally ensure suggestions reopen when typing + setIsOpen(true) }This preserves your existing submit-time normalization (e.g. country code conversion in
onSubmit) while ensuring that validation and applied filters always reflect the visible input.Also applies to: 380-485
739-797: Headless UIListboxOptionshould useactive(notfocus) for highlight stateBoth Listbox option lists use
className={({ focus }) => ...}, but Headless UI’sListboxOptionrender-prop exposes{ active, selected, disabled }, notfocus. As a result, your "active" background/text styles never apply, and keyboard/mouse hover state isn’t visually indicated.Update the render-prop destructuring and conditionals to use
active:- className={({ focus }) => - cx('relative cursor-pointer py-2 pr-4 pl-3 select-none', { - 'bg-gray-100 dark:bg-slate-700': focus, - 'text-gray-700 dark:text-gray-50': !focus, - }) - } + className={({ active }) => + cx('relative cursor-pointer py-2 pr-4 pl-3 select-none', { + 'bg-gray-100 dark:bg-slate-700': active, + 'text-gray-700 dark:text-gray-50': !active, + }) +}…and similarly for the operator options (
pl-8variant).Headless UI React ListboxOption (v2.x) render-prop API: which state props (`active`, `selected`, `disabled`, etc.) are exposed for styling options?Also applies to: 821-851
🧹 Nitpick comments (2)
web/app/pages/Project/View/components/AddAViewModal.tsx (1)
397-446: Filter key now depends on operator flags; consider a more stable identity ifFilterbecomes statefulUsing
key={${column}-${filter}-${isExclusive}-${isContains}}guarantees uniqueness but also forces React to remount theFilterwhenever the operator flags change, which can reset any internal stateFiltermight hold (focus, animations, etc.). Since(column, filter)pairs are already unique in this UI, you could keep the key stable (e.g.,${column}-${filter}or a dedicatedidfield on the filter) and rely on props for operator changes.web/app/pages/Project/View/components/SearchFilters.tsx (1)
526-575: Prefetching all filter columns on modal open may be unnecessarily heavy
useEffectprefetches values for everypanelOptionscolumn whenever the modal opens. For projects with many distinct values per column, this can trigger a large number of backend queries and increase initial modal latency, even if the user only interacts with 1–2 columns.Consider lazily fetching values per column (on first selection) and optionally adding a background prefetch for the most commonly used panels, so initial open is cheaper while still benefiting from caching.
Also applies to: 587-617
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/app/pages/Project/View/components/AddAViewModal.tsx(1 hunks)web/app/pages/Project/View/components/SearchFilters.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
web/app/pages/Project/View/components/SearchFilters.tsx (4)
web/app/pages/Project/View/ViewProject.helpers.tsx (1)
deviceIconMapping(1646-1654)web/app/lib/constants/index.ts (5)
BROWSER_LOGO_MAP(929-976)OS_LOGO_MAP(978-1004)OS_LOGO_MAP_DARK(1006-1011)FILTERS_PANELS_ORDER(253-273)ERRORS_FILTERS_PANELS_ORDER(274-274)web/app/providers/ThemeProvider.tsx (1)
useTheme(29-37)web/app/api/index.ts (3)
getVersionFilters(1434-1444)getErrorsFilters(1422-1432)getFilters(1410-1420)
🔇 Additional comments (1)
web/app/pages/Project/View/components/SearchFilters.tsx (1)
87-137: URL filter encoding, operators, and version handling look consistent and correctThe combination of
operatorToFilter, the updated map key ingetFiltersUrlParams(includingisExclusiveandisContains), and the^/~prefixes yields a coherent mapping between UI operators and URL params. Thebrv/osvhandling (splitting combined values into{br, brv}/{os, osv}pairs) also aligns with the version helpers and should serialize cleanly throughgetFiltersUrlParams.Also applies to: 139-167, 489-500, 630-713
1 participant
#420
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit
New Features
Style
Localization
Other
✏️ Tip: You can customize this high-level summary in your review settings.