-
Notifications
You must be signed in to change notification settings - Fork 607
feat: dashboard openapi diffing #4042
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
de6a847
7fcd515
92efcee
4ce07ae
a73ade9
ca0f0ee
62f301e
c728366
8736852
86a8728
c8b4c9f
a1178cb
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,38 +1,87 @@ | ||
| import { Cloud } from "@unkey/icons"; | ||
| import { InfoTooltip } from "@unkey/ui"; | ||
| import { cn } from "@unkey/ui/src/lib/utils"; | ||
|
|
||
| export type DiffStatus = "breaking" | "warning" | "safe" | "loading"; | ||
|
|
||
| type StatusIndicatorProps = { | ||
| status?: DiffStatus; | ||
| withSignal?: boolean; | ||
| className?: string; | ||
| }; | ||
|
|
||
| const getTooltipContent = (status: DiffStatus): string => { | ||
| switch (status) { | ||
| case "breaking": | ||
| return "Breaking changes detected - this deployment may break existing API clients"; | ||
| case "warning": | ||
| return "API changes detected - review the differences before deploying"; | ||
| case "safe": | ||
| return "No API changes detected"; | ||
| case "loading": | ||
| return "Analyzing API differences..."; | ||
| } | ||
| }; | ||
|
|
||
| export function StatusIndicator({ | ||
| status = "safe", | ||
| withSignal = false, | ||
| }: { | ||
| withSignal?: boolean; | ||
| }) { | ||
| className, | ||
| }: StatusIndicatorProps) { | ||
| const isBreaking = status === "breaking"; | ||
| const isWarning = status === "warning"; | ||
| const isLoading = status === "loading"; | ||
|
|
||
| const pulseColors = isBreaking | ||
| ? ["bg-error-9", "bg-error-10", "bg-error-11", "bg-error-12"] | ||
| : isWarning | ||
| ? ["bg-warning-9", "bg-warning-10", "bg-warning-11", "bg-warning-12"] | ||
| : ["bg-successA-9", "bg-successA-10", "bg-successA-11", "bg-successA-12"]; | ||
|
|
||
| const coreColor = isBreaking ? "bg-error-9" : isWarning ? "bg-warning-9" : "bg-successA-9"; | ||
|
|
||
| return ( | ||
| <div className="relative"> | ||
| <div className="size-5 rounded flex items-center justify-center cursor-pointer border border-grayA-3 transition-all duration-100 bg-grayA-3"> | ||
| <Cloud size="sm-regular" className="text-gray-12" /> | ||
| </div> | ||
| {withSignal && ( | ||
| <div className="absolute -top-0.5 -right-0.5"> | ||
| {[0, 0.15, 0.3, 0.45].map((delay, index) => ( | ||
| <div | ||
| // biome-ignore lint/suspicious/noArrayIndexKey: <explanation> | ||
| key={index} | ||
| className={cn( | ||
| "absolute inset-0 size-2 rounded-full", | ||
| index === 0 && "bg-successA-9 opacity-75", | ||
| index === 1 && "bg-successA-10 opacity-60", | ||
| index === 2 && "bg-successA-11 opacity-40", | ||
| index === 3 && "bg-successA-12 opacity-25", | ||
| )} | ||
| style={{ | ||
| animation: "ping 2s cubic-bezier(0, 0, 0.2, 1) infinite", | ||
| animationDelay: `${delay}s`, | ||
| }} | ||
| /> | ||
| ))} | ||
| <div className="relative size-2 bg-successA-9 rounded-full" /> | ||
| <InfoTooltip | ||
| content={getTooltipContent(status)} | ||
| position={{ | ||
| side: "bottom", | ||
| align: "center", | ||
| }} | ||
| className="max-w-[300px]" | ||
| > | ||
| <div className="relative"> | ||
| <div | ||
| className={cn( | ||
| "size-5 rounded flex items-center justify-center cursor-pointer border border-grayA-3 transition-all duration-100 bg-grayA-3", | ||
| className, | ||
| )} | ||
| > | ||
| <Cloud size="sm-regular" className="text-gray-12" /> | ||
| </div> | ||
| )} | ||
| </div> | ||
| {withSignal && !isLoading && ( | ||
| <div className="absolute -top-0.5 -right-0.5"> | ||
| {[0, 0.15, 0.3, 0.45].map((delay, index) => ( | ||
| <div | ||
| // biome-ignore lint/suspicious/noArrayIndexKey: <explanation> | ||
| key={index} | ||
| className={cn( | ||
| "absolute inset-0 size-2 rounded-full", | ||
| pulseColors[index], | ||
| index === 0 && "opacity-75", | ||
| index === 1 && "opacity-60", | ||
| index === 2 && "opacity-40", | ||
| index === 3 && "opacity-25", | ||
| )} | ||
| style={{ | ||
| animation: "ping 2s cubic-bezier(0, 0, 0.2, 1) infinite", | ||
| animationDelay: `${delay}s`, | ||
| }} | ||
| /> | ||
| ))} | ||
| <div className={cn("relative size-2 rounded-full", coreColor)} /> | ||
| </div> | ||
| )} | ||
| </div> | ||
| </InfoTooltip> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,108 @@ | ||||||||||
| "use client"; | ||||||||||
| import { shortenId } from "@/lib/shorten-id"; | ||||||||||
| import { trpc } from "@/lib/trpc/client"; | ||||||||||
| import { useLiveQuery } from "@tanstack/react-db"; | ||||||||||
| import { ArrowRight } from "@unkey/icons"; | ||||||||||
| import type { GetOpenApiDiffResponse } from "@unkey/proto"; | ||||||||||
| import { InfoTooltip } from "@unkey/ui"; | ||||||||||
| import Link from "next/link"; | ||||||||||
| import { useParams } from "next/navigation"; | ||||||||||
| import { useProjectLayout } from "../../../layout-provider"; | ||||||||||
| import { type DiffStatus, StatusIndicator } from "../../active-deployment-card/status-indicator"; | ||||||||||
|
|
||||||||||
| const getDiffStatus = (data?: GetOpenApiDiffResponse): DiffStatus => { | ||||||||||
| if (!data) { | ||||||||||
| return "loading"; | ||||||||||
| } | ||||||||||
| if (data.hasBreakingChanges) { | ||||||||||
| return "breaking"; | ||||||||||
| } | ||||||||||
| if (data.summary?.diff) { | ||||||||||
| return "warning"; | ||||||||||
| } | ||||||||||
| return "safe"; | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| export const OpenApiDiff = () => { | ||||||||||
| const params = useParams(); | ||||||||||
| const { collections, liveDeploymentId } = useProjectLayout(); | ||||||||||
|
|
||||||||||
| const query = useLiveQuery( | ||||||||||
| (q) => | ||||||||||
| q | ||||||||||
| .from({ deployment: collections.deployments }) | ||||||||||
| .orderBy(({ deployment }) => deployment.createdAt, "desc") | ||||||||||
| .limit(2) | ||||||||||
| .select((c) => ({ | ||||||||||
| id: c.deployment.id, | ||||||||||
| })), | ||||||||||
| [liveDeploymentId], | ||||||||||
| ); | ||||||||||
|
|
||||||||||
| const [newDeployment, oldDeployment] = query.data ?? []; | ||||||||||
|
|
||||||||||
| const diff = trpc.deploy.deployment.getOpenApiDiff.useQuery({ | ||||||||||
| newDeploymentId: newDeployment?.id ?? "", | ||||||||||
| oldDeploymentId: oldDeployment?.id ?? "", | ||||||||||
| }); | ||||||||||
|
Comment on lines
+44
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major 🧩 Analysis chainVerify TRPC query behavior with empty deployment IDs. When 🏁 Script executed: #!/bin/bash
# Check the getOpenApiDiff implementation to verify empty string handling
ast-grep --pattern 'getOpenApiDiff$$$'
rg -nP --type=ts -C5 'getOpenApiDiff' apps/dashboard/lib/trpc/routers/deployment/Length of output: 83 🏁 Script executed: #!/bin/bash
# locate implementation file for getOpenApiDiff
files=$(rg -l "getOpenApiDiff" -g '*.ts' apps/dashboard/lib/trpc/routers || true)
echo "Found files:"
echo "$files"
# inspect occurrences in each file
for file in $files; do
echo "--- File: $file ---"
rg -n -C5 "getOpenApiDiff" "$file"
doneLength of output: 1945 Guard the TRPC query from running with empty IDs 🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
| // @ts-expect-error I have no idea why this whines about type diff | ||||||||||
| const status = getDiffStatus(diff.data); | ||||||||||
|
Comment on lines
+49
to
+50
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Investigate and document the type mismatch. The If the types genuinely conflict and there's a valid reason, update the comment to be more specific: - // @ts-expect-error I have no idea why this whines about type diff
+ // @ts-expect-error GetOpenApiDiffResponse type doesn't match getDiffStatus parameter due to [specific reason]
const status = getDiffStatus(diff.data);Or investigate and fix the underlying type issue. 📝 Committable suggestion
Suggested change
|
||||||||||
|
|
||||||||||
| if (newDeployment && !oldDeployment) { | ||||||||||
| return ( | ||||||||||
| <div className="rounded-[10px] flex items-center border border-gray-5 h-[52px] w-full max-w-md"> | ||||||||||
| <div className="bg-grayA-2 rounded-l-[10px] border-r border-grayA-3 h-full w-[52px] flex items-center justify-center shrink-0"> | ||||||||||
| <StatusIndicator status="safe" className="bg-transparent" /> | ||||||||||
| </div> | ||||||||||
| <div className="flex flex-col flex-1 px-3 min-w-0"> | ||||||||||
| <div className="text-grayA-9 text-xs">current</div> | ||||||||||
| <div className="text-accent-12 font-medium text-xs truncate"> | ||||||||||
| {shortenId(newDeployment.id)} | ||||||||||
| </div> | ||||||||||
| </div> | ||||||||||
| </div> | ||||||||||
| ); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (!newDeployment) { | ||||||||||
| return null; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const diffUrl = `/${params?.workspaceSlug}/projects/${params?.projectId}/openapi-diff?from=${oldDeployment.id}&to=${newDeployment.id}`; | ||||||||||
| return ( | ||||||||||
| <InfoTooltip | ||||||||||
| content="View detailed API diff comparison" | ||||||||||
| position={{ side: "top", align: "center" }} | ||||||||||
| asChild | ||||||||||
| > | ||||||||||
| <Link href={diffUrl} className="hover:opacity-80 transition-opacity block"> | ||||||||||
| <div className="gap-4 items-center flex w-full"> | ||||||||||
| <div className="rounded-[10px] flex items-center border border-gray-5 h-[52px] w-full"> | ||||||||||
| <div className="bg-grayA-2 rounded-l-[10px] border-r border-grayA-3 h-full w-1/3 flex items-center justify-center"> | ||||||||||
| <StatusIndicator className="bg-transparent" /> | ||||||||||
| </div> | ||||||||||
| <div className="flex flex-col flex-1 px-3"> | ||||||||||
| <div className="text-grayA-9 text-xs">from</div> | ||||||||||
| <div className="text-accent-12 font-medium text-xs"> | ||||||||||
| {shortenId(oldDeployment.id)} | ||||||||||
| </div> | ||||||||||
| </div> | ||||||||||
| </div> | ||||||||||
| <ArrowRight className="shrink-0 text-gray-9 size-[14px]" size="sm-regular" /> | ||||||||||
| <div className="rounded-[10px] flex items-center border border-gray-5 h-[52px] w-full"> | ||||||||||
| <div className="bg-grayA-2 border-r border-grayA-3 h-full w-1/3 flex items-center justify-center"> | ||||||||||
| <StatusIndicator status={status} withSignal className="bg-transparent" /> | ||||||||||
| </div> | ||||||||||
| <div className="flex flex-col flex-1 px-3"> | ||||||||||
| <div className="text-grayA-9 text-xs">to</div> | ||||||||||
| <div className="text-accent-12 font-medium text-xs"> | ||||||||||
| {shortenId(newDeployment.id)} | ||||||||||
| </div> | ||||||||||
| </div> | ||||||||||
| </div> | ||||||||||
| </div> | ||||||||||
| </Link> | ||||||||||
| </InfoTooltip> | ||||||||||
| ); | ||||||||||
| }; | ||||||||||
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.
Replace placeholder biome-ignore explanation.
The ignore comment has a placeholder
<explanation>instead of a meaningful justification.Apply this diff:
📝 Committable suggestion
🤖 Prompt for AI Agents