fix: project collection and db idx for environment#3973
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
📝 WalkthroughWalkthroughReorganized TRPC under a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Browser UI
participant Layout as useProjectLayout
participant Hook as useDeployments
participant Collections as Project Collections
participant TRPC as trpcClient.deploy.*
UI->>Layout: mount page -> request collections for project
UI->>Hook: mount -> request deployments (uses collections + filters)
Hook->>Collections: query project -> read liveDeploymentId
Hook->>Collections: query activeDeployment by liveDeploymentId
Hook->>Collections: query deployments (join environments, apply filters)
Collections->>TRPC: fetch via trpcClient.deploy.deployment.list(...)
TRPC-->>Collections: return deployments
Collections-->>Hook: project, activeDeployment, deployments
Hook-->>UI: provide { deployments, activeDeployment, activeDeploymentId }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings🧬 Code graph analysis (2)go/pkg/db/models_generated.go (1)
go/pkg/db/querier_generated.go (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (2)
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 |
ogzhanolguncu
requested review from
Flo4604,
MichaelUnkey,
chronark,
imeyer,
mcstepp and
perkinsjr
as code owners
| }, | ||
| (table) => ({ | ||
| uniqueSlug: uniqueIndex("environments_workspace_id_slug_idx").on(table.workspaceId, table.slug), | ||
| uniqueSlug: uniqueIndex("environments_project_id_slug_idx").on(table.projectId, table.slug), |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/dashboard/lib/trpc/routers/deploy/deployment/getOpenApiDiff.ts (1)
181-183: Bug: environment equality compares object references.Compare ids or slugs; current check will almost always be false for identical envs from separate queries.
Apply:
- isSameEnvironment: oldDeployment.environment === newDeployment.environment, + isSameEnvironment: + oldDeployment.environment?.id === newDeployment.environment?.id || + oldDeployment.environment?.slug === newDeployment.environment?.slug,apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/page.tsx (1)
171-173: User-visible string bug:${date}renders literally.In JSX, use
{date}not a literal${date}.Apply:
- {environment?.slug} • ${date} + {environment?.slug} • {date}Do this in both baseline and comparison selectors.
Also applies to: 249-251
apps/dashboard/lib/trpc/routers/deploy/project/create.ts (1)
66-80: Close the slug race: convert DB unique violations into 409 CONFLICT.
Pre‑check helps but TOCTOU still exists. If two requests pass the check concurrently, the insert must translate a unique‑key error into TRPCError(CONFLICT) for consistent UX.Apply this diff to handle duplicate slugs gracefully:
@@ - } catch (txErr) { - console.error({ + } catch (txErr) { + // If the DB has a unique constraint on (workspaceId, slug) for projects, + // translate duplicate key errors into a 409. + if (txErr && typeof txErr === "object" && "code" in (txErr as any)) { + const code = (txErr as any).code; + // 23505 = unique_violation (Postgres) + if (code === "23505") { + throw new TRPCError({ + code: "CONFLICT", + message: `A project with slug "${input.slug}" already exists in this workspace`, + }); + } + } + console.error({ message: "Transaction failed during project creation", userId, workspaceId, projectId, projectSlug: input.slug, error: txErr instanceof Error ? txErr.message : String(txErr), stack: txErr instanceof Error ? txErr.stack : undefined, }); throw txErr; // Re-throw to be caught by outer catch }Also applies to: 135-147
🧹 Nitpick comments (16)
apps/dashboard/app/(app)/projects/_components/list/index.tsx (3)
15-21: Skip the WHERE when search is empty to avoid a no-op ILIKE.Avoid
ILIKE '%%'to reduce unnecessary filtering and keep the planner free to use order-only indexes.- const projects = useLiveQuery( - (q) => - q - .from({ project: collection.projects }) - .orderBy(({ project }) => project.updatedAt, "desc") - .where(({ project }) => ilike(project.name, `%${projectName}%`)), - [projectName], - ); + const projects = useLiveQuery( + (q) => { + let base = q + .from({ project: collection.projects }) + .orderBy(({ project }) => project.updatedAt, "desc"); + if (projectName.trim() !== "") { + base = base.where(({ project }) => ilike(project.name, `%${projectName}%`)); + } + return base; + }, + [projectName], + );
20-20: Escape LIKE wildcards in user input.Unescaped
%,_, or\inprojectNamewill change match semantics.- .where(({ project }) => ilike(project.name, `%${projectName}%`)), + .where(({ project }) => ilike(project.name, `%${escapeLike(projectName)}%`)),Add this helper near the top of the file:
function escapeLike(input: string) { // Escape backslash first, then % and _ return input.replace(/([\\%_])/g, "\\$1"); }
20-20: Consider a trigram/CITEXT index for case-insensitive contains.ILIKE '%…%' won't use a plain btree index. If this runs on Postgres and the dataset grows, add a GIN trigram index (or CITEXT + trigram) on projects.name to keep searches fast.
Example:
- CREATE EXTENSION IF NOT EXISTS pg_trgm;
- CREATE INDEX CONCURRENTLY IF NOT EXISTS projects_name_trgm_idx ON projects USING gin (name gin_trgm_ops);
internal/db/src/schema/environments.ts (2)
15-15: Fix stale comment (“within workspace” → “within project”)The slug is now project‑scoped.
- slug: varchar("slug", { length: 256 }).notNull(), // URL-safe identifier within workspace + slug: varchar("slug", { length: 256 }).notNull(), // URL-safe identifier within project
21-23: Consider add’l index if querying by workspaceIdIf envs are commonly listed by workspace, add an index on workspaceId (or composite workspaceId+projectId).
-import { mysqlTable, uniqueIndex, varchar } from "drizzle-orm/mysql-core"; +import { mysqlTable, uniqueIndex, varchar, index } from "drizzle-orm/mysql-core"; ... (table) => ({ uniqueSlug: uniqueIndex("environments_project_id_slug_idx").on(table.projectId, table.slug), + workspaceIdx: index("workspace_idx").on(table.workspaceId), }),apps/dashboard/lib/collections/projects.ts (2)
39-40: Tiny nit: drop unnecessary async/awaitReturn the promise directly for parity with other collections.
- queryFn: async () => { - return await trpcClient.deploy.project.list.query(); - }, + queryFn: () => trpcClient.deploy.project.list.query(),
45-49: Normalize empty gitRepositoryUrl to null (optional)Prevents sending empty strings if the backend prefers null.
- const createInput = createProjectRequestSchema.parse({ + const createInput = createProjectRequestSchema.parse({ name: changes.name, slug: changes.slug, - gitRepositoryUrl: changes.gitRepositoryUrl, + gitRepositoryUrl: changes.gitRepositoryUrl === "" ? null : changes.gitRepositoryUrl, });apps/dashboard/lib/collections/domains.ts (1)
21-21: Namespace switch OK — domain.list is workspace‑scoped; no extra params required.Server proc apps/dashboard/lib/trpc/routers/deploy/domains/list.ts uses requireWorkspace and filters by ctx.workspace.id, so trpcClient.deploy.domain.list.query() does not need workspaceId/projectId inputs. Optional: scope the cache key in apps/dashboard/lib/collections/domains.ts (currently queryKey: ["domains"]) to include workspaceId (e.g. ["domains", workspaceId]) to avoid cross‑workspace cache collisions.
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx (1)
10-11: Temporary endpoint name (list_dummy)—track and gate.If this is mock-only, gate behind a flag or TODO with an issue reference, and plan the rename to
listto minimize churn.Proposed TODO:
- const { data } = trpc.deploy.environment.list_dummy.useQuery({ projectId }); + // TODO(unkey#xxxx): replace list_dummy with list once backend is wired to DB + const { data } = trpc.deploy.environment.list_dummy.useQuery({ projectId });apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx (1)
62-71: Validate slug when auto‑updating it from name.
setValue doesn’t validate by default; isValid may lag. Trigger validation to keep the submit button state accurate.- setValue("slug", slug); + setValue("slug", slug, { shouldValidate: true });apps/dashboard/lib/trpc/routers/index.ts (1)
320-321: Rename list_dummy to something stable before publicizing.
list_dummy suggests placeholder; consider listVariables or getEnvs for consistency.apps/dashboard/components/list-search-input.tsx (5)
21-26: Remove dead prop from the public type.
debounceTimeis no longer used or destructured; keep the API clean to avoid confusion.Apply this diff:
type Props<T extends BaseFilter = BaseFilter> = { useFiltersHook: () => FilterHook<T>; placeholder?: string; - debounceTime?: number; className?: string; };
39-39: Delete leftover debounce plumbing.No timeout is scheduled anymore; the ref and clearTimeouts add noise and maintenance overhead.
Apply this diff:
- const debounceRef = useRef<NodeJS.Timeout>(); + // no debounce; update on each keystroke - // Cleanup debounce on unmount - useEffect(() => { - return () => { - if (debounceRef.current) { - clearTimeout(debounceRef.current); - } - }; - }, []); + // no debounce cleanup needed const handleInputChange = (e: React.ChangeEvent<HTMLInputElement>) => { const value = e.target.value; setSearchText(value); - - // Clear existing debounce - if (debounceRef.current) { - clearTimeout(debounceRef.current); - } - updateQuery(value); }; const handleClear = () => { setSearchText(""); - - // Clear debounce - if (debounceRef.current) { - clearTimeout(debounceRef.current); - } - // Immediately update filters updateQuery(""); }; const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => { if (e.key === "Escape") { handleClear(); inputRef.current?.blur(); } if (e.key === "Enter") { - // Clear debounce and immediately update - if (debounceRef.current) { - clearTimeout(debounceRef.current); - } updateQuery(searchText); } };Also applies to: 60-68, 94-98, 105-109, 121-125
69-88: Preserve a stable filter id for the query chip.Creating a new random id on every keystroke can cause unnecessary re-renders or chip remounts. Reuse the existing id if present.
Apply this diff:
- const updateQuery = (value: string) => { - // Remove existing filters for query field - const filtersWithoutCurrent = filters.filter((f) => f.field !== "query"); - - if (value.trim()) { - // Add new filter - updateFilters([ - ...filtersWithoutCurrent, - { - field: "query", - id: crypto.randomUUID(), - operator: "contains", - value: value.trim(), - } as T, - ]); - } else { - // Just remove query filters if empty - updateFilters(filtersWithoutCurrent); - } - }; + const updateQuery = (value: string) => { + const trimmed = value.trim(); + const existing = filters.find((f) => f.field === "query"); + const id = existing?.id ?? crypto.randomUUID(); + const filtersWithoutCurrent = filters.filter((f) => f.field !== "query"); + + if (trimmed) { + updateFilters([ + ...filtersWithoutCurrent, + { + field: "query", + id, + operator: "contains", + value: trimmed, + } as T, + ]); + } else { + updateFilters(filtersWithoutCurrent); + } + };
153-153: HonorclassNamein the non-loading state too.It’s applied in the loading skeleton wrapper but ignored in the normal render.
Apply this diff:
- return ( - <div className="flex items-center gap-2 w-full flex-1 md:w-80 h-8"> + return ( + <div className={cn("flex items-center gap-2 w-full flex-1 md:w-80 h-8", className)}>
114-127: Avoid redundant Enter-triggered updates.Since updates happen on each keystroke, pressing Enter can still fire another state update (and potential network call). Guard for no-op.
Apply this diff:
if (e.key === "Enter") { - updateQuery(searchText); + const current = filters.find((f) => f.field === "query"); + if ((current?.value ?? "") !== searchText.trim()) { + updateQuery(searchText); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/controls/components/deployment-list-search/index.tsx(1 hunks)apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/hooks/use-deployment-logs.tsx(1 hunks)apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx(1 hunks)apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/page.tsx(1 hunks)apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx(5 hunks)apps/dashboard/app/(app)/projects/_components/create-project/create-project.schema.ts(0 hunks)apps/dashboard/app/(app)/projects/_components/create-project/use-create-project.ts(0 hunks)apps/dashboard/app/(app)/projects/_components/list/index.tsx(1 hunks)apps/dashboard/components/list-search-input.tsx(1 hunks)apps/dashboard/lib/collections/deployments.ts(1 hunks)apps/dashboard/lib/collections/domains.ts(1 hunks)apps/dashboard/lib/collections/environments.ts(1 hunks)apps/dashboard/lib/collections/projects.ts(1 hunks)apps/dashboard/lib/trpc/routers/deploy/deployment/getById.ts(1 hunks)apps/dashboard/lib/trpc/routers/deploy/deployment/getOpenApiDiff.ts(1 hunks)apps/dashboard/lib/trpc/routers/deploy/deployment/index.ts(1 hunks)apps/dashboard/lib/trpc/routers/deploy/deployment/list.ts(1 hunks)apps/dashboard/lib/trpc/routers/deploy/project/create.ts(3 hunks)apps/dashboard/lib/trpc/routers/deployment/listByEnvironment.ts(0 hunks)apps/dashboard/lib/trpc/routers/deployment/listByProject.ts(0 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)go/pkg/db/schema.sql(1 hunks)internal/db/src/schema/environments.ts(1 hunks)
💤 Files with no reviewable changes (4)
- apps/dashboard/lib/trpc/routers/deployment/listByEnvironment.ts
- apps/dashboard/app/(app)/projects/_components/create-project/use-create-project.ts
- apps/dashboard/lib/trpc/routers/deployment/listByProject.ts
- apps/dashboard/app/(app)/projects/_components/create-project/create-project.schema.ts
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Learnt from: perkinsjr
PR: unkeyed/unkey#3775
File: apps/dashboard/lib/trpc/routers/api/keys/query-overview-timeseries/index.ts:1-2
Timestamp: 2025-08-22T12:50:07.024Z
Learning: The team at Unkey is comfortable accepting TRPC schema imports from app route folders in server-side TRPC routers as technical debt, since they're planning to migrate away from TRPC to calling their v2 API directly, making these imports temporary and not worth refactoring.
Learnt from: perkinsjr
PR: unkeyed/unkey#3775
File: apps/dashboard/lib/trpc/routers/workspace/onboarding.ts:1-1
Timestamp: 2025-08-22T12:45:07.187Z
Learning: The team at Unkey is planning to move from TRPC to calling their v2 API directly, making current TRPC schema organization temporary. They're comfortable with keeping server input schemas imported from app route folders as technical debt since this code will be replaced.
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/hooks/use-deployment-logs.tsxapps/dashboard/lib/trpc/routers/deploy/deployment/getOpenApiDiff.tsapps/dashboard/lib/collections/deployments.tsapps/dashboard/lib/collections/environments.tsapps/dashboard/app/(app)/projects/[projectId]/deployments/components/controls/components/deployment-list-search/index.tsxapps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/page.tsxapps/dashboard/lib/collections/domains.tsapps/dashboard/lib/trpc/routers/deploy/deployment/index.tsapps/dashboard/lib/trpc/routers/deploy/project/create.tsapps/dashboard/lib/trpc/routers/index.tsapps/dashboard/lib/trpc/routers/deploy/deployment/list.tsapps/dashboard/lib/trpc/routers/deploy/deployment/getById.ts
📚 Learning: 2025-07-25T19:11:00.208Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/getOpenApiDiff.ts:110-147
Timestamp: 2025-07-25T19:11:00.208Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/getOpenApiDiff.ts, the user mcstepp prefers to keep mock data fallbacks in POC/demonstration code for simplicity, even if it wouldn't be production-ready. This aligns with the PR being work-in-progress for demonstration purposes.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/hooks/use-deployment-logs.tsxapps/dashboard/lib/trpc/routers/deploy/deployment/getOpenApiDiff.tsapps/dashboard/lib/collections/deployments.tsapps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/page.tsxapps/dashboard/lib/trpc/routers/deploy/deployment/index.tsapps/dashboard/lib/trpc/routers/index.tsapps/dashboard/lib/trpc/routers/deploy/deployment/list.tsapps/dashboard/lib/trpc/routers/deploy/deployment/getById.ts
📚 Learning: 2025-08-31T19:30:38.171Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3853
File: apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts:75-87
Timestamp: 2025-08-31T19:30:38.171Z
Learning: In apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts, the getEnvs procedure currently uses mock data (VARIABLES) and ignores the projectId input parameter. This is intentional temporary behavior - the user ogzhanolguncu indicated they plan to hook this up to the database later.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsxapps/dashboard/lib/collections/environments.tsapps/dashboard/lib/trpc/routers/deploy/deployment/index.ts
📚 Learning: 2025-06-02T11:09:58.791Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3292
File: apps/dashboard/lib/trpc/routers/key/create.ts:11-14
Timestamp: 2025-06-02T11:09:58.791Z
Learning: In the unkey codebase, TypeScript and the env() function implementation already provide sufficient validation for environment variables, so additional runtime error handling for missing env vars is not needed.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx
📚 Learning: 2025-07-28T20:38:53.244Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/components/client.tsx:322-341
Timestamp: 2025-07-28T20:38:53.244Z
Learning: In apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/components/client.tsx, mcstepp prefers to keep hardcoded endpoint logic in the getDiffType function during POC phases for demonstrating diff functionality, rather than implementing a generic diff algorithm. This follows the pattern of keeping simplified implementations for demonstration purposes.
Applied to files:
apps/dashboard/lib/trpc/routers/deploy/deployment/getOpenApiDiff.tsapps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/page.tsx
📚 Learning: 2024-12-03T14:23:07.189Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2143
File: apps/dashboard/app/(app)/logs/components/log-details/resizable-panel.tsx:37-49
Timestamp: 2024-12-03T14:23:07.189Z
Learning: In `apps/dashboard/app/(app)/logs/components/log-details/resizable-panel.tsx`, the resize handler is already debounced.
Applied to files:
apps/dashboard/components/list-search-input.tsx
📚 Learning: 2025-08-22T12:50:07.024Z
Learnt from: perkinsjr
PR: unkeyed/unkey#3775
File: apps/dashboard/lib/trpc/routers/api/keys/query-overview-timeseries/index.ts:1-2
Timestamp: 2025-08-22T12:50:07.024Z
Learning: The team at Unkey is comfortable accepting TRPC schema imports from app route folders in server-side TRPC routers as technical debt, since they're planning to migrate away from TRPC to calling their v2 API directly, making these imports temporary and not worth refactoring.
Applied to files:
apps/dashboard/lib/trpc/routers/index.ts
🧬 Code graph analysis (12)
apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/hooks/use-deployment-logs.tsx (1)
apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)
apps/dashboard/app/(app)/projects/[projectId]/details/env-variables-section/hooks/use-env-var-manager.tsx (1)
apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)
apps/dashboard/lib/collections/deployments.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
apps/dashboard/lib/collections/environments.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/controls/components/deployment-list-search/index.tsx (1)
apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)
apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/page.tsx (1)
apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)
apps/dashboard/lib/collections/domains.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
apps/dashboard/lib/trpc/routers/deploy/project/create.ts (1)
apps/dashboard/lib/collections/projects.ts (1)
createProjectRequestSchema(16-28)
apps/dashboard/lib/trpc/routers/index.ts (9)
apps/dashboard/lib/trpc/routers/deploy/project/list.ts (1)
listProjects(5-30)apps/dashboard/lib/trpc/routers/deploy/project/create.ts (1)
createProject(8-177)apps/dashboard/lib/trpc/routers/deploy/envs/list.ts (1)
getEnvs(68-80)apps/dashboard/lib/trpc/routers/environment/list.ts (1)
listEnvironments(5-24)apps/dashboard/lib/trpc/routers/deploy/domains/list.ts (1)
listDomains(5-28)apps/dashboard/lib/trpc/routers/deploy/deployment/list.ts (1)
listDeployments(5-37)apps/dashboard/lib/trpc/routers/deploy/deployment/llm-search/index.ts (1)
searchDeployments(13-20)apps/dashboard/lib/trpc/routers/deploy/deployment/getOpenApiDiff.ts (1)
getOpenApiDiff(7-196)apps/dashboard/lib/trpc/routers/deploy/deployment/buildLogs.ts (1)
getDeploymentBuildLogs(217-230)
apps/dashboard/lib/collections/projects.ts (2)
internal/db/src/schema/projects.ts (1)
projects(8-31)apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
apps/dashboard/app/(app)/projects/_components/list/index.tsx (2)
apps/dashboard/app/(app)/projects/_components/hooks/use-projects-filters.ts (1)
useProjectsFilters(17-103)apps/dashboard/lib/collections/index.ts (1)
collection(17-24)
apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx (2)
apps/dashboard/lib/collections/projects.ts (2)
CreateProjectRequestSchema(31-31)createProjectRequestSchema(16-28)apps/dashboard/lib/collections/index.ts (2)
collection(17-24)reset(27-34)
🔇 Additional comments (19)
apps/dashboard/app/(app)/projects/_components/list/index.tsx (2)
2-2: LGTM: correct imports for ilike/useLiveQuery.
13-14: Confirmed: filter field key is "query".
projectsFilterFieldConfig defines a "query" field and projectsListFilterFieldNames exports it (apps/dashboard/app/(app)/projects/_components/projects-filters.schema.ts), so the code will correctly read the query filter.internal/db/src/schema/environments.ts (1)
22-22: Index scope change LGTMunique(projectId, slug) aligns with the PR goal and generated SQL.
Please confirm a matching migration exists (drop old workspace_id index, create new project_id index).
apps/dashboard/lib/collections/deployments.ts (1)
45-45: Deployments: confirm trpc route and include env/project filters in call & queryKeySearch returned no matches; verify that trpcClient.deploy.deployment.list exists and whether it accepts { environmentId, projectId }. If it does, pass those params to queryFn and include them in the queryKey for correct caching (e.g. queryFn: () => trpcClient.deploy.deployment.list.query({ environmentId, projectId }), queryKey: ['deployments', projectId, environmentId]). Location: apps/dashboard/lib/collections/deployments.ts — current line: queryFn: () => trpcClient.deploy.deployment.list.query().
apps/dashboard/lib/collections/projects.ts (1)
16-28: Server imports shared schema; confirm gitRepositoryUrl normalizationcreateProjectRequestSchema is imported by the server create route (apps/dashboard/lib/trpc/routers/deploy/project/create.ts) and used by the client form (apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx). No code was found that converts an empty gitRepositoryUrl to null (apps/dashboard/lib/collections/projects.ts uses createProjectRequestSchema.parse but contains no explicit ''→null normalization). If the server normalizes empty string → null, mirror that client-side before submit; otherwise no change required.
apps/dashboard/lib/trpc/routers/deploy/deployment/getById.ts (1)
2-2: Alias import switch looks good.Consistent with the deploy namespace reorg.
apps/dashboard/lib/trpc/routers/deploy/deployment/getOpenApiDiff.ts (1)
3-3: Alias import switch LGTM.apps/dashboard/lib/trpc/routers/deploy/deployment/list.ts (1)
2-2: Alias import switch LGTM.apps/dashboard/app/(app)/projects/[projectId]/deployments/components/controls/components/deployment-list-search/index.tsx (1)
9-10: Mutation path update looks correct.No behavior change; UI callbacks intact.
apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/hooks/use-deployment-logs.tsx (1)
53-55: Ensure router export exists and guard query when id is empty.Add { enabled: Boolean(deploymentId) } to the useQuery to avoid calls with falsy ids and confirm deploy.deployment.buildLogs is exported on the server router — search returned no matches in apps/dashboard/lib/trpc/routers.
- const { data: logsData, isLoading } = trpc.deploy.deployment.buildLogs.useQuery({ - deploymentId, - }); + const { data: logsData, isLoading } = trpc.deploy.deployment.buildLogs.useQuery( + { deploymentId }, + { enabled: Boolean(deploymentId) }, + );Location: apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/hooks/use-deployment-logs.tsx:53-55. Check server routers under apps/dashboard/lib/trpc/routers for the buildLogs export.
apps/dashboard/lib/trpc/routers/deploy/deployment/index.ts (1)
1-10: Router surface changed — removed procedures aren’t used. No occurrences of listByEnvironment or listByProject were found; the deploy root is mounted at apps/dashboard/lib/trpc/routers/index.ts:314 and the deployment router exposes list (listDeployments), search, and getOpenApiDiff.apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/page.tsx (1)
50-58: Namespace update reads right. Hook path aligns with deploy router. Ran a repo-wide regex search for trpc.deployment.(getOpenApiDiff|list|search|buildLogs); no matches found — manual verification recommended for aliased/indirect references.apps/dashboard/lib/trpc/routers/deploy/project/create.ts (3)
2-12: Input schema swap looks correct and aligns with shared collection schema.
Using createProjectRequestSchema keeps client/server validation in sync.
102-111: No DB migration needed — environments already enforce (projectId, slug) uniqueness. Schema defines uniqueIndex("environments_project_id_slug_idx").on(table.projectId, table.slug) in internal/db/src/schema/environments.ts.
63-79: No change required — schema uses numeric timestamps (bigint, mode: "number").createdAt/updatedAt are defined in internal/db/src/schema/util/lifecycle_dates.ts as bigint(..., { mode: "number" }) with $defaultFn/$onUpdateFn using Date.now(), so assigning Date.now() (ms number) in create.ts is correct. Also applies to the other mentioned lines (149–155).
Likely an incorrect or invalid review comment.
apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx (1)
100-103: Button state ties to isSubmitting—now correct once insert is awaited.
After the await fix, disabled/loading behavior will reflect the real request lifecycle.apps/dashboard/lib/trpc/routers/index.ts (2)
314-332: Deploy namespace wiring looks good and consistent.
project/environment/domain/deployment nested routes are clear; good consolidation.
314-332: Sweep callers for old trpc paths and migrate to trpc.deployEnsure all usages of trpc.(project|deployment|environment|domain).* are updated to trpc.deploy.* to avoid runtime breaks. Automated scan in this environment skipped files — run these locally and fix any matches:
# search dotted and bracketed usages rg -nP -S -uu -C2 '\btrpc\.(project|deployment|environment|domain)\.' || true rg -nP -S -uu -C2 "trpc\[['\"](?:project|deployment|environment|domain)['\"]\]" || true # verify new path and any env var callers rg -nP -S -uu -C2 '\btrpc\.deploy\b' || true rg -nP -S -uu -C2 '\benvironmentVariables\b' || trueapps/dashboard/components/list-search-input.tsx (1)
31-36: Ensure ListSearchInput is client-only (hooks require client runtime)Automated search found no importers; confirm this component is only imported from Client Components — either add "use client" at the top of apps/dashboard/components/list-search-input.tsx or ensure every importing file has "use client" in its first lines.
apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx
Show resolved
Hide resolved
|
@perkinsjr @chronark @mcstepp Folks can I get a quick review for this? I wanna start fresh tomorrow |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx (1)
36-47: Awaiting persistence looks good; confirms previous concern is addressedSwitching to await tx.isPersisted.promise prevents premature modal close and ensures errors bubble to the catch block. Please confirm that DuplicateKeyError is raised through isPersisted.promise in your collection layer.
#!/bin/bash # Verify where DuplicateKeyError is thrown/rethrown so the catch will see it. rg -n -C2 'DuplicateKeyError|duplicate key|unique constraint' --ignore-case
🧹 Nitpick comments (2)
apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx (2)
41-45: Trim DB-managed fields from insert payloadAvoid sending server-managed fields (activeDeploymentId, updatedAt, id) from the client to reduce coupling and future breakage.
- const tx = collection.projects.insert({ - name: values.name, - slug: values.slug, - gitRepositoryUrl: values.gitRepositoryUrl || null, - activeDeploymentId: null, - updatedAt: null, - id: "will-be-replace-by-server", - }); + const tx = collection.projects.insert({ + name: values.name, + slug: values.slug, + gitRepositoryUrl: values.gitRepositoryUrl || null, + });
23-27: Don’t overwrite a hand-edited slug; validate slug when auto-updatingOnly auto-generate the slug while the slug field hasn’t been touched, and ensure setValue triggers validation so isValid updates immediately.
- setError, - reset, - formState: { errors, isSubmitting, isValid }, + setError, + reset, + formState: { errors, isSubmitting, isValid, dirtyFields },- const handleNameChange = (e: React.ChangeEvent<HTMLInputElement>) => { + const handleNameChange = (e: React.ChangeEvent<HTMLInputElement>) => { const name = e.target.value; const slug = name .toLowerCase() .replace(/[^a-z0-9\s-]/g, "") .replace(/\s+/g, "-") .replace(/-+/g, "-") .replace(/^-|-$/g, ""); - setValue("slug", slug); + if (!dirtyFields?.slug) { + setValue("slug", slug, { shouldDirty: true, shouldTouch: true, shouldValidate: true }); + } };Also applies to: 63-72
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.436Z
Learning: Repo: unkeyed/unkey — Preference: If imeyer comments “issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Learnt from: perkinsjr
PR: unkeyed/unkey#3775
File: apps/dashboard/lib/trpc/routers/workspace/onboarding.ts:1-1
Timestamp: 2025-08-22T12:45:07.187Z
Learning: The team at Unkey is planning to move from TRPC to calling their v2 API directly, making current TRPC schema organization temporary. They're comfortable with keeping server input schemas imported from app route folders as technical debt since this code will be replaced.
Learnt from: perkinsjr
PR: unkeyed/unkey#3775
File: apps/dashboard/lib/trpc/routers/api/keys/query-overview-timeseries/index.ts:1-2
Timestamp: 2025-08-22T12:50:07.024Z
Learning: The team at Unkey is comfortable accepting TRPC schema imports from app route folders in server-side TRPC routers as technical debt, since they're planning to migrate away from TRPC to calling their v2 API directly, making these imports temporary and not worth refactoring.
🧬 Code graph analysis (1)
apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx (2)
apps/dashboard/lib/collections/projects.ts (2)
CreateProjectRequestSchema(31-31)createProjectRequestSchema(16-28)apps/dashboard/lib/collections/index.ts (2)
collection(17-24)reset(27-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test Packages / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
🔇 Additional comments (2)
apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx (2)
102-103: Good submit gatingdisabled={isSubmitting || !isValid} and loading={isSubmitting} correctly prevent double submissions and reflect async state.
10-10: Confirm DuplicateKeyError constructor identity to avoid instanceof false negativesYou import DuplicateKeyError from "@tanstack/react-db" and use instanceof checks — confirm that errors thrown by collection code come from that same constructor (no duplicate package/module copies) so instanceof will work at runtime.
Locations:
- Imports: apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx:10; apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx:6; apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/identifier-dialog.tsx:5
- throw site: apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/identifier-dialog.tsx:74
- instanceof checks: apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx:51; apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx:53; apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/identifier-dialog.tsx:88
|
I would, but I'm already resolving merge conflict hell from one of my open PRs that I'm trying to get merged soon too, and I fear this would add to it. |
@mcstepp I agree lets first merge yours |
chronark
left a comment
chronark
left a comment
There was a problem hiding this comment.
I'm confused, you asked about the nested trpc routes here, the feedback was that we don't need to nest them. and then you do exactly that in this PR?
I don't necessarily care about the nesting as much, but why are you asking if you disregard it silently afterwards?
I think we had a misunderstanding. I don't mind keeping everything flat for actual router like |
|
I migrated the schema in planetsacle |
:ack: |
|
@chronark I'm sending one more minor fix then i'll let you folks merge 🫡 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go/apps/ctrl/services/deployment/deploy_workflow.go (1)
169-171: Compile-time bug: invalid range over integer
for i := range 300is not valid Go and will not compile. Use a classic for loop.- for i := range 300 { + for i := 0; i < 300; i++ {
🧹 Nitpick comments (1)
go/apps/ctrl/services/deployment/deploy_workflow.go (1)
384-393: Avoid leaking/overwriting outer err; reuse completionTime; add success logInside the hydra step, assigning to the outer-scoped err is brittle and hard to read. Prefer a locally scoped variable. Also, reuse completionTime to keep timestamps consistent within the step and consider logging success.
- // TODO: This section will be removed in the future in favor of "Promote to Production" - err = db.Query.UpdateProjectActiveDeploymentId(stepCtx, w.db.RW(), db.UpdateProjectActiveDeploymentIdParams{ - ID: req.ProjectID, - ActiveDeploymentID: sql.NullString{Valid: true, String: req.DeploymentID}, - UpdatedAt: sql.NullInt64{Valid: true, Int64: time.Now().UnixMilli()}, - }) - if err != nil { - return nil, fmt.Errorf("failed to update project %s active deployment ID to %s: %w", req.ProjectID, req.DeploymentID, err) - } + // TODO: This will be removed in favor of an explicit "Promote to Production" flow + if projUpdateErr := db.Query.UpdateProjectActiveDeploymentId( + stepCtx, + w.db.RW(), + db.UpdateProjectActiveDeploymentIdParams{ + ID: req.ProjectID, + ActiveDeploymentID: sql.NullString{Valid: true, String: req.DeploymentID}, + UpdatedAt: sql.NullInt64{Valid: true, Int64: completionTime}, + }, + ); projUpdateErr != nil { + return nil, fmt.Errorf("failed to update project %s active deployment ID to %s: %w", req.ProjectID, req.DeploymentID, projUpdateErr) + } + w.logger.Info("project active deployment updated", "project_id", req.ProjectID, "deployment_id", req.DeploymentID)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
go/apps/ctrl/services/deployment/deploy_workflow.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Learnt from: perkinsjr
PR: unkeyed/unkey#3775
File: apps/dashboard/lib/trpc/routers/api/keys/query-overview-timeseries/index.ts:1-2
Timestamp: 2025-08-22T12:50:07.024Z
Learning: The team at Unkey is comfortable accepting TRPC schema imports from app route folders in server-side TRPC routers as technical debt, since they're planning to migrate away from TRPC to calling their v2 API directly, making these imports temporary and not worth refactoring.
Learnt from: perkinsjr
PR: unkeyed/unkey#3775
File: apps/dashboard/lib/trpc/routers/workspace/onboarding.ts:1-1
Timestamp: 2025-08-22T12:45:07.187Z
Learning: The team at Unkey is planning to move from TRPC to calling their v2 API directly, making current TRPC schema organization temporary. They're comfortable with keeping server input schemas imported from app route folders as technical debt since this code will be replaced.
🧬 Code graph analysis (1)
go/apps/ctrl/services/deployment/deploy_workflow.go (2)
go/pkg/db/project_update_active_deployment_id.sql_generated.go (1)
UpdateProjectActiveDeploymentIdParams(19-23)go/pkg/db/types/null_string.go (1)
NullString(10-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Packages / Test
- GitHub Check: Build / Build
- GitHub Check: Test Go API Local / Test
|
sure |
|
can you do me a favour that captures the meaning better |
|
might as well do it here since we do db changes |
|
@chronark we are good to go. Could you please double check my last commit to see if we are missing anything? |
|
on it |
|
lemme fix it real quick |
|
@chronark https://github.com/unkeyed/unkey/pull/3973/commits/6081ff31d3a86a0d8d57092aa01e55f3460b3f09#diff-674f5a38d8c562257c213[…]abd43692d8b8314716dc471c109 this looks good here? did you accidentally linked wrong thing maybe? |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/app/(app)/projects/[projectId]/diff/page.tsx (1)
22-29: Avoid mutating query data; use a copy and the already-captured projectId.
- Use the
projectIdvariable (already typed) in the where-clause.- Don’t call
.sortondeployments.datadirectly; copy before sorting to avoid mutating cached data.Apply:
- .from({ deployment: collections.deployments }) - .where(({ deployment }) => eq(deployment.projectId, params?.projectId)) + .from({ deployment: collections.deployments }) + .where(({ deployment }) => eq(deployment.projectId, projectId))And below:
- const sortedDeployments = deployments.data.sort( + const sortedDeployments = [...deployments.data].sort( (a, b) => b.deployment.createdAt - a.deployment.createdAt, );
♻️ Duplicate comments (2)
go/apps/ctrl/services/deployment/deploy_workflow.go (1)
384-393: Unconditional “promote to live” risks flipping production on every ready deploy. Gate or remove now.This auto-writes projects.live_deployment_id for every deployment; preview/staging or concurrent deploys can hijack “live.”
Apply one of:
- Remove auto-promotion until an explicit “Promote to Production” flow exists.
- Or gate by environment (needs env in DeployRequest).
Minimal safe fix (remove for now):
- // TODO: This section will be removed in the future in favor of "Promote to Production" - err = db.Query.UpdateProjectLiveDeploymentId(stepCtx, w.db.RW(), db.UpdateProjectLiveDeploymentIdParams{ - ID: req.ProjectID, - LiveDeploymentID: sql.NullString{Valid: true, String: req.DeploymentID}, - UpdatedAt: sql.NullInt64{Valid: true, Int64: time.Now().UnixMilli()}, - }) - if err != nil { - return nil, fmt.Errorf("failed to update project %s active deployment ID to %s: %w", req.ProjectID, req.DeploymentID, err) - } + // Intentionally not auto-promoting to live; promotion is an explicit action.If you prefer gating, extend DeployRequest with EnvironmentID and only promote when env == production.
apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx (1)
46-49: Resolved: persistence is awaited before closing the modal.Awaiting
tx.isPersisted.promiseaddresses the prior “insert not awaited” bug; UI no longer closes early and errors are catchable.
🧹 Nitpick comments (30)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/controls/components/deployment-list-datetime/index.tsx (4)
67-69: Type-safety nit: avoid passingsetStatedirectly.Depending on prop typing,
onSuggestionChange={(t) => setTitle(t)}is safer than handingsetTitlethrough (avoids contravariant callback issues).- initialTitle={title ?? ""} - onSuggestionChange={setTitle} + initialTitle={title} + onSuggestionChange={(t) => setTitle(t)}
75-78: Brittle style toggle based on string equality.Checking
title !== "Last 12 hours"couples styling to wording. Base it on underlying filters instead.Add this near where
timeValuesis defined:const isDefaultRange = timeValues?.since === "12h" && !timeValues?.startTime && !timeValues?.endTime;Then update the class toggle:
- title !== "Last 12 hours" ? "bg-gray-4" : "", + isDefaultRange ? "" : "bg-gray-4",
79-83: A11y/text accuracy: label mentions “logs” in a deployments view.Update copy to reflect deployments and avoid confusion. Also, only keep the keyboard-hint title if 'T' is actually bound here.
- aria-label="Filter logs by time" + aria-label="Filter deployments by time" @@ - title="Press 'T' to toggle filters" + title="Press 'T' to toggle date/time filters"If there is no 'T' hotkey for this control, drop the
titleattribute entirely.
82-86: Remove the disabled/loading phase.Once the title is derived from filters, keeping the button disabled and showing “Loading...” is unnecessary and harms UX.
- disabled={!title} + // Keep interactive; label is always derived from filters + disabled={false} @@ - <span className="text-gray-12 font-medium text-[13px]">{title ?? "Loading..."}</span> + <span className="text-gray-12 font-medium text-[13px]">{title}</span>apps/dashboard/app/(app)/projects/[projectId]/deployments/components/controls/components/deployment-list-filters/components/environment-filter.tsx (1)
10-17: Guard for undefined data and use stable IDs.useLiveQuery may return undefined before hydrate; also prefer environment.id over array index for option id. Consider sorting for stable UX.
- const environments = useLiveQuery((q) => q.from({ environment: collections.environments })); + const environments = useLiveQuery((q) => q.from({ environment: collections.environments })); + const envs = (environments.data ?? []).slice().sort((a, b) => a.slug.localeCompare(b.slug)); ... - options={environments.data.map((environment, i) => ({ - id: i, - slug: environment.slug, - }))} + options={envs.map((environment) => ({ + id: environment.id, + slug: environment.slug, + }))}internal/db/src/schema/projects.ts (1)
21-21: Index liveDeploymentId for lookups.If you ever resolve project from a deployment quickly, add a non-unique index on liveDeploymentId.
(table) => ({ workspaceIdx: index("workspace_idx").on(table.workspaceId), workspaceSlugIdx: uniqueIndex("workspace_slug_idx").on(table.workspaceId, table.slug), + liveDeploymentIdx: index("projects_live_deployment_id_idx").on(table.liveDeploymentId), }),apps/dashboard/lib/trpc/routers/environment/list.ts (1)
18-21: Add deterministic ordering and ensure supporting index.Sort by slug for stable UI; consider an index on (project_id) or (workspace_id, project_id) to match this predicate.
- return await db.query.environments.findMany({ + return await db.query.environments.findMany({ where: and( eq(environments.workspaceId, ctx.workspace.id), eq(environments.projectId, input.projectId), ), + orderBy: (t, { asc }) => [asc(t.slug)], columns: { id: true, projectId: true, slug: true, }, });To back the query, add in internal/db/src/schema/environments.ts:
(table) => ({ uniqueSlug: uniqueIndex("environments_project_id_slug_idx").on(table.projectId, table.slug), + projectIdx: index("environments_project_id_idx").on(table.projectId), + workspaceProjectIdx: index("environments_workspace_project_id_idx").on(table.workspaceId, table.projectId), }),apps/dashboard/lib/trpc/routers/deploy/domains/list.ts (1)
14-16: Add ordering by domain for stable UI; ensure index on projectId.Sorting improves UX; an index on domains.project_id (and/or workspace_id, project_id) will support the filter.
- where: (table, { eq, and }) => - and(eq(table.workspaceId, ctx.workspace.id), eq(table.projectId, input.projectId)), + where: (table, { eq, and }) => + and(eq(table.workspaceId, ctx.workspace.id), eq(table.projectId, input.projectId)), + orderBy: (t, { asc }) => [asc(t.domain)],Schema suggestion (internal DB):
// domains table indexes +domainsProjectIdx: index("domains_project_id_idx").on(table.projectId), +domainsWorkspaceProjectIdx: index("domains_workspace_project_id_idx").on(table.workspaceId, table.projectId),apps/dashboard/lib/trpc/routers/deploy/project/create.ts (4)
39-47: Normalize slug to lowercase server-side.Prevents case-related duplicates regardless of client behavior.
- const existingProject = await db.query.projects.findFirst({ + const slugNormalized = input.slug.trim().toLowerCase(); + const existingProject = await db.query.projects.findFirst({ where: (table, { eq, and }) => - and(eq(table.workspaceId, workspaceId), eq(table.slug, input.slug)), + and(eq(table.workspaceId, workspaceId), eq(table.slug, slugNormalized)),
66-79: Use normalized slug and trimmed repo URL in insert.Apply the same normalization during insert for consistency.
- await tx.insert(schema.projects).values({ + await tx.insert(schema.projects).values({ id: projectId, workspaceId, name: input.name, - slug: input.slug, + slug: slugNormalized, liveDeploymentId: null, - gitRepositoryUrl: input.gitRepositoryUrl || null, + gitRepositoryUrl: input.gitRepositoryUrl?.trim() || null, defaultBranch: "main", deleteProtection: false, createdAt: now, updatedAt: now, });
87-95: Reflect normalized slug in audit log.Keep logs consistent with stored values.
- description: `Created project "${input.name}" with slug "${input.slug}"`, + description: `Created project "${input.name}" with slug "${slugNormalized}"`,
135-147: Map duplicate-key to CONFLICT instead of 500.On rare races, insert may still hit unique constraint. Translate duplicate errors to TRPC CONFLICT.
- } catch (txErr) { + } catch (txErr) { console.error({ message: "Transaction failed during project creation", userId, workspaceId, projectId, projectSlug: input.slug, error: txErr instanceof Error ? txErr.message : String(txErr), stack: txErr instanceof Error ? txErr.stack : undefined, }); - - throw txErr; // Re-throw to be caught by outer catch + if (txErr instanceof Error && /duplicate|dup key|unique/i.test(txErr.message)) { + throw new TRPCError({ + code: "CONFLICT", + message: `A project with slug "${slugNormalized}" already exists in this workspace`, + }); + } + throw txErr; // Re-throw to be caught by outer catchapps/dashboard/lib/trpc/routers/deploy/project/list.ts (1)
10-21: Add deterministic ordering (updatedAt DESC) to avoid flicker/non‑stable pages.Limit without order can shuffle results between requests.
Example (Drizzle):
.findMany({ where: (table, { eq }) => eq(table.workspaceId, ctx.workspace.id), + orderBy: (table, { desc }) => [desc(table.updatedAt)], columns: {apps/dashboard/lib/collections/deploy/deployments.ts (2)
12-23: Don’t hardcode git metadata as non‑nullable — add a presence flag.Returning dummy values server‑side + non‑nullable client schema risks showing fake data as real.
Apply:
// Git information - // TEMP: Git fields as non-nullable for UI development with mock data - // TODO: Convert to nullable (.nullable()) when real git integration is added - // In production, deployments may not have git metadata if triggered manually - gitCommitSha: z.string(), - gitBranch: z.string(), - gitCommitMessage: z.string(), - gitCommitAuthorName: z.string(), - gitCommitAuthorEmail: z.string(), - gitCommitAuthorUsername: z.string(), - gitCommitAuthorAvatarUrl: z.string(), - gitCommitTimestamp: z.number().int(), + // In production, deployments may not have git metadata (manual triggers, etc.) + hasGitMetadata: z.boolean().default(false), + gitCommitSha: z.string().nullable(), + gitBranch: z.string().nullable(), + gitCommitMessage: z.string().nullable(), + gitCommitAuthorName: z.string().nullable(), + gitCommitAuthorEmail: z.string().nullable(), + gitCommitAuthorUsername: z.string().nullable(), + gitCommitAuthorAvatarUrl: z.string().nullable(), + gitCommitTimestamp: z.number().int().nullable(),Follow‑up: set hasGitMetadata server‑side when any git field is non‑null.
41-55: Stabilize cache identity and reduce refetch churn.Include a static prefix and consider a small staleTime.
- queryKey: [projectId, "deployments"], + queryKey: ["project", projectId, "deployments"], retry: 3, queryFn: () => trpcClient.deploy.deployment.list.query({ projectId }), getKey: (item) => item.id, id: `${projectId}-deployments`, + staleTime: 30_000,apps/dashboard/lib/trpc/routers/deploy/deployment/list.ts (1)
13-33: Order results by createdAt DESC and consider pagination.Current limit: 500 without order can yield non‑deterministic pages and heavy payloads for large projects.
- const deployments = await db.query.deployments.findMany({ + const deployments = await db.query.deployments.findMany({ where: (table, { eq, and }) => and(eq(table.workspaceId, ctx.workspace.id), eq(table.projectId, input.projectId)), + orderBy: (table, { desc }) => [desc(table.createdAt)], columns: {If needed soon, add cursor/offset inputs.
apps/dashboard/app/(app)/projects/[projectId]/layout.tsx (1)
30-39: Memoize per‑project collections to avoid re‑creating on each render.Prevents unnecessary re-renders and re-subscriptions downstream.
-import { useState } from "react"; +import { useMemo, useState } from "react"; ... - const collections = collectionManager.getProjectCollections(projectId); + const collections = useMemo( + () => collectionManager.getProjectCollections(projectId), + [projectId], + );Optional: also memoize the provider value.
go/apps/ctrl/services/deployment/deploy_workflow.go (1)
106-116: Nit: step label says “version” while updating a deployment.For consistency in logs/telemetry, rename to “update-deployment-building”.
- _, err = hydra.Step(ctx, "update-version-building", func(stepCtx context.Context) (*struct{}, error) { + _, err = hydra.Step(ctx, "update-deployment-building", func(stepCtx context.Context) (*struct{}, error) {Same for “update-version-deploying” below.
apps/dashboard/lib/collections/ratelimit/overrides.ts (1)
21-24: Remove noisy console.info in data fetch.This will spam consoles in production.
- console.info("DB fetching ratelimitOverrides");apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/index.tsx (3)
108-109: Clamp long commit messages to avoid overflow.- <div className="text-gray-9 text-xs">{deployment.gitCommitMessage}</div> + <div className="text-gray-9 text-xs truncate max-w-[38ch]"> + {deployment.gitCommitMessage} + </div>
121-124: Improve avatar alt text for a11y.- <Avatar src={deployment.gitCommitAuthorAvatarUrl} alt="Author" /> + <Avatar + src={deployment.gitCommitAuthorAvatarUrl} + alt={deployment.gitCommitAuthorName || "Commit author"} + />
145-151: Field shape check: confirm branch/sha fields; add safe fallbacks.If the canonical Deployment shape uses
source.branch/source.gitSha(per recent components), verify these top‑level fields exist; otherwise swap. Also add a fallback when missing.- <span className="text-grayA-9 text-xs truncate max-w-32"> - {deployment.gitBranch} + <span className="text-grayA-9 text-xs truncate max-w-32"> + {deployment.gitBranch ?? deployment.source?.branch ?? "unknown"} </span> @@ - <span className="text-grayA-9 text-xs"> - {(deployment.gitCommitSha ?? "").slice(0, 7)} + <span className="text-grayA-9 text-xs"> + {(deployment.gitCommitSha ?? deployment.source?.gitSha ?? "").slice(0, 7) || "—"} </span>apps/dashboard/lib/collections/deploy/domains.ts (1)
1-31: Solid per‑project domains collection; consider runtime validation of responses.Add Zod validation in
queryFnto catch server/shape drift early in clients.- queryFn: () => trpcClient.deploy.domain.list.query({ projectId }), + queryFn: async () => { + const data = await trpcClient.deploy.domain.list.query({ projectId }); + return z.array(schema).parse(data); + },go/apps/ctrl/services/routing/service.go (1)
361-365: Rename complete; fix log message and keep status checks consistent.
- Great switch to
UpdateProjectLiveDeploymentId.- Update the nearby error log to say “live deployment ID”.
- Prefer the enum constant for VM status everywhere for consistency.
- s.logger.ErrorContext(ctx, "failed to update project active deployment ID", + s.logger.ErrorContext(ctx, "failed to update project live deployment ID",Outside this hunk, also align VM status check:
- for _, vm := range vms { - if vm.Status == "running" { + for _, vm := range vms { + if vm.Status == partitiondb.VmsStatusRunning { gatewayConfig.Vms = append(gatewayConfig.Vms, &partitionv1.VM{ Id: vm.ID, }) } }apps/dashboard/lib/collections/deploy/environments.ts (1)
1-31: Per‑project environments collection — LGTM; optionally validate payloads.Mirror the domains collection suggestion to parse responses with Zod.
- queryFn: () => trpcClient.deploy.environment.list.query({ projectId }), + queryFn: async () => { + const data = await trpcClient.deploy.environment.list.query({ projectId }); + return z.array(schema).parse(data); + },apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx (3)
10-10: Handle TRPC conflict in UI and drop dead DuplicateKeyError path.Conflicts are surfaced by TRPC (err.data.code === "CONFLICT"), not
DuplicateKeyError. Set the slug field error on that condition; remove the unused import/branch.Apply this diff:
-import { DuplicateKeyError } from "@tanstack/react-db";- } catch (error) { - if (error instanceof DuplicateKeyError) { - setError("slug", { - type: "custom", - message: "Project with this slug already exists", - }); - } else { - console.error("Form submission error:", error); - // The collection's onInsert will handle showing error toasts - } - } + } catch (error: any) { + if (error?.data?.code === "CONFLICT") { + setError("slug", { + type: "custom", + message: "Project with this slug already exists", + }); + return; + } + console.error("Form submission error:", error); + // The collection's onInsert already surfaces a toast + }Also applies to: 50-59
38-45: Send only request-shaped fields on insert.
onInsertonly uses name/slug/gitRepositoryUrl; extra fields (liveDeploymentId,updatedAt, placeholderid) are unused noise.Apply this diff:
- const tx = collection.projects.insert({ - name: values.name, - slug: values.slug, - gitRepositoryUrl: values.gitRepositoryUrl || null, - liveDeploymentId: null, - updatedAt: null, - id: "will-be-replace-by-server", - }); + const tx = collection.projects.insert({ + name: values.name, + slug: values.slug, + gitRepositoryUrl: values.gitRepositoryUrl || null, + });
63-72: Re-validate slug when auto-updating from name.Without flags,
setValuewon’t trigger validation; users may not see updated error state.Apply this diff:
- setValue("slug", slug); + setValue("slug", slug, { shouldDirty: true, shouldValidate: true });apps/dashboard/lib/collections/deploy/projects.ts (1)
43-49: Don’t assumemutations[0]exists.Accessing
transaction.mutations[0]can throw if the array is empty or reordered by future changes. Guard or pick by intent (e.g., find the insert mutation) to make this resilient.Apply this defensive tweak:
- const { changes } = transaction.mutations[0]; + const first = transaction.mutations[0]; + if (!first) return; + const { changes } = first;apps/dashboard/lib/collections/index.ts (1)
17-22: Optional: renameprojectsin ProjectCollections to avoid confusion.Since this is the global/shared collection, a name like
sharedProjectsis clearer.Apply this diff (non-breaking if you update callsites):
-type ProjectCollections = { - environments: ReturnType<typeof createEnvironmentsCollection>; - domains: ReturnType<typeof createDomainsCollection>; - deployments: ReturnType<typeof createDeploymentsCollection>; - projects: typeof projects; -}; +type ProjectCollections = { + environments: ReturnType<typeof createEnvironmentsCollection>; + domains: ReturnType<typeof createDomainsCollection>; + deployments: ReturnType<typeof createDeploymentsCollection>; + sharedProjects: typeof projects; +}; @@ - this.projectCollections.set(projectId, { + this.projectCollections.set(projectId, { environments: createEnvironmentsCollection(projectId), domains: createDomainsCollection(projectId), deployments: createDeploymentsCollection(projectId), - projects, + sharedProjects: projects, });Also applies to: 32-37
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/controls/components/deployment-list-datetime/index.tsx(1 hunks)apps/dashboard/app/(app)/projects/[projectId]/deployments/components/controls/components/deployment-list-filters/components/environment-filter.tsx(1 hunks)apps/dashboard/app/(app)/projects/[projectId]/deployments/hooks/use-deployments.ts(1 hunks)apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/index.tsx(5 hunks)apps/dashboard/app/(app)/projects/[projectId]/details/project-details-expandables/index.tsx(2 hunks)apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/page.tsx(4 hunks)apps/dashboard/app/(app)/projects/[projectId]/diff/page.tsx(2 hunks)apps/dashboard/app/(app)/projects/[projectId]/layout-provider.tsx(1 hunks)apps/dashboard/app/(app)/projects/[projectId]/layout.tsx(2 hunks)apps/dashboard/app/(app)/projects/[projectId]/page.tsx(2 hunks)apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx(5 hunks)apps/dashboard/app/(app)/projects/_components/list/index.tsx(2 hunks)apps/dashboard/lib/collections/deploy/deployments.ts(1 hunks)apps/dashboard/lib/collections/deploy/domains.ts(1 hunks)apps/dashboard/lib/collections/deploy/environments.ts(1 hunks)apps/dashboard/lib/collections/deploy/projects.ts(1 hunks)apps/dashboard/lib/collections/domains.ts(0 hunks)apps/dashboard/lib/collections/environments.ts(0 hunks)apps/dashboard/lib/collections/index.ts(1 hunks)apps/dashboard/lib/collections/ratelimit/namespaces.ts(3 hunks)apps/dashboard/lib/collections/ratelimit/overrides.ts(2 hunks)apps/dashboard/lib/trpc/routers/deploy/deployment/list.ts(1 hunks)apps/dashboard/lib/trpc/routers/deploy/domains/list.ts(1 hunks)apps/dashboard/lib/trpc/routers/deploy/project/create.ts(4 hunks)apps/dashboard/lib/trpc/routers/deploy/project/list.ts(1 hunks)apps/dashboard/lib/trpc/routers/environment/list.ts(1 hunks)go/apps/ctrl/services/deployment/deploy_workflow.go(1 hunks)go/apps/ctrl/services/routing/service.go(1 hunks)go/pkg/db/models_generated.go(2 hunks)go/pkg/db/project_update_active_deployment_id.sql_generated.go(0 hunks)go/pkg/db/project_update_live_deployment_id.sql_generated.go(1 hunks)go/pkg/db/querier_generated.go(2 hunks)go/pkg/db/queries/project_update_active_deployment_id.sql(0 hunks)go/pkg/db/queries/project_update_live_deployment_id.sql(1 hunks)go/pkg/db/schema.sql(2 hunks)internal/db/src/schema/projects.ts(1 hunks)
💤 Files with no reviewable changes (4)
- go/pkg/db/queries/project_update_active_deployment_id.sql
- apps/dashboard/lib/collections/environments.ts
- go/pkg/db/project_update_active_deployment_id.sql_generated.go
- apps/dashboard/lib/collections/domains.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/dashboard/app/(app)/projects/_components/list/index.tsx
- go/pkg/db/schema.sql
- apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/page.tsx
- apps/dashboard/app/(app)/projects/[projectId]/deployments/hooks/use-deployments.ts
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
📚 Learning: 2025-09-12T08:01:20.752Z
Learnt from: Flo4604
PR: unkeyed/unkey#3944
File: go/pkg/db/acme_challenge_update_verified_with_expiry.sql_generated.go:31-39
Timestamp: 2025-09-12T08:01:20.752Z
Learning: Do not review or suggest changes to files with sql_generated.go suffix or other files marked as auto-generated (containing "Code generated by" comments), as these are generated by tools like sqlc and changes would be overwritten on regeneration.
Applied to files:
go/pkg/db/project_update_live_deployment_id.sql_generated.go
📚 Learning: 2025-08-31T19:30:38.171Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3853
File: apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts:75-87
Timestamp: 2025-08-31T19:30:38.171Z
Learning: In apps/dashboard/lib/trpc/routers/deploy/project/envs/list.ts, the getEnvs procedure currently uses mock data (VARIABLES) and ignores the projectId input parameter. This is intentional temporary behavior - the user ogzhanolguncu indicated they plan to hook this up to the database later.
Applied to files:
apps/dashboard/lib/collections/deploy/environments.tsapps/dashboard/lib/trpc/routers/deploy/project/list.tsapps/dashboard/lib/trpc/routers/environment/list.tsapps/dashboard/lib/trpc/routers/deploy/deployment/list.tsapps/dashboard/lib/trpc/routers/deploy/project/create.ts
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Applied to files:
apps/dashboard/lib/collections/deploy/deployments.tsapps/dashboard/lib/trpc/routers/deploy/domains/list.tsapps/dashboard/lib/trpc/routers/deploy/project/list.tsapps/dashboard/lib/trpc/routers/deploy/deployment/list.tsinternal/db/src/schema/projects.tsapps/dashboard/lib/trpc/routers/deploy/project/create.ts
📚 Learning: 2025-07-25T19:11:00.208Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/getOpenApiDiff.ts:110-147
Timestamp: 2025-07-25T19:11:00.208Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/getOpenApiDiff.ts, the user mcstepp prefers to keep mock data fallbacks in POC/demonstration code for simplicity, even if it wouldn't be production-ready. This aligns with the PR being work-in-progress for demonstration purposes.
Applied to files:
apps/dashboard/lib/trpc/routers/deploy/project/list.tsapps/dashboard/lib/trpc/routers/deploy/project/create.ts
📚 Learning: 2025-08-25T12:56:59.310Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts:59-66
Timestamp: 2025-08-25T12:56:59.310Z
Learning: In the ratelimit namespace query system (apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts), the nameQuery filter is designed as an array for future extensibility to support multiple filters, but currently only the first filter (index 0) is processed. This is intentional future-proofing.
Applied to files:
apps/dashboard/lib/collections/ratelimit/namespaces.ts
📚 Learning: 2025-07-28T20:38:53.244Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/components/client.tsx:322-341
Timestamp: 2025-07-28T20:38:53.244Z
Learning: In apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/components/client.tsx, mcstepp prefers to keep hardcoded endpoint logic in the getDiffType function during POC phases for demonstrating diff functionality, rather than implementing a generic diff algorithm. This follows the pattern of keeping simplified implementations for demonstration purposes.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/diff/page.tsx
📚 Learning: 2025-08-18T10:28:47.391Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3797
File: apps/dashboard/app/(app)/projects/[projectId]/deployments/components/control-cloud/index.tsx:1-4
Timestamp: 2025-08-18T10:28:47.391Z
Learning: In Next.js App Router, components that use React hooks don't need their own "use client" directive if they are rendered within a client component that already has the directive. The client boundary propagates to child components.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/index.tsx
📚 Learning: 2025-09-12T17:57:12.128Z
Learnt from: mcstepp
PR: unkeyed/unkey#3952
File: apps/dashboard/app/(app)/projects/[projectId]/deployments/components/rollback-dialog.tsx:111-116
Timestamp: 2025-09-12T17:57:12.128Z
Learning: In the Deployment type used across the dashboard deployment components, the source field is required and will always be present, so optional chaining (source?.branch) is not needed when accessing source.branch or source.gitSha.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/index.tsx
🧬 Code graph analysis (24)
apps/dashboard/lib/collections/deploy/domains.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
go/pkg/db/project_update_live_deployment_id.sql_generated.go (1)
go/pkg/db/types/null_string.go (1)
NullString(10-10)
apps/dashboard/app/(app)/projects/[projectId]/layout.tsx (2)
apps/dashboard/lib/collections/index.ts (1)
collectionManager(69-69)apps/dashboard/app/(app)/projects/[projectId]/layout-provider.tsx (1)
ProjectLayoutContext(11-11)
apps/dashboard/lib/collections/ratelimit/overrides.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
apps/dashboard/lib/collections/deploy/environments.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
apps/dashboard/lib/collections/deploy/projects.ts (3)
go/pkg/db/models_generated.go (1)
Project(671-682)internal/db/src/schema/projects.ts (1)
projects(8-31)apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
apps/dashboard/lib/collections/deploy/deployments.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
apps/dashboard/lib/trpc/routers/deploy/domains/list.ts (1)
apps/dashboard/lib/trpc/trpc.ts (4)
t(8-8)requireUser(10-21)requireWorkspace(23-36)withRatelimit(122-138)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/controls/components/deployment-list-filters/components/environment-filter.tsx (2)
apps/dashboard/app/(app)/projects/[projectId]/layout-provider.tsx (1)
useProjectLayout(13-19)internal/db/src/schema/environments.ts (1)
environments(8-24)
apps/dashboard/lib/trpc/routers/environment/list.ts (3)
apps/dashboard/lib/trpc/trpc.ts (3)
t(8-8)requireUser(10-21)requireWorkspace(23-36)apps/dashboard/lib/db.ts (1)
db(5-26)internal/db/src/schema/environments.ts (1)
environments(8-24)
go/apps/ctrl/services/routing/service.go (1)
go/pkg/db/project_update_live_deployment_id.sql_generated.go (1)
UpdateProjectLiveDeploymentIdParams(19-23)
apps/dashboard/lib/collections/ratelimit/namespaces.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
apps/dashboard/lib/trpc/routers/deploy/deployment/list.ts (2)
apps/dashboard/lib/trpc/trpc.ts (3)
t(8-8)requireUser(10-21)requireWorkspace(23-36)internal/db/src/schema/deployments.ts (1)
deployments(9-52)
apps/dashboard/lib/trpc/routers/deploy/project/create.ts (1)
apps/dashboard/lib/collections/deploy/projects.ts (1)
createProjectRequestSchema(16-28)
apps/dashboard/app/(app)/projects/[projectId]/page.tsx (3)
apps/dashboard/app/(app)/projects/[projectId]/layout-provider.tsx (1)
useProjectLayout(13-19)internal/db/src/schema/domains.ts (1)
domains(4-20)apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/index.tsx (1)
ActiveDeploymentCard(71-272)
apps/dashboard/app/(app)/projects/[projectId]/diff/page.tsx (1)
apps/dashboard/app/(app)/projects/[projectId]/layout-provider.tsx (1)
useProjectLayout(13-19)
go/pkg/db/querier_generated.go (2)
go/pkg/partition/db/database.go (1)
DBTX(10-10)go/pkg/db/project_update_live_deployment_id.sql_generated.go (1)
UpdateProjectLiveDeploymentIdParams(19-23)
apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/index.tsx (3)
apps/dashboard/app/(app)/projects/[projectId]/layout-provider.tsx (1)
useProjectLayout(13-19)apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/git-avatar.tsx (1)
Avatar(10-29)apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/info-chip.tsx (1)
InfoChip(8-13)
apps/dashboard/app/(app)/projects/[projectId]/layout-provider.tsx (1)
apps/dashboard/lib/collections/index.ts (1)
collectionManager(69-69)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/controls/components/deployment-list-datetime/index.tsx (1)
apps/dashboard/app/(app)/projects/[projectId]/deployments/hooks/use-filters.ts (1)
useFilters(33-127)
apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx (2)
apps/dashboard/lib/collections/deploy/projects.ts (2)
CreateProjectRequestSchema(31-31)createProjectRequestSchema(16-28)apps/dashboard/lib/collections/index.ts (2)
collection(72-76)reset(78-86)
go/apps/ctrl/services/deployment/deploy_workflow.go (2)
go/pkg/db/project_update_live_deployment_id.sql_generated.go (1)
UpdateProjectLiveDeploymentIdParams(19-23)go/pkg/db/types/null_string.go (1)
NullString(10-10)
apps/dashboard/app/(app)/projects/[projectId]/details/project-details-expandables/index.tsx (2)
apps/dashboard/app/(app)/projects/[projectId]/layout-provider.tsx (1)
useProjectLayout(13-19)apps/dashboard/lib/collections/index.ts (1)
collection(72-76)
apps/dashboard/lib/collections/index.ts (4)
apps/dashboard/lib/collections/deploy/environments.ts (1)
createEnvironmentsCollection(15-30)apps/dashboard/lib/collections/deploy/domains.ts (1)
createDomainsCollection(16-31)apps/dashboard/lib/collections/deploy/deployments.ts (1)
createDeploymentsCollection(41-56)apps/dashboard/lib/collections/deploy/projects.ts (1)
projects(33-98)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages / Test
- GitHub Check: Test Go API Local / Test
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (22)
apps/dashboard/lib/trpc/routers/environment/list.ts (1)
28-36: Good error normalization.Re-throwing TRPCError and mapping others to INTERNAL_SERVER_ERROR is solid.
apps/dashboard/lib/trpc/routers/deploy/project/create.ts (1)
102-111: LGTM: default environments created atomically.Creating production/preview within the same transaction is correct and matches the new (project_id, slug) uniqueness.
go/pkg/db/querier_generated.go (1)
1677-1683: API rename acknowledged — sweep callsitesInterface renamed to UpdateProjectLiveDeploymentId in go/pkg/db/querier_generated.go (lines 1677–1683). Confirm all callsites migrated from UpdateProjectActiveDeploymentId to UpdateProjectLiveDeploymentId; e.g. run: rg -nP '\b(UpdateProjectActiveDeploymentId|UpdateProjectLiveDeploymentId)\b' .
go/pkg/db/models_generated.go (2)
3-3: Inconsistent sqlc versions in generated files — align to a single sqlc version (recommended: v1.29.0)Most generated files are stamped v1.29.0, but the following have older headers — regenerate these with sqlc v1.29.0 (do not edit generated files manually):
v1.27.0
- go/pkg/db/models_generated.go
- go/pkg/db/project_update_live_deployment_id.sql_generated.go
- go/pkg/db/querier_generated.go
v1.28.0
- go/pkg/kv/stores/mysql/db.go
- go/pkg/kv/stores/mysql/delete.sql.go
- go/pkg/kv/stores/mysql/delete_expired.sql.go
- go/pkg/kv/stores/mysql/get.sql.go
- go/pkg/kv/stores/mysql/list_by_workspace.sql.go
- go/pkg/kv/stores/mysql/models.go
- go/pkg/kv/stores/mysql/set.sql.go
Regenerate these files with sqlc v1.29.0 or standardize the generator in CI to ensure reproducible builds.
⛔ Skipped due to learnings
Learnt from: Flo4604 PR: unkeyed/unkey#3944 File: go/pkg/db/acme_challenge_update_verified_with_expiry.sql_generated.go:31-39 Timestamp: 2025-09-12T08:01:20.752Z Learning: Do not review or suggest changes to files with sql_generated.go suffix or other files marked as auto-generated (containing "Code generated by" comments), as these are generated by tools like sqlc and changes would be overwritten on regeneration.Learnt from: chronark PR: unkeyed/unkey#3420 File: go/pkg/hydra/store/gorm/gorm.go:486-498 Timestamp: 2025-07-02T11:51:58.572Z Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
672-682: LiveDeploymentID rename validated — confirm migration appliedBackend models, generated queries, and schema now use live_deployment_id; no occurrences of ActiveDeploymentID/active_deployment_id found. Remaining hits are a frontend sentinel and an unrelated count alias.
- go/pkg/db/models_generated.go — Project.LiveDeploymentID (
db:"live_deployment_id").- go/pkg/db/schema.sql — contains
live_deployment_id.- go/pkg/db/project_update_live_deployment_id.sql & go/pkg/db/project_update_live_deployment_id.sql_generated.go — update query for live_deployment_id.
- go/apps/ctrl/services/deployment/deploy_workflow.go:387 and go/apps/ctrl/services/routing/service.go:363 — set LiveDeploymentID.
- internal/db/src/schema/projects.ts:21 — liveDeploymentId.
- Frontend sentinel: apps/dashboard/.../deployment-list-table-action.popover.constants.tsx:56 —
"current_active_deployment_id"(UI-only).- Unrelated: go/apps/metald/sqlc/queries.sql & go/apps/metald/internal/database/queries.sql.go —
active_deploymentsused as a count alias.Confirm the DB migration that renames/applies this column was executed in all target environments (or include the migration if missing).
go/pkg/db/queries/project_update_live_deployment_id.sql (1)
1-4: LGTM — confirm timestamp semantics.projects.updated_at is BIGINT (go/pkg/db/schema.sql) and UpdateProjectLiveDeploymentId expects UpdatedAt sql.NullInt64 (go/pkg/db/project_update_live_deployment_id.sql_generated.go). Repo lifecycle defaults use Date.now() (milliseconds) (internal/db/src/schema/util/lifecycle_dates.ts). Ensure callers pass epoch milliseconds (not NOW()) when calling this query.
apps/dashboard/lib/collections/ratelimit/namespaces.ts (3)
6-6: Import path change to shared client looks good.
30-33: Namespace create call formatting change is no‑op — fine to keep.
62-64: Namespace delete call formatting change is no‑op — fine to keep.apps/dashboard/app/(app)/projects/[projectId]/page.tsx (2)
41-48: liveDeploymentId swap looks correct.ActiveDeploymentCard will now read from the per‑project deployments collection; good alignment.
14-17: Resolved — domains collection is project‑scoped.
createDomainsCollection(projectId) is used when populating projectCollections (apps/dashboard/lib/collections/index.ts) and createDomainsCollection throws if projectId is missing (apps/dashboard/lib/collections/deploy/domains.ts).apps/dashboard/lib/collections/ratelimit/overrides.ts (2)
6-6: Import path change to shared client is correct.
59-61: Delete mutation formatting change is a no‑op — fine.apps/dashboard/app/(app)/projects/[projectId]/details/project-details-expandables/index.tsx (1)
23-31: Join source swap + liveDeploymentId — looks good; verify cross-collection join compatibility.Using global
collection.projectswith per-projectcollections.deploymentsshould share the same React‑DB client; confirm they’re produced by the same queryClient to avoid join-time cache mismatches.apps/dashboard/app/(app)/projects/[projectId]/diff/page.tsx (1)
8-12: Confirm layout wrapper usage.This page assumes
useProjectLayout()context. Ensure it’s always rendered underProjectLayoutWrapper.apps/dashboard/app/(app)/projects/[projectId]/details/active-deployment-card/index.tsx (1)
72-77: Collection source switch — good.Querying
collections.deploymentsbydeploymentIdmatches the new per‑project pattern.go/pkg/db/project_update_live_deployment_id.sql_generated.go (1)
1-34: Auto-generated file — no review.apps/dashboard/lib/collections/deploy/projects.ts (2)
33-41: Projects list wiring looks good.Solid query keying and list fetch via
trpcClient.deploy.project.list.query().getKey: item.idis appropriate.
52-95: Good UX on error mapping and toast lifecycle.
toast.promise(mutation, …); await mutation;provides user feedback and keeps the transaction lifecycle correct.Please confirm the created project appears in the list without a manual reload (collection should reconcile automatically). If it doesn’t, we can add an invalidate/refetch hook after success.
apps/dashboard/lib/collections/index.ts (3)
24-41: Per‑project collection cache is clean and cohesive.Nice encapsulation of env/domains/deployments keyed by projectId; returning the shared
projectscollection is pragmatic.
56-66: Cleanup order and concurrency look fine.Project collections are cleaned first, then global collections;
Promise.allprevents leaks.
1-87: Verify active→live deployment rename is complete — manual re-check requiredripgrep in the verification run skipped files; results are inconclusive. From the repo root run (locally) to confirm no stale references remain:
# Search code for leftover fields/keys git ls-files -z | xargs -0 rg -nP -S '\bactiveDeploymentId\b|active_deployment_id|active[_-]?deployment' || true # Search DB migrations / index names for environment uniqueness by project git ls-files -z | xargs -0 rg -nP -S -i '(environment.*unique.*project_id)|(unique.*project_id.*environment)' || true # Fallback (if rg isn't available or to double-check) git grep -nE 'activeDeploymentId|active_deployment_id|active[_-]?deployment' || true git grep -niE 'environment.*unique.*project_id|unique.*project_id.*environment' || trueIf any matches are found, update usages/migrations to the new "live" naming and ensure DB/index names and any serialized keys are migrated consistently.
...ts/[projectId]/deployments/components/controls/components/deployment-list-datetime/index.tsx
Show resolved
Hide resolved
|
just waiting on green CI here |
3 participants
What does this PR do?
This PR fixes project collections and the database index for environments. The old index was preventing us from adding new projects since they were treated as a unique
workspaceId-slugconstraint (e.g.,ws_asdsad-production). When you tried to add a new project, that would fail due to this constraint.Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Improvements
Behavior Changes
Removed