-
Notifications
You must be signed in to change notification settings - Fork 18
feat: enable scroll and search for version decisions dialog #1029
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,34 @@ | ||
| /* eslint-disable @typescript-eslint/prefer-nullish-coalescing */ | ||
| import { ShieldOffIcon } from "lucide-react"; | ||
| import { useState } from "react"; | ||
| import { keepPreviousData } from "@tanstack/react-query"; | ||
| import { Loader2, SearchIcon, ShieldOffIcon } from "lucide-react"; | ||
| import { useDebounce } from "react-use"; | ||
|
|
||
| import type { DeploymentVersionStatus } from "../types"; | ||
| import { trpc } from "~/api/trpc"; | ||
| import { Button } from "~/components/ui/button"; | ||
| import { | ||
| Dialog, | ||
| DialogContent, | ||
| DialogHeader, | ||
| DialogTitle, | ||
| } from "~/components/ui/dialog"; | ||
| import { Input } from "~/components/ui/input"; | ||
| import { DeploymentVersion } from "./DeploymentVersion"; | ||
| import { PolicySkipDialog } from "./policy-skip/PolicySkipDialog"; | ||
| import { usePolicyRulesForVersion } from "./usePolicyRulesForVersion"; | ||
|
|
||
| const PAGE_SIZE = 20; | ||
|
|
||
| type Version = { | ||
| id: string; | ||
| name?: string; | ||
| tag?: string; | ||
| status: DeploymentVersionStatus; | ||
| }; | ||
|
|
||
| type VersionRowProps = { | ||
| version: { | ||
| id: string; | ||
| name?: string; | ||
| tag?: string; | ||
| status: DeploymentVersionStatus; | ||
| }; | ||
| version: Version; | ||
| environment: { id: string; name: string }; | ||
| }; | ||
|
|
||
|
|
@@ -57,39 +66,103 @@ function VersionRow({ version, environment }: VersionRowProps) { | |
| type EnvironmentVersionDecisionsProps = { | ||
| environment: { id: string; name: string }; | ||
| deploymentId: string; | ||
| versions: { | ||
| id: string; | ||
| name?: string; | ||
| tag?: string; | ||
| status: DeploymentVersionStatus; | ||
| }[]; | ||
| open: boolean; | ||
| onOpenChange: (open: boolean) => void; | ||
| }; | ||
|
|
||
| export function EnvironmentVersionDecisions({ | ||
| environment, | ||
| versions, | ||
| deploymentId, | ||
| open, | ||
| onOpenChange, | ||
| }: EnvironmentVersionDecisionsProps) { | ||
| const [search, setSearch] = useState(""); | ||
| const [debouncedSearch, setDebouncedSearch] = useState(""); | ||
|
|
||
| useDebounce(() => setDebouncedSearch(search), 250, [search]); | ||
|
|
||
| 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], | ||
| ); | ||
|
|
||
| const versions = versionsQuery.data?.pages.flat() ?? []; | ||
| const isInitialLoading = versionsQuery.isLoading; | ||
| const isEmpty = !isInitialLoading && versions.length === 0; | ||
|
|
||
| return ( | ||
| <Dialog open={open} onOpenChange={onOpenChange}> | ||
| <DialogContent className="flex max-h-[85vh] max-w-2xl flex-col overflow-hidden p-0"> | ||
| <DialogHeader className="border-b p-4"> | ||
| <DialogTitle className="text-base">{environment.name}</DialogTitle> | ||
| </DialogHeader> | ||
|
|
||
| <div className="max-h-[calc(85vh-120px)] overflow-y-auto px-4 pb-4"> | ||
| <div className="space-y-4"> | ||
| {versions.map((version) => ( | ||
| <VersionRow | ||
| key={version.id} | ||
| version={version} | ||
| environment={environment} | ||
| /> | ||
| ))} | ||
| <div className="max-h-[calc(85vh-180px)] overflow-y-auto px-4 pb-4"> | ||
| <div className="relative"> | ||
| <SearchIcon className="absolute left-2.5 top-1/2 size-4 -translate-y-1/2 text-muted-foreground" /> | ||
| <Input | ||
| value={search} | ||
| onChange={(e) => setSearch(e.target.value)} | ||
| placeholder="Search by version name or tag..." | ||
| className="pl-8" | ||
| /> | ||
| </div> | ||
| {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> | ||
| )} | ||
|
Comment on lines
+120
to
+133
|
||
|
|
||
| {!isInitialLoading && versions.length > 0 && ( | ||
| <div className="space-y-4 pt-4"> | ||
| {versions.map((version) => ( | ||
| <VersionRow | ||
| key={version.id} | ||
| version={version} | ||
| environment={environment} | ||
| /> | ||
| ))} | ||
|
Comment on lines
+135
to
+143
|
||
|
|
||
| {versionsQuery.hasNextPage && ( | ||
| <div className="flex justify-center pt-2"> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={() => versionsQuery.fetchNextPage()} | ||
| disabled={versionsQuery.isFetchingNextPage} | ||
| > | ||
| {versionsQuery.isFetchingNextPage ? ( | ||
| <> | ||
| <Loader2 className="mr-2 size-3 animate-spin" /> | ||
| Loading... | ||
| </> | ||
| ) : ( | ||
| "Load more" | ||
| )} | ||
| </Button> | ||
| </div> | ||
| )} | ||
| </div> | ||
| )} | ||
| </div> | ||
| </DialogContent> | ||
| </Dialog> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,16 @@ import { parse } from "cel-js"; | |||||
| import { v4 as uuidv4 } from "uuid"; | ||||||
| import { z } from "zod"; | ||||||
|
|
||||||
| import { and, asc, desc, eq, inArray, takeFirst } from "@ctrlplane/db"; | ||||||
| import { | ||||||
| and, | ||||||
| asc, | ||||||
| desc, | ||||||
| eq, | ||||||
| ilike, | ||||||
| inArray, | ||||||
| or, | ||||||
| takeFirst, | ||||||
| } from "@ctrlplane/db"; | ||||||
| import { | ||||||
| enqueueDeploymentSelectorEval, | ||||||
| enqueuePolicyEval, | ||||||
|
|
@@ -144,6 +153,39 @@ export const deploymentsRouter = router({ | |||||
| return versions; | ||||||
| }), | ||||||
|
|
||||||
| searchVersions: protectedProcedure | ||||||
| .meta({ | ||||||
| authorizationCheck: ({ canUser, input }) => | ||||||
| canUser | ||||||
| .perform(Permission.DeploymentVersionList) | ||||||
| .on({ type: "deployment", id: input.deploymentId }), | ||||||
| }) | ||||||
| .input( | ||||||
| z.object({ | ||||||
| deploymentId: z.uuid(), | ||||||
| query: z.string().optional(), | ||||||
| limit: z.number().min(1).max(100).default(20), | ||||||
|
||||||
| limit: z.number().min(1).max(100).default(20), | |
| limit: z.number().min(1).max(1000).default(20), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:refetchInterval: open ? 5000 : falseplusrefetchOnWindowFocusand only polling the first page (e.g., invalidating/refetching a sibling non-infinite query for "latest versions"), or🤖 Prompt for AI Agents