Conversation
… a multi-value item
Advanced Analytics Filters
…lter from the URL
…lback return value.
…} falls through to empty-values default.
…vents and SQL injection
…etadata comparisons
…al, getFirstFilterValue in partner-profile/export routes
Instead of the partner searching the web for the company, we should the domain of the group destination URL from the default link settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/ui/analytics/use-analytics-filters.tsx`:
- Around line 1004-1018: The loader placeholders in activeFiltersWithStreaming
produce numeric values (values: [i]) which mismatches the string[] shape used
elsewhere; update the streaming branch that creates loader filter entries (the
Array.from(...).map callback) to emit string values (e.g., String(i) or `${i}`)
so the resulting loader filter has values: string[] and matches the
type/behavior expected by consumers of activeFiltersWithStreaming, leaving
activeFilters and streaming logic unchanged.
🧹 Nitpick comments (3)
apps/web/ui/analytics/use-analytics-filters.tsx (3)
121-123: Unnecessary indirection —parseFilterParamis a trivial wrapper.
parseFilterParamjust delegates toparseFilterValuewith no additional logic or closure state. You can callparseFilterValuedirectly at every call site and remove thisuseCallback.
165-165: Deeply nested ternary chain harms readability.This single line chains four ternary operators to resolve the param value for destructured keys. A lookup map would be clearer and easier to extend.
♻️ Suggested refactor
- const value = params[filter] || (filter === "domain" ? domain : filter === "tagIds" ? tagIds : filter === "root" ? root : filter === "folderId" ? folderId : undefined); + const destructuredParams: Record<string, string | undefined> = { + domain, + tagIds, + root, + folderId, + }; + const value = params[filter] || destructuredParams[filter];
879-887: URL construction for link filter assumes no protocol invalue.
new URL(\https://${value}`)will produce an invalid URL likehttps://https://example.com/...` ifvalueever contains a protocol prefix. Currently,linkConstructor({ pretty: true })strips the protocol, so this works — but it's an implicit coupling. A guard or comment would prevent a subtle breakage iflinkConstructor's behavior changes.🛡️ Defensive guard
} else if (key === "link") { + const linkUrl = value.startsWith("http") ? value : `https://${value}`; queryParams({ set: { - domain: new URL(`https://${value}`).hostname, - key: new URL(`https://${value}`).pathname.slice(1) || "_root", + domain: new URL(linkUrl).hostname, + key: new URL(linkUrl).pathname.slice(1) || "_root", }, del: "page", scroll: false, });
| const activeFiltersWithStreaming = useMemo( | ||
| () => [ | ||
| ...activeFilters, | ||
| ...(streaming && !activeFilters.length | ||
| ? Array.from({ length: 2 }, (_, i) => i).map((i) => ({ | ||
| () => { | ||
| return [ | ||
| ...activeFilters, | ||
| ...(streaming && !activeFilters.length | ||
| ? Array.from({ length: 2 }, (_, i) => i).map((i) => ({ | ||
| key: "loader", | ||
| value: i, | ||
| values: [i], | ||
| operator: "IS" as const, | ||
| })) | ||
| : []), | ||
| ], | ||
| : []), | ||
| ]; | ||
| }, | ||
| [activeFilters, streaming], | ||
| ); |
There was a problem hiding this comment.
Loader placeholder uses numeric values while all other filters use string arrays.
The loader entries use values: [i] where i is a number (0 or 1), while every other filter builds values as string[]. If any downstream consumer iterates activeFiltersWithStreaming and calls string methods on values, this will throw at runtime. Consider using string values for consistency.
Proposed fix
- ? Array.from({ length: 2 }, (_, i) => i).map((i) => ({
+ ? Array.from({ length: 2 }, (_, i) => i).map((i) => ({
key: "loader",
- values: [i],
+ values: [String(i)],
operator: "IS" as const,
}))🤖 Prompt for AI Agents
In `@apps/web/ui/analytics/use-analytics-filters.tsx` around lines 1004 - 1018,
The loader placeholders in activeFiltersWithStreaming produce numeric values
(values: [i]) which mismatches the string[] shape used elsewhere; update the
streaming branch that creates loader filter entries (the Array.from(...).map
callback) to emit string values (e.g., String(i) or `${i}`) so the resulting
loader filter has values: string[] and matches the type/behavior expected by
consumers of activeFiltersWithStreaming, leaving activeFilters and streaming
logic unchanged.
Adds support for advanced filter operators in analytics:
IS,IS NOT.Changes
Frontend:
?country=US,BRor?country=-USfor negationBackend:
@dub/utilsfor frontend/backend consistencyTinybird:
Summary by CodeRabbit
New Features
Improvements
Tests