feat: enable scroll and search for version decisions dialog#1029
feat: enable scroll and search for version decisions dialog#1029adityachoudhari26 merged 2 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughThe changes implement on-demand version fetching in the deployment UI. The EnvironmentVersionDecisions component now fetches deployment versions using a debounced search query instead of receiving a preloaded array. A new searchVersions TRPC procedure enables text-based version search with pagination, querying the database with optional filters on name and tag fields. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as EnvironmentVersionDecisions
participant TRPC as searchVersions Procedure
participant DB as Database
User->>UI: Types search query
UI->>UI: Debounce input (triggers search)
UI->>TRPC: searchVersions(deploymentId, query, limit, cursor)
TRPC->>TRPC: Validate query, limit (1-100), cursor
TRPC->>TRPC: Authorize permission.DeploymentVersionList
TRPC->>DB: Find versions WHERE deploymentId<br/>AND (name ILIKE query OR tag ILIKE query)<br/>ORDER BY createdAt DESC<br/>LIMIT/OFFSET pagination
DB->>TRPC: Return paginated version results
TRPC->>UI: Resolve infinite query page
UI->>UI: Flatten pages and render VersionRow items
UI->>UI: Conditionally show "Load more" button
UI->>User: Display search results with pagination
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Pull request overview
Adds server-backed search + pagination to the “version decisions” dialog so users can scroll and find/approve older deployment versions (fixing the UI limitation described in #1026).
Changes:
- Introduces
deployment.searchVersionsTRPC endpoint with optionalqueryfilter (name/tag) and pagination. - Updates
EnvironmentVersionDecisionsto fetch versions on-demand, add a search input, and support “Load more” + scrollable dialog body. - Stops passing the limited
versionslist from the deployment detail page into the dialog.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/trpc/src/routes/deployments.ts | Adds searchVersions query with ilike filtering + pagination. |
| apps/web/app/routes/ws/deployments/page.$deploymentId.tsx | Removes versions prop usage; dialog now fetches its own data. |
| apps/web/app/routes/ws/deployments/_components/environmentversiondecisions/EnvironmentVersionDecisions.tsx | Implements searchable, scrollable, paginated versions list backed by TRPC. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const versionsQuery = trpc.deployment.searchVersions.useQuery( | ||
| { | ||
| deploymentId, | ||
| query: debouncedSearch || undefined, | ||
| limit: PAGE_SIZE * pageCount, | ||
| offset: 0, | ||
| }, | ||
| { refetchInterval: 5000, placeholderData: keepPreviousData }, |
There was a problem hiding this comment.
This pagination approach increases limit while keeping offset at 0. Once pageCount grows past what the backend allows (currently max limit=100), the query will start failing, and even before that it re-downloads all previously loaded versions on every “Load more”. Prefer offset-based pagination (fixed PAGE_SIZE + offset = PAGE_SIZE*(pageCount-1) and append results) or switch to useInfiniteQuery to avoid validation failures and redundant data transfer.
| {isInitialLoading && ( | ||
| <div className="flex items-center justify-center py-8 text-sm text-muted-foreground"> | ||
| <Loader2 className="mr-2 size-4 animate-spin" /> | ||
| Loading versions... | ||
| </div> | ||
| )} | ||
|
|
||
| {isEmpty && ( | ||
| <div className="py-8 text-center text-sm text-muted-foreground"> | ||
| {debouncedSearch | ||
| ? `No versions match "${debouncedSearch}"` | ||
| : "No versions found"} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
versionsQuery errors (e.g., validation error once limit exceeds server max, or network failures) aren’t handled in the UI. In those cases the dialog will show neither results nor an error message, which is confusing and makes troubleshooting harder. Consider rendering an explicit error state when versionsQuery.isError is true (and optionally disable “Load more” when errored).
| z.object({ | ||
| deploymentId: z.uuid(), | ||
| query: z.string().optional(), | ||
| limit: z.number().min(1).max(100).default(20), |
There was a problem hiding this comment.
The UI paginates by increasing limit (PAGE_SIZE * pageCount). With this procedure’s input schema capping limit to 100, clicking “Load more” past 5 pages will start failing validation and the dialog will be unable to load older versions. Consider switching to offset-based pagination (fixed limit + increasing offset) or raising/removing the 100 max and aligning it with the frontend pagination approach.
| limit: z.number().min(1).max(100).default(20), | |
| limit: z.number().min(1).max(1000).default(20), |
| {!isInitialLoading && versions.length > 0 && ( | ||
| <div className="space-y-4 pt-4"> | ||
| {versions.map((version) => ( | ||
| <VersionRow | ||
| key={version.id} | ||
| version={version} | ||
| environment={environment} | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
Rendering many VersionRows now means usePolicyRulesForVersion() (used inside VersionRow) will fire one TRPC query per row and poll every 5s. With scroll + “Load more”, this can become dozens/hundreds of concurrent requests and significantly increase backend load. Consider batching policy evaluations for multiple versions, removing per-row polling, and/or virtualizing rows so only visible items fetch policy data.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/app/routes/ws/deployments/_components/environmentversiondecisions/EnvironmentVersionDecisions.tsx (1)
90-97: Avoid theas Parameters<...>[1]cast — type the options properly.The cast masks a real type mismatch:
searchVersionsreturns database rows wherenameandtagare non-null (defined as.notNull()in the schema), but the localVersiontype declares them as optional (name?: string,tag?: string). This inconsistency should surface as a compile error so that any future schema change is caught here rather than silently ignored.Prefer defining an explicit return type for
searchVersionsor correcting theVersioninterface to reflect the actual non-null guarantee from the database.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/routes/ws/deployments/_components/environmentversiondecisions/EnvironmentVersionDecisions.tsx` around lines 90 - 97, The code is masking a type mismatch by casting the options to Parameters<typeof trpc.deployment.searchVersions.useInfiniteQuery>[1]; remove that cast and fix the types properly: either update the local Version interface (used by EnvironmentVersionDecisions.tsx) to make name and tag non-optional (name: string; tag: string) to match the DB schema, or declare an explicit return type for trpc.deployment.searchVersions that reflects non-null columns and use that type for the infinite query options (the call around trpc.deployment.searchVersions.useInfiniteQuery with initialCursor, getNextPageParam, refetchInterval, placeholderData). Ensure the option object types line up so the compiler surfaces any real mismatches instead of being suppressed by the as-cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web/app/routes/ws/deployments/_components/environmentversiondecisions/EnvironmentVersionDecisions.tsx`:
- Around line 84-97: The infinite query versionsQuery
(trpc.deployment.searchVersions.useInfiniteQuery) is currently polling every 5s
and refetches every loaded page; change the polling to be conditional so it only
runs when the dialog is open by replacing refetchInterval: 5000 with
refetchInterval: open ? 5000 : false and set refetchOnWindowFocus: false (or
remove refetchInterval entirely if polling is not needed); keep existing params
(deploymentId, debouncedSearch, PAGE_SIZE, keepPreviousData) and the
getNextPageParam logic unchanged.
In `@packages/trpc/src/routes/deployments.ts`:
- Around line 171-187: The current search builds ilike patterns by interpolating
input.query directly into `%${search}%`, which treats user `%` and `_` as SQL
wildcards; fix by escaping LIKE metacharacters before interpolation. Implement
an escape helper (e.g., escapeLike) and call it on search (escape backslash
first, then replace % -> \% and _ -> \_) and then use `%${escaped}%` in the
ilike calls inside ctx.db.query.deploymentVersion.findMany so
ilike(schema.deploymentVersion.name, `%${escaped}%`) and
ilike(schema.deploymentVersion.tag, `%${escaped}%`) instead of using the raw
search. Ensure the same trimmed input.query is passed through the helper.
---
Nitpick comments:
In
`@apps/web/app/routes/ws/deployments/_components/environmentversiondecisions/EnvironmentVersionDecisions.tsx`:
- Around line 90-97: The code is masking a type mismatch by casting the options
to Parameters<typeof trpc.deployment.searchVersions.useInfiniteQuery>[1]; remove
that cast and fix the types properly: either update the local Version interface
(used by EnvironmentVersionDecisions.tsx) to make name and tag non-optional
(name: string; tag: string) to match the DB schema, or declare an explicit
return type for trpc.deployment.searchVersions that reflects non-null columns
and use that type for the infinite query options (the call around
trpc.deployment.searchVersions.useInfiniteQuery with initialCursor,
getNextPageParam, refetchInterval, placeholderData). Ensure the option object
types line up so the compiler surfaces any real mismatches instead of being
suppressed by the as-cast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 301bb2a6-1d30-4343-95fa-d056316f4580
📒 Files selected for processing (3)
apps/web/app/routes/ws/deployments/_components/environmentversiondecisions/EnvironmentVersionDecisions.tsxapps/web/app/routes/ws/deployments/page.$deploymentId.tsxpackages/trpc/src/routes/deployments.ts
💤 Files with no reviewable changes (1)
- apps/web/app/routes/ws/deployments/page.$deploymentId.tsx
| const versionsQuery = trpc.deployment.searchVersions.useInfiniteQuery( | ||
| { | ||
| deploymentId, | ||
| query: debouncedSearch || undefined, | ||
| limit: PAGE_SIZE, | ||
| }, | ||
| { | ||
| initialCursor: 0, | ||
| getNextPageParam: (lastPage: Version[], allPages: Version[][]) => | ||
| lastPage.length < PAGE_SIZE ? undefined : allPages.length * PAGE_SIZE, | ||
| refetchInterval: 5000, | ||
| placeholderData: keepPreviousData, | ||
| } as Parameters<typeof trpc.deployment.searchVersions.useInfiniteQuery>[1], | ||
| ); |
There was a problem hiding this comment.
Polling refetches every loaded page every 5s.
React Query refetches all previously loaded pages on an infinite query by default. Combined with refetchInterval: 5000, a user who has clicked "Load more" several times will trigger N paged queries every 5 seconds for as long as the dialog is open — this scales poorly and defeats much of the on-demand paging benefit that motivates this PR. Consider either:
- Dropping the 5s poll (the original motivation was visibility/search, not liveness), or
- Gating it with
refetchInterval: open ? 5000 : falseplusrefetchOnWindowFocusand only polling the first page (e.g., invalidating/refetching a sibling non-infinite query for "latest versions"), or - Increasing the interval significantly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/web/app/routes/ws/deployments/_components/environmentversiondecisions/EnvironmentVersionDecisions.tsx`
around lines 84 - 97, The infinite query versionsQuery
(trpc.deployment.searchVersions.useInfiniteQuery) is currently polling every 5s
and refetches every loaded page; change the polling to be conditional so it only
runs when the dialog is open by replacing refetchInterval: 5000 with
refetchInterval: open ? 5000 : false and set refetchOnWindowFocus: false (or
remove refetchInterval entirely if polling is not needed); keep existing params
(deploymentId, debouncedSearch, PAGE_SIZE, keepPreviousData) and the
getNextPageParam logic unchanged.
| .query(async ({ input, ctx }) => { | ||
| const search = input.query?.trim(); | ||
| return ctx.db.query.deploymentVersion.findMany({ | ||
| where: and( | ||
| eq(schema.deploymentVersion.deploymentId, input.deploymentId), | ||
| search | ||
| ? or( | ||
| ilike(schema.deploymentVersion.name, `%${search}%`), | ||
| ilike(schema.deploymentVersion.tag, `%${search}%`), | ||
| ) | ||
| : undefined, | ||
| ), | ||
| limit: input.limit, | ||
| offset: input.cursor, | ||
| orderBy: desc(schema.deploymentVersion.createdAt), | ||
| }); | ||
| }), |
There was a problem hiding this comment.
Escape LIKE wildcards in user-supplied search.
% and _ characters in input.query are treated as wildcards by ilike, so a query like _ or % matches everything and 50% matches any string containing 50. Consider escaping them before interpolation:
🛡️ Proposed fix
- const search = input.query?.trim();
+ const search = input.query?.trim();
+ const escaped = search?.replace(/[\\%_]/g, (c) => `\\${c}`);
return ctx.db.query.deploymentVersion.findMany({
where: and(
eq(schema.deploymentVersion.deploymentId, input.deploymentId),
search
? or(
- ilike(schema.deploymentVersion.name, `%${search}%`),
- ilike(schema.deploymentVersion.tag, `%${search}%`),
+ ilike(schema.deploymentVersion.name, `%${escaped}%`),
+ ilike(schema.deploymentVersion.tag, `%${escaped}%`),
)
: undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/trpc/src/routes/deployments.ts` around lines 171 - 187, The current
search builds ilike patterns by interpolating input.query directly into
`%${search}%`, which treats user `%` and `_` as SQL wildcards; fix by escaping
LIKE metacharacters before interpolation. Implement an escape helper (e.g.,
escapeLike) and call it on search (escape backslash first, then replace % -> \%
and _ -> \_) and then use `%${escaped}%` in the ilike calls inside
ctx.db.query.deploymentVersion.findMany so ilike(schema.deploymentVersion.name,
`%${escaped}%`) and ilike(schema.deploymentVersion.tag, `%${escaped}%`) instead
of using the raw search. Ensure the same trimmed input.query is passed through
the helper.
fixes #1026
Summary by CodeRabbit