refactor: cursor environment deployment page refactor #595
Conversation
WalkthroughThis update refactors the deployment environment page by extracting all inline table rendering and search/filter UI into dedicated components. New components— Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EnvironmentDeploymentsPageContent
participant SearchAndFilters
participant DeploymentsTable
participant API
User->>SearchAndFilters: Types search/filter/order
SearchAndFilters->>EnvironmentDeploymentsPageContent: Updates state via props
EnvironmentDeploymentsPageContent->>API: Fetch deployment stats (with filters)
API-->>EnvironmentDeploymentsPageContent: Returns deployment stats
EnvironmentDeploymentsPageContent->>DeploymentsTable: Passes deployment stats and loading state
DeploymentsTable->>User: Renders table (with DeploymentRow or SkeletonRow)
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentRow.tsx (2)
30-38: Consider memoizing the condition and URL construction.The environment condition and URL compression are recalculated on every render. Since these depend only on stable route parameters, consider using
useMemofor optimization:+ const environmentCondition = useMemo(() => ({ + type: JobConditionType.Environment, + value: environmentId, + operator: ColumnOperator.Equals, + }), [environmentId]); + const conditionHash = useMemo(() => + LZString.compressToEncodedURIComponent(JSON.stringify(environmentCondition)), + [environmentCondition] + );
76-82: Simplify repeated success rate calculations.The success rate percentage is calculated multiple times. Consider extracting it to a variable for better readability and performance:
+ const successPercentage = deploymentStat.successRate * 100; className={`h-full rounded-full ${ - deploymentStat.successRate * 100 > 90 + successPercentage > 90 ? "bg-green-500" - : deploymentStat.successRate * 100 > 70 + : successPercentage > 70 ? "bg-amber-500" : "bg-red-500" }`} - style={{ width: `${Number(deploymentStat.successRate * 100)}%` }} + style={{ width: `${successPercentage}%` }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/EnvironmentDeploymentsPageContent.tsx(2 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentRow.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentsTable.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/SearchAndFilters.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/SkeletonRow.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error...
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/SkeletonRow.tsxapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentsTable.tsxapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/SearchAndFilters.tsxapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentRow.tsxapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/types.tsapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/EnvironmentDeploymentsPageContent.tsx
🧬 Code Graph Analysis (5)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/SkeletonRow.tsx (2)
packages/ui/src/table.tsx (2)
TableRow(117-117)TableCell(118-118)packages/ui/src/skeleton.tsx (1)
Skeleton(15-15)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentsTable.tsx (4)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/types.ts (1)
DeploymentStat(6-14)packages/ui/src/table.tsx (5)
Table(112-112)TableHeader(113-113)TableRow(117-117)TableHead(116-116)TableBody(114-114)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/SkeletonRow.tsx (1)
SkeletonRow(6-14)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentRow.tsx (1)
DeploymentRow(20-100)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/SearchAndFilters.tsx (3)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/types.ts (1)
StatusFilter(16-21)packages/ui/src/input.tsx (1)
Input(24-24)packages/ui/src/select.tsx (5)
Select(154-154)SelectTrigger(157-157)SelectValue(156-156)SelectContent(158-158)SelectItem(160-160)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentRow.tsx (5)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/types.ts (1)
DeploymentStat(6-14)packages/validators/src/jobs/conditions/job-condition.ts (1)
JobCondition(30-39)apps/webservice/src/app/urls.ts (1)
urls(262-262)packages/ui/src/table.tsx (2)
TableRow(117-117)TableCell(118-118)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/StatusBadge.tsx (1)
StatusBadge(3-58)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/EnvironmentDeploymentsPageContent.tsx (2)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/SearchAndFilters.tsx (1)
SearchAndFilters(26-81)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentsTable.tsx (1)
DeploymentsTable(20-70)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (linux/amd64)
- GitHub Check: Typecheck
- GitHub Check: Lint
🔇 Additional comments (7)
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/types.ts (1)
1-22: Well-structured type definitions.The type definitions are comprehensive and provide excellent type safety for the deployment components. The
DeploymentStattype captures all necessary deployment information, and theStatusFilterunion type properly handles all filtering states.apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/SkeletonRow.tsx (1)
6-14: Clean loading state implementation.The skeleton row component is well-implemented with consistent styling and proper key handling for the mapped cells.
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentsTable.tsx (1)
44-44: Verify Tailwind class validity.The classes
w-1/8may not be standard Tailwind CSS classes. Tailwind typically includes fractions like 1/2, 1/3, 1/4, 1/5, 1/6, and 1/12, butw-1/8might need to be defined in your Tailwind config or replaced with a standard class.#!/bin/bash # Check if custom width classes are defined in Tailwind config fd -e js -e ts -e json tailwind.config | xargs grep -l "1/8\|custom.*width" || echo "No custom width configurations found"Also applies to: 47-47
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/SearchAndFilters.tsx (1)
15-81: Excellent component design and implementation.The component is well-structured with proper TypeScript typing, responsive design, and clean separation of concerns. The controlled inputs and proper change handlers follow React best practices.
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/EnvironmentDeploymentsPageContent.tsx (1)
6-9: Excellent refactoring that improves code organization.The extraction of inline UI into dedicated components significantly improves maintainability and follows the single responsibility principle. The component props are properly typed and passed correctly.
Also applies to: 46-59
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentRow.tsx (2)
1-14: LGTM: Clean import organization.The imports are well-organized with external libraries first, followed by UI components, and then local imports. All imported modules are properly used in the component.
49-99: LGTM: Well-structured table row component.The component effectively renders deployment information with proper styling, hover effects, and click navigation. The use of formatting libraries (
prettyMilliseconds,formatDistanceToNow) and theStatusBadgecomponent maintains consistency with the application's design system.
| {isLoading && | ||
| Array.from({ length: 3 }).map((_, index) => ( | ||
| <SkeletonRow key={index} /> | ||
| ))} | ||
| {deploymentStats.map((deploymentStat) => ( | ||
| <DeploymentRow | ||
| key={deploymentStat.deployment.id} | ||
| deploymentStat={deploymentStat} | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
Fix rendering logic to prevent simultaneous skeleton and data rows.
Both skeleton rows and actual deployment rows will render when isLoading is true and deploymentStats has data, creating visual confusion.
Apply this diff to fix the conditional rendering:
<TableBody>
- {isLoading &&
- Array.from({ length: 3 }).map((_, index) => (
- <SkeletonRow key={index} />
- ))}
- {deploymentStats.map((deploymentStat) => (
- <DeploymentRow
- key={deploymentStat.deployment.id}
- deploymentStat={deploymentStat}
- />
- ))}
+ {isLoading ? (
+ Array.from({ length: 3 }).map((_, index) => (
+ <SkeletonRow key={index} />
+ ))
+ ) : (
+ deploymentStats.map((deploymentStat) => (
+ <DeploymentRow
+ key={deploymentStat.deployment.id}
+ deploymentStat={deploymentStat}
+ />
+ ))
+ )}
</TableBody>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {isLoading && | |
| Array.from({ length: 3 }).map((_, index) => ( | |
| <SkeletonRow key={index} /> | |
| ))} | |
| {deploymentStats.map((deploymentStat) => ( | |
| <DeploymentRow | |
| key={deploymentStat.deployment.id} | |
| deploymentStat={deploymentStat} | |
| /> | |
| ))} | |
| <TableBody> | |
| {isLoading ? ( | |
| Array.from({ length: 3 }).map((_, index) => ( | |
| <SkeletonRow key={index} /> | |
| )) | |
| ) : ( | |
| deploymentStats.map((deploymentStat) => ( | |
| <DeploymentRow | |
| key={deploymentStat.deployment.id} | |
| deploymentStat={deploymentStat} | |
| /> | |
| )) | |
| )} | |
| </TableBody> |
🤖 Prompt for AI Agents
In
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentsTable.tsx
around lines 56 to 65, the rendering logic currently shows both skeleton loading
rows and actual deployment rows simultaneously when isLoading is true and
deploymentStats has data. To fix this, update the rendering condition so that
skeleton rows are only shown when isLoading is true and deployment rows are only
rendered when isLoading is false, preventing overlap and visual confusion.
| <TableCell className="truncate py-3 text-neutral-300"> | ||
| {deploymentStat.deployedBy} | ||
| </TableCell> |
There was a problem hiding this comment.
Handle potential null value for deployedBy.
According to the DeploymentStat type definition, deployedBy can be null, but it's rendered directly without null handling. This could display "null" in the UI:
- {deploymentStat.deployedBy}
+ {deploymentStat.deployedBy ?? "Unknown"}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <TableCell className="truncate py-3 text-neutral-300"> | |
| {deploymentStat.deployedBy} | |
| </TableCell> | |
| <TableCell className="truncate py-3 text-neutral-300"> | |
| {deploymentStat.deployedBy ?? "Unknown"} | |
| </TableCell> |
🤖 Prompt for AI Agents
In
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/environments/[environmentId]/deployments/_components/DeploymentRow.tsx
around lines 90 to 92, the deploymentStat.deployedBy value can be null but is
rendered directly, which may show "null" in the UI. Update the rendering logic
to check if deployedBy is null and display an empty string or a placeholder
instead to avoid showing "null" text.
Summary by CodeRabbit
New Features
Refactor