refactor: extract DeploymentStatus to shared location#5264
Merged
ogzhanolguncu merged 10 commits intomainfrom Mar 9, 2026
Merged
refactor: extract DeploymentStatus to shared location#5264ogzhanolguncu merged 10 commits intomainfrom
ogzhanolguncu merged 10 commits intomainfrom
Conversation
ogzhanolguncu
requested review from
Flo4604,
MichaelUnkey,
chronark,
imeyer,
mcstepp and
perkinsjr
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughDeployment status values and types were centralized into a new module. The old local DeploymentStatusBadge was removed and replaced by a single, refactored badge component at a higher-level path using a data-driven config. Zod schemas, imports, and UI code were updated to use the shared status constant/type. A compound-duration formatter was added. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/components/deployment-status-badge.tsx (1)
96-98: Defensive check for type-safe code.Since
statusis typed asDeploymentStatusandstatusConfigsis a completeRecord<DeploymentStatus, StatusConfig>, this check will never trigger at runtime (TypeScript guarantees exhaustiveness). This is fine as defensive coding, but the error would only surface if there's a type mismatch at the boundary. Consider whether this runtime check adds value or could be removed in favor of trusting the type system.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/components/deployment-status-badge.tsx around lines 96 - 98, Remove the redundant runtime null-check and throw for `config` since `status` is typed as `DeploymentStatus` and `statusConfigs: Record<DeploymentStatus, StatusConfig>` is exhaustive; delete the if (!config) { throw new Error(`Invalid deployment status: ${status}`); } block (referencing `status`, `statusConfigs`, `StatusConfig`, and the `config` variable) so the code relies on the type system, or alternatively replace it with a compile-time exhaustive check pattern (e.g., a `never`-case helper) if you want explicit exhaustiveness handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/components/deployment-status-badge.tsx:
- Around line 96-98: Remove the redundant runtime null-check and throw for
`config` since `status` is typed as `DeploymentStatus` and `statusConfigs:
Record<DeploymentStatus, StatusConfig>` is exhaustive; delete the if (!config) {
throw new Error(`Invalid deployment status: ${status}`); } block (referencing
`status`, `statusConfigs`, `StatusConfig`, and the `config` variable) so the
code relies on the type system, or alternatively replace it with a compile-time
exhaustive check pattern (e.g., a `never`-case helper) if you want explicit
exhaustiveness handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 966c4bbb-847e-40be-a70b-a3f36766bb78
📒 Files selected for processing (8)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/components/deployment-status-badge.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/components/domain_list.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/deployments-list.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/filters.schema.tsweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/components/deployment-status-badge.tsxweb/apps/dashboard/lib/collections/deploy/deployment-status.tsweb/apps/dashboard/lib/collections/deploy/deployments.tsweb/apps/dashboard/lib/collections/index.ts
💤 Files with no reviewable changes (1)
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/components/deployment-status-badge.tsx
Contributor
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)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/filters.schema.ts (1)
42-54:⚠️ Potential issue | 🟡 MinorMap
readyto the success color.Line 44 still checks
"completed", butvalidValuesin this file only allow"pending" | "deploying" | "ready" | "failed". As written,"ready"falls through to the default info color and renders like an in-progress state.Minimal fix
getColorClass: (value) => { - if (value === "completed") { + if (value === "ready") { return "bg-success-9"; } if (value === "failed") { return "bg-error-9"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/filters.schema.ts around lines 42 - 54, The getColorClass function currently checks for "completed" but GROUPED_DEPLOYMENT_STATUSES only contains "pending" | "deploying" | "ready" | "failed", so "ready" falls through to the default; update getColorClass (the function defined alongside validValues and GROUPED_DEPLOYMENT_STATUSES) to map the "ready" value to the success color (bg-success-9) instead of checking "completed" so ready deployments render with the success color.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/components/deployment-status-badge.tsx:
- Around line 13-18: The file imports IconProps from the private path
"@unkey/icons/src/props"; update the import to use the public package entry
(e.g. import type { IconProps } from "@unkey/icons") or remove the dependency by
inferring the prop type directly via React.ComponentProps from a concrete icon
(e.g. ComponentProps<typeof CircleCheck>) and then update the StatusConfig type
(icon: FC<IconProps>) to use the chosen, public prop type so you no longer
depend on an internal path.
---
Outside diff comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/filters.schema.ts:
- Around line 42-54: The getColorClass function currently checks for "completed"
but GROUPED_DEPLOYMENT_STATUSES only contains "pending" | "deploying" | "ready"
| "failed", so "ready" falls through to the default; update getColorClass (the
function defined alongside validValues and GROUPED_DEPLOYMENT_STATUSES) to map
the "ready" value to the success color (bg-success-9) instead of checking
"completed" so ready deployments render with the success color.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a3400b13-9b66-477d-982d-e7ec5535e9db
📒 Files selected for processing (4)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/components/domain_list.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/deployments-list.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/filters.schema.tsweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/components/deployment-status-badge.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/components/domain_list.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/deployments-list.tsx
...hboard/app/(app)/[workspaceSlug]/projects/[projectId]/components/deployment-status-badge.tsx
Show resolved
Hide resolved
chronark
approved these changes
Mar 9, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-step.tsx:
- Around line 91-93: The span currently hides zero-duration values because it
uses a truthiness check on duration; update the conditional to explicitly check
for null/undefined (e.g., duration !== null && duration !== undefined) so that 0
is rendered (call formatCompoundDuration(duration) when duration is defined).
Locate the span that references duration and formatCompoundDuration in
deployment-step.tsx and replace the truthy check with an explicit null/undefined
check to ensure "0ms" is displayed.
In `@web/apps/dashboard/lib/utils/metric-formatters.ts`:
- Around line 17-18: The branch handling sub-second durations incorrectly uses
rounding and can yield "1000ms" for values like 999.6; change the logic in
metric-formatters.ts where the variable ms is checked (the if (ms < 1000)
branch) to floor the milliseconds instead of rounding so values remain below
1000 (i.e., use Math.floor on ms when formatting the `${...}ms` output).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 30219b06-aef8-4a4f-a0da-29564ac22d88
📒 Files selected for processing (3)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-progress.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-step.tsxweb/apps/dashboard/lib/utils/metric-formatters.ts
.../[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-step.tsx
Show resolved
Hide resolved
ogzhanolguncu
force-pushed
the
deployment-status-shared
branch
from
9e3caed to
a03ce3c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Use one shared DeploymentStatus and DeploymentStatusBadge.
How should this be tested?