fix: Deployment resource drawer #237
Conversation
WalkthroughThe pull request introduces significant changes to the deployment resource management within the web service application. It replaces the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 6
🧹 Outside diff range and nitpick comments (16)
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/deployments/DeployButton.tsx (2)
Line range hint
26-33: Consider enhancing error handling for deployment failures.The current implementation only handles the success case. Consider adding error handling to improve user experience.
onClick={() => deploy .mutateAsync({ environmentId, releaseId, }) .then(() => router.refresh()) + .catch((error) => { + console.error('Deployment failed:', error); + // TODO: Add error notification/toast here + }) }
Line range hint
34-38: Consider enhancing the loading state UI.While the button is correctly disabled during deployment, adding a loading indicator would improve user feedback.
disabled={deploy.isPending} > - Deploy + {deploy.isPending ? ( + <span className="flex items-center gap-2"> + <span className="animate-spin">⟳</span> Deploying... + </span> + ) : ( + "Deploy" + )} </Button>apps/webservice/src/app/[workspaceSlug]/(app)/layout.tsx (1)
50-50: Consider implementing drawer coordination.The addition of
DeploymentResourceDraweris well-positioned among other drawer components. However, with the growing number of drawer components, consider implementing a coordination mechanism to ensure only one drawer can be open at a time, preventing potential UI/UX issues.apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/ReleaseTable.tsx (3)
1-23: LGTM! Consider extracting types to a separate file.The type definitions and imports are well-structured and properly typed. However, as the application grows, consider moving the
ReleaseWithTriggersandReleaseTablePropstypes to a separate types file for better maintainability and reusability.Consider creating a new file
types.ts:import type { RouterOutputs } from "@ctrlplane/api"; import type * as SCHEMA from "@ctrlplane/db/schema"; export type ReleaseWithTriggers = RouterOutputs["job"]["config"]["byDeploymentEnvAndResource"][number]; export type ReleaseTableProps = { releasesWithTriggers: ReleaseWithTriggers[]; environment: SCHEMA.Environment & { system: SCHEMA.System }; deployment: SCHEMA.Deployment; resource: SCHEMA.Resource; };
25-30: LGTM! Consider memoizing the component for performance.The component implementation is clean and type-safe. However, since it might receive frequent prop updates, consider memoizing it.
-export const ReleaseTable: React.FC<ReleaseTableProps> = ({ +export const ReleaseTable = React.memo(({ releasesWithTriggers, environment, deployment, resource, -}) => ( +}: ReleaseTableProps) => (
31-66: Enhance accessibility and maintainability.The table implementation is structurally sound, but could benefit from improved accessibility and maintainability.
Consider these improvements:
- <div className="rounded-md border border-neutral-800"> + <div + className="rounded-md border border-neutral-800" + role="region" + aria-label="Deployment releases" + > <Table className="table-fixed"> - <TableHeader> + <TableHeader aria-label="Releases table header"> <TableRow> <TableHead>Release</TableHead> <TableHead>Status</TableHead> <TableHead>Created</TableHead> <TableHead>Links</TableHead> <TableHead /> </TableRow> </TableHeader> <TableBody> {releasesWithTriggers.map((release) => { return ( <ReleaseRows release={release} environment={environment} deployment={deployment} resource={resource} key={release.id} /> ); })} {releasesWithTriggers.length === 0 && ( <TableRow> <TableCell - colSpan={5} + colSpan={TableHead.length} className="text-center text-muted-foreground" > No releases found </TableCell> </TableRow> )} </TableBody> </Table> </div>apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/useDeploymentResourceDrawer.ts (2)
9-11: Consider moving UUID_REGEX to a shared utilityThe UUID validation regex could be useful in other parts of the application. Consider extracting it to a shared utility file for reusability.
17-42: Consider refactoring for better error handling and readabilityWhile the logic is robust, consider these improvements:
- Use early returns to reduce nesting
- Add different error states to distinguish between parsing failures and validation failures
Here's a suggested refactor:
const { deploymentId, environmentId, resourceId } = useMemo(() => { if (deploymentResourceId == null) return { deploymentId: null, environmentId: null, resourceId: null }; const [rawDeploymentId, rawEnvironmentId, rawResourceId] = decodeURIComponent(deploymentResourceId).split(DELIMITER); - if ( - rawDeploymentId == null || - rawEnvironmentId == null || - rawResourceId == null - ) - return { deploymentId: null, environmentId: null, resourceId: null }; + if (!rawDeploymentId || !rawEnvironmentId || !rawResourceId) { + console.warn('Incomplete deployment resource ID parts'); + return { deploymentId: null, environmentId: null, resourceId: null }; + } - if ( - !UUID_REGEX.test(rawDeploymentId) || - !UUID_REGEX.test(rawEnvironmentId) || - !UUID_REGEX.test(rawResourceId) - ) - return { deploymentId: null, environmentId: null, resourceId: null }; + const isValidUUID = (id: string) => UUID_REGEX.test(id); + if (!isValidUUID(rawDeploymentId) || !isValidUUID(rawEnvironmentId) || !isValidUUID(rawResourceId)) { + console.warn('Invalid UUID format in deployment resource ID'); + return { deploymentId: null, environmentId: null, resourceId: null }; + } return { deploymentId: rawDeploymentId, environmentId: rawEnvironmentId, resourceId: rawResourceId, }; }, [deploymentResourceId]);apps/webservice/src/app/[workspaceSlug]/(app)/(targets)/targets/[targetId]/visualize/nodes/DeploymentNode.tsx (2)
9-9: Consider using path aliases for deep imports.The import path is quite nested. Consider using path aliases (e.g.,
@components/) to improve maintainability and readability.
Line range hint
13-94: Well-structured component with good separation of concerns.The component demonstrates good practices:
- Clear type definitions with SCHEMA types
- Proper separation of visual and logical concerns
- Consistent status handling and visual feedback
- Regular data refetching for active releases
Consider extracting the status-related logic into a custom hook for better reusability if this pattern is used elsewhere in the application.
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx (3)
25-25: Consider memoizing the setIsOpen callbackThe
setIsOpenfunction is recreated on every render. Consider usinguseCallbackto memoize it:- const setIsOpen = () => setDeploymentEnvResourceId(null, null, null); + const setIsOpen = useCallback( + () => setDeploymentEnvResourceId(null, null, null), + [setDeploymentEnvResourceId] + );
68-74: Consider enhancing Drawer component to handle loading statesThe current workaround of rendering empty
DrawerTitleandDrawerDescriptioncomponents to prevent errors suggests that the base Drawer component could be improved.Consider enhancing the Drawer component to:
- Make title and description optional
- Add built-in support for loading states
- Use proper accessibility patterns for loading states
76-103: Simplify conditional rendering and extract stylesThe current implementation has multiple areas for improvement:
- Simplify the null checks using optional chaining:
- {!loading && - deployment != null && - resource != null && - environment != null && ( + {!loading && deployment?.name && resource?.name && environment?.name && (
- Extract the hardcoded styles into a CSS module or Tailwind component:
const drawerContentStyles = "scrollbar-thin scrollbar-thumb-neutral-800 scrollbar-track-neutral-900 left-auto right-0 top-0 mt-0 h-screen w-2/3 max-w-7xl overflow-auto rounded-none focus-visible:outline-none";apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/JobDropdownMenu.tsx (1)
94-94: Consider optimizing cache invalidation strategyWhile the current cache invalidation approach works, consider implementing a more granular invalidation strategy. Instead of invalidating the entire
byDeploymentEnvAndResourcecache, you could invalidate only the specific keys affected by the mutation.Example approach:
// Instead of utils.job.config.byDeploymentEnvAndResource.invalidate() // Consider utils.job.config.byDeploymentEnvAndResource.invalidate({ environmentId, resourceId })This would help improve performance by preventing unnecessary re-fetches of unaffected data.
Also applies to: 223-225, 276-278
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/TableRow.tsx (1)
114-143: Handle cases with more than 3 links inLinksCell.Currently, when there are more than three links, the
LinksCellrenders an empty cell. Consider implementing a way to display additional links, such as showing all links, adding a "View more" option, or using a dropdown menu to enhance user experience.packages/api/src/router/job.ts (1)
318-337: Optimize authorization checks with parallel executionCurrently, the authorization checks for deployment, environment, and resource permissions are performed sequentially using
Promise.all:return Promise.all([ canUserGetDeploymentPromise, canUserGetEnvironmentPromise, canUserGetResourcePromise, ]).then( ([deployment, environment, resource]) => deployment && environment && resource, );You can optimize this by initiating the permission checks simultaneously, reducing the total authorization time, especially if these checks involve I/O operations.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (9)
apps/webservice/src/app/[workspaceSlug]/(app)/(targets)/targets/[targetId]/visualize/nodes/DeploymentNode.tsx(3 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/ReleaseTable.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/TableRow.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/useDeploymentResourceDrawer.ts(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/layout.tsx(2 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/deployments/DeployButton.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/JobDropdownMenu.tsx(7 hunks)packages/api/src/router/job.ts(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/TableRow.tsx
[error] 333-347: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (16)
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/deployments/DeployButton.tsx (1)
13-14: LGTM! Clean implementation of the optional className prop.
The className prop addition follows React best practices and TypeScript conventions.
apps/webservice/src/app/[workspaceSlug]/(app)/layout.tsx (1)
8-8: LGTM!
The import statement follows the established pattern and correctly references the new deployment resource drawer component.
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/ReleaseTable.tsx (1)
13-13: Verify the TableRow component implementation.
Let's ensure that the imported ReleaseRows component exists and is properly implemented.
✅ Verification successful
ReleaseRows component is properly implemented and exported
The ReleaseRows component is correctly implemented in TableRow.tsx with proper type definitions, props interface, and functionality. It's a React functional component that handles release job triggers with collapsible rows, making it suitable for its intended use in ReleaseTable.tsx.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if TableRow.tsx exists and exports ReleaseRows
fd -e tsx -e ts "TableRow" -x cat {} | grep -A 5 "export.*ReleaseRows"
Length of output: 190
Script:
#!/bin/bash
# Let's get more context about the ReleaseRows component implementation
fd -e tsx -e ts "TableRow" -x cat {}
Length of output: 9880
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/useDeploymentResourceDrawer.ts (3)
13-15: LGTM!
Clean hook implementation following React best practices.
64-70: LGTM!
Clean and complete return object providing all necessary values and setter.
13-70: Verify error handling in consuming components
Let's verify that components using this hook properly handle null states and implement appropriate error boundaries.
✅ Verification successful
Let me gather more context about the error handling in the component.
Let me try a different approach using ripgrep to find and examine the component's implementation.
Let me try one more time without specifying the file type.
Error handling is properly implemented in the consuming component
The component using the hook implements proper error handling and null state management:
- Checks for null states using the
isOpenflag that validates all IDs are present - Handles loading states with a dedicated loading indicator
- Uses null coalescing for API queries with fallback empty values
- Enables queries only when required IDs are present
- Properly handles drawer state changes through the
setDeploymentEnvResourceIdfunction
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using this hook and check their error handling
ast-grep --pattern 'const { $$$, deploymentId, environmentId, resourceId, $$$} = useDeploymentEnvResourceDrawer()' -A 10
Length of output: 2499
Script:
#!/bin/bash
# Get the full implementation of DeploymentResourceDrawer to check error handling
cat apps/webservice/src/app/[workspaceSlug]\(app\)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx
Length of output: 267
Script:
#!/bin/bash
# Search for the DeploymentResourceDrawer component implementation
rg -A 50 "export.*DeploymentResourceDrawer.*=.*" --type typescript
Length of output: 103
Script:
#!/bin/bash
# Search for the DeploymentResourceDrawer component implementation
rg -A 50 "export.*DeploymentResourceDrawer.*=.*"
Length of output: 7538
apps/webservice/src/app/[workspaceSlug]/(app)/(targets)/targets/[targetId]/visualize/nodes/DeploymentNode.tsx (2)
52-52: LGTM! Clean implementation of the click handler.
The changes improve consistency with:
- Uniform cursor styling for better UX
- Clear parameter passing in the click handler
Also applies to: 58-60
22-22: Verify the hook implementation and error handling.
The migration from useJobDrawer to useDeploymentEnvResourceDrawer looks good. Let's verify the hook's implementation:
✅ Verification successful
Let me gather more information about the hook implementation since the first search didn't show the complete hook definition.
Let me try a different approach to read the file content by using fd to find the exact path first.
Let me try one last time with a simpler pattern to get the hook implementation.
Based on the implementation shown, I can now generate the final response.
The hook implementation is properly validated and secure
The useDeploymentEnvResourceDrawer hook includes:
- Proper parameter validation using UUID regex pattern
- Null checks for all parameters
- URL-safe encoding/decoding of parameters
- Clean error state handling by returning null values when validation fails
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the hook implementation for proper error handling
ast-grep --pattern 'export function useDeploymentEnvResourceDrawer() {
$$$
}'
# Check for proper parameter validation
rg -A 5 "setDeploymentEnvResourceId.*=.*"
Length of output: 1895
Script:
#!/bin/bash
# Search for the hook implementation file content
cat apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/useDeploymentResourceDrawer.ts
# Check if there are any error boundaries or error handling related to this drawer
rg -A 5 "DeploymentEnvResource.*Error"
Length of output: 302
Script:
#!/bin/bash
# Find the exact path of the hook implementation file
fd useDeploymentResourceDrawer.ts
# Use ripgrep to show the complete hook implementation
rg -U "export.*useDeploymentEnvResourceDrawer.*{[\s\S]*?}" --multiline
Length of output: 398
Script:
#!/bin/bash
# Use ripgrep to show the content around the hook implementation
rg -B 2 -A 20 "export.*useDeploymentEnvResourceDrawer"
# Alternative search for the hook name
rg -B 2 -A 20 "export.*useDeploymentResourceDrawer"
Length of output: 3644
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx (2)
95-100: Verify ReleaseTable component integration
Ensure that the ReleaseTable component properly handles all the props being passed to it.
✅ Verification successful
ReleaseTable component integration is properly implemented
The component implementation matches exactly with the props being passed:
ReleaseTablePropstype definition inReleaseTable.tsxdeclares all required props:releasesWithTriggers,environment,deployment, andresource- The props passed in
DeploymentResourceDrawer.tsxmatch the type definition exactly
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check ReleaseTable component implementation
ast-grep --pattern 'export $_ ReleaseTable: React.FC<{
$$$
}>'
Length of output: 66
Script:
#!/bin/bash
# Let's try a broader search for ReleaseTable component definition
ast-grep --pattern 'const ReleaseTable = $_'
# Also search for ReleaseTable type definition
ast-grep --pattern 'type ReleaseTableProps = {
$$$
}'
# Backup search using ripgrep to find any ReleaseTable references
rg "ReleaseTable" -A 5
Length of output: 8213
14-14:
Fix inconsistent hook naming
The imported hook name useDeploymentResourceDrawer doesn't match the used hook name useDeploymentEnvResourceDrawer. This inconsistency should be resolved to maintain code clarity.
apps/webservice/src/app/[workspaceSlug]/(app)/systems/[systemSlug]/deployments/[deploymentSlug]/releases/[versionId]/JobDropdownMenu.tsx (4)
94-94: LGTM: Cache invalidation and event handling improvements
The changes improve the component's behavior in two ways:
- Added cache invalidation ensures the UI stays in sync with the latest deployment environment and resource state
- Preventing event propagation in DialogContent maintains proper modal interaction
Also applies to: 108-108
187-187: LGTM: Consistent implementation of cache management
The changes follow the same pattern as the OverrideJobStatusDialog:
- Cache invalidation ensures UI consistency after force release
- Event propagation handling prevents unwanted click behavior
Also applies to: 194-194, 223-225
246-246: LGTM: Consistent implementation across components
The changes maintain consistency with other dialog components:
- Cache invalidation for deployment environment and resource
- Event propagation handling for proper modal behavior
Also applies to: 252-252, 276-278
314-314: LGTM: Consistent event handling in dropdown menu
Added stopPropagation to prevent unwanted menu closing when interacting with menu items.
packages/api/src/router/job.ts (2)
386-386: Ensure releaseMatchesCondition can handle an undefined filter
At line 386, releaseMatchesCondition is called with filter which might be undefined:
releaseMatchesCondition(ctx.db, filter)Verify that releaseMatchesCondition gracefully handles a null or undefined filter without throwing an error. If it doesn't, you should add a check before calling it, or modify the function to handle such cases.
396-409: Prevent duplicate releases in the query results
When processing the query results, you group releases by their ID:
_.chain(rows)
.groupBy((r) => r.release.id)
.map((r) => ({
...r[0]!.release,
releaseJobTriggers: r
.filter((r) => r.release_job_trigger != null)
.map((r) => ({
...r.release_job_trigger!,
job: r.job!,
})),
}))
.value();However, if a release has multiple releaseJobTriggers, it might still appear multiple times in the result set before grouping. Ensure that the grouping correctly consolidates all triggers under their respective releases to prevent duplicates in the final output.
| const setDeploymentEnvResourceId = ( | ||
| deploymentId: string | null, | ||
| environmentId: string | null, | ||
| resourceId: string | null, | ||
| ) => | ||
| setParams({ | ||
| [param]: | ||
| deploymentId == null || environmentId == null || resourceId == null | ||
| ? null | ||
| : encodeURIComponent( | ||
| `${deploymentId}${DELIMITER}${environmentId}${DELIMITER}${resourceId}`, | ||
| ), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding UUID validation in setter method
While the getter validates UUIDs, the setter accepts any string input. Consider adding the same UUID validation to prevent invalid states from being set.
const setDeploymentEnvResourceId = (
deploymentId: string | null,
environmentId: string | null,
resourceId: string | null,
) => {
+ if (deploymentId && !UUID_REGEX.test(deploymentId)) {
+ console.warn('Invalid deployment ID format');
+ return;
+ }
+ if (environmentId && !UUID_REGEX.test(environmentId)) {
+ console.warn('Invalid environment ID format');
+ return;
+ }
+ if (resourceId && !UUID_REGEX.test(resourceId)) {
+ console.warn('Invalid resource ID format');
+ return;
+ }
setParams({
[param]:
deploymentId == null || environmentId == null || resourceId == null
? null
: encodeURIComponent(
`${deploymentId}${DELIMITER}${environmentId}${DELIMITER}${resourceId}`,
),
});
};Committable suggestion skipped: line range outside the PR's diff.
| api.job.config.byDeploymentEnvAndResource.useQuery( | ||
| { | ||
| deploymentId: deploymentId ?? "", | ||
| environmentId: environmentId ?? "", | ||
| resourceId: resourceId ?? "", | ||
| }, | ||
| { enabled: isOpen, refetchInterval: 5_000 }, | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize query conditions to prevent unnecessary API calls
The current implementation uses empty strings when IDs are null, which could trigger unnecessary API calls. Consider enhancing the enabled condition to check for valid IDs:
api.job.config.byDeploymentEnvAndResource.useQuery(
{
- deploymentId: deploymentId ?? "",
- environmentId: environmentId ?? "",
- resourceId: resourceId ?? "",
+ deploymentId,
+ environmentId,
+ resourceId,
},
- { enabled: isOpen, refetchInterval: 5_000 },
+ {
+ enabled: isOpen && deploymentId && environmentId && resourceId,
+ refetchInterval: 5_000
+ },
);📝 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.
| api.job.config.byDeploymentEnvAndResource.useQuery( | |
| { | |
| deploymentId: deploymentId ?? "", | |
| environmentId: environmentId ?? "", | |
| resourceId: resourceId ?? "", | |
| }, | |
| { enabled: isOpen, refetchInterval: 5_000 }, | |
| ); | |
| api.job.config.byDeploymentEnvAndResource.useQuery( | |
| { | |
| deploymentId, | |
| environmentId, | |
| resourceId, | |
| }, | |
| { | |
| enabled: isOpen && deploymentId && environmentId && resourceId, | |
| refetchInterval: 5_000 | |
| }, | |
| ); |
There was a problem hiding this comment.
isOpen already checks if the vars are null
There was a problem hiding this comment.
@adityachoudhari26, understood. Since isOpen already checks if the variables are null, the current implementation is appropriate.
✏️ Learnings added
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#237
File: apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx:43-50
Timestamp: 2024-11-27T23:16:35.378Z
Learning: In `DeploymentResourceDrawer.tsx`, the `isOpen` variable already checks whether `deploymentId`, `environmentId`, and `resourceId` are non-null, so additional null checks in query `enabled` conditions are not necessary.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| <> | ||
| {releaseJobTriggers.map((trigger, idx) => { | ||
| if (idx === 0) return null; | ||
| return ( | ||
| <ReleaseJobTriggerChildRow | ||
| release={release} | ||
| releaseJobTrigger={trigger} | ||
| environment={environment} | ||
| deployment={deployment} | ||
| resource={resource} | ||
| key={trigger.id} | ||
| /> | ||
| ); | ||
| })} | ||
| </> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary Fragment in CollapsibleContent.
The Fragment wrapping the mapped child rows is unnecessary since it contains only one child. Removing it simplifies the code without affecting functionality.
Apply this diff to remove the redundant Fragment:
<CollapsibleContent asChild>
- <>
{releaseJobTriggers.map((trigger, idx) => {
if (idx === 0) return null;
return (
<ReleaseJobTriggerChildRow
release={release}
releaseJobTrigger={trigger}
environment={environment}
deployment={deployment}
resource={resource}
key={trigger.id}
/>
);
})}
- </>
</CollapsibleContent>📝 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.
| <> | |
| {releaseJobTriggers.map((trigger, idx) => { | |
| if (idx === 0) return null; | |
| return ( | |
| <ReleaseJobTriggerChildRow | |
| release={release} | |
| releaseJobTrigger={trigger} | |
| environment={environment} | |
| deployment={deployment} | |
| resource={resource} | |
| key={trigger.id} | |
| /> | |
| ); | |
| })} | |
| </> | |
| {releaseJobTriggers.map((trigger, idx) => { | |
| if (idx === 0) return null; | |
| return ( | |
| <ReleaseJobTriggerChildRow | |
| release={release} | |
| releaseJobTrigger={trigger} | |
| environment={environment} | |
| deployment={deployment} | |
| resource={resource} | |
| key={trigger.id} | |
| /> | |
| ); | |
| })} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 333-347: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
packages/api/src/router/job.ts
Outdated
| authorizationCheck: ({ canUser, input }) => { | ||
| const { deploymentId, environmentId, resourceId } = input; | ||
| const canUserGetDeploymentPromise = canUser | ||
| .perform(Permission.DeploymentGet) | ||
| .on({ type: "deployment", id: deploymentId }); | ||
| const canUserGetEnvironmentPromise = canUser | ||
| .perform(Permission.SystemGet) | ||
| .on({ type: "environment", id: environmentId }); | ||
| const canUserGetResourcePromise = canUser | ||
| .perform(Permission.ResourceGet) | ||
| .on({ type: "resource", id: resourceId }); | ||
| return Promise.all([ | ||
| canUserGetDeploymentPromise, | ||
| canUserGetEnvironmentPromise, | ||
| canUserGetResourcePromise, | ||
| ]).then( | ||
| ([deployment, environment, resource]) => | ||
| deployment && environment && resource, | ||
| ); | ||
| }, |
There was a problem hiding this comment.
Incorrect permission used for environment authorization
In the authorizationCheck at lines 318-337, you're using Permission.SystemGet to authorize access to an environment:
const canUserGetEnvironmentPromise = canUser
.perform(Permission.SystemGet)
.on({ type: "environment", id: environmentId });This might be a mistake, as environments may have their own specific permissions. Consider using Permission.EnvironmentGet instead to accurately represent the permission required to access an environment.
| const releaseChannelId = | ||
| env.releaseChannels.at(0)?.channelId ?? | ||
| env.policy?.environmentPolicyReleaseChannels.at(0)?.channelId; | ||
|
|
There was a problem hiding this comment.
Handle cases where no release channel is associated with the environment
When determining the releaseChannelId, the code uses:
const releaseChannelId =
env.releaseChannels.at(0)?.channelId ??
env.policy?.environmentPolicyReleaseChannels.at(0)?.channelId;If both env.releaseChannels and env.policy?.environmentPolicyReleaseChannels are empty or undefined, releaseChannelId will be undefined, leading to rc being undefined later. This means filter at line 374 will also be undefined, which might cause issues in the subsequent query.
Ensure that your code handles situations where there is no associated release channel. You might need to provide a default filter or handle the absence of a filter explicitly to prevent runtime errors.
There was a problem hiding this comment.
this is handled because the releaseMatchesCondition returns undefined if the filter is null, which means the check will not happen in the and(..)
There was a problem hiding this comment.
@adityachoudhari26, thanks for the clarification. It's great that releaseMatchesCondition returns undefined when filter is null, ensuring that the absence of a release channel doesn't cause any issues in the query.
✏️ Learnings added
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#237
File: packages/api/src/router/job.ts:362-365
Timestamp: 2024-11-27T23:18:41.874Z
Learning: In the file `packages/api/src/router/job.ts`, the function `releaseMatchesCondition` returns undefined if the `filter` parameter is null. This behavior ensures that when constructing the query with `and(...)`, the condition is omitted, allowing the query to function correctly even if there is no release channel associated with the environment.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| and( | ||
| releaseMatchesCondition(ctx.db, filter), | ||
| eq(release.deploymentId, deploymentId), | ||
| or( | ||
| isNull(releaseJobTrigger.id), | ||
| eq(releaseJobTrigger.resourceId, resourceId), | ||
| ), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potentially including unintended releases in the query result
In the where clause of the query starting at line 385, the condition:
or(
isNull(releaseJobTrigger.id),
eq(releaseJobTrigger.resourceId, resourceId),
),could lead to including releases that have no associated releaseJobTrigger or ones that are associated with a different resourceId. This might not be the intended behavior if you aim to fetch releases specifically related to the given resourceId.
Consider revising the condition to ensure only relevant releases are included. For example, you might want to exclude releases without a releaseJobTrigger or ensure that only those with a matching resourceId are fetched.
There was a problem hiding this comment.
releases are not scoped to a specific resource
There was a problem hiding this comment.
@adityachoudhari26 Thank you for the clarification. I understand that releases are not scoped to a specific resource.
(_/)
(•.•) ?
✏️ Learnings added
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#237
File: packages/api/src/router/job.ts:382-390
Timestamp: 2024-11-27T23:18:59.130Z
Learning: In the codebase, releases are not scoped to a specific resource.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/TableRow.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/TableRow.tsx
[error] 389-403: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (1)
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/TableRow.tsx (1)
389-403: Remove unnecessary Fragment in CollapsibleContent.
The Fragment wrapping the mapped child rows is unnecessary since it contains only one child. Removing it simplifies the code without affecting functionality.
Apply this diff to remove the redundant Fragment:
<CollapsibleContent asChild>
- <>
{releaseJobTriggers.map((trigger, idx) => {
if (idx === 0) return null;
return (
<ReleaseJobTriggerChildRow
release={release}
releaseJobTrigger={trigger}
environment={environment}
deployment={deployment}
resource={resource}
key={trigger.id}
/>
);
})}
- </>
</CollapsibleContent>🧰 Tools
🪛 Biome (1.9.4)
[error] 389-403: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
| ? (JSON.parse(linksMetadata.value) as Record<string, string>) | ||
| : null; |
There was a problem hiding this comment.
Add error handling for JSON.parse to prevent runtime exceptions.
Parsing JSON without error handling can lead to runtime exceptions if the data is malformed. It's advisable to wrap JSON.parse in a try-catch block to handle potential errors gracefully.
Apply this diff to add error handling:
const links =
linksMetadata != null
- ? (JSON.parse(linksMetadata.value) as Record<string, string>)
+ ? (() => {
+ try {
+ return JSON.parse(linksMetadata.value) as Record<string, string>;
+ } catch (error) {
+ console.error("Failed to parse links metadata:", error);
+ return null;
+ }
+ })()
: null;📝 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.
| ? (JSON.parse(linksMetadata.value) as Record<string, string>) | |
| : null; | |
| ? (() => { | |
| try { | |
| return JSON.parse(linksMetadata.value) as Record<string, string>; | |
| } catch (error) { | |
| console.error("Failed to parse links metadata:", error); | |
| return null; | |
| } | |
| })() | |
| : null; |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
packages/validators/src/auth/index.ts (1)
77-78: Consider completing the CRUD permission set for environments.Most resources in the system have a complete set of CRUD operations (create, read, update, delete) plus list. However, the environment permissions only include
getandupdate. Consider adding:
EnvironmentCreateEnvironmentDeleteEnvironmentListThis would maintain consistency with other resources and provide flexibility for future features.
EnvironmentGet = "environment.get", EnvironmentUpdate = "environment.update", +EnvironmentCreate = "environment.create", +EnvironmentDelete = "environment.delete", +EnvironmentList = "environment.list",apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/TableRow.tsx (4)
61-67: Add ARIA attributes for better accessibility.The click handler's div should have appropriate ARIA attributes to indicate it's interactive.
- <div onClick={(e) => e.stopPropagation()}> + <div + onClick={(e) => e.stopPropagation()} + role="button" + aria-label="Deploy release" + >
95-200: Optimize component performance with useMemo.The component performs complex rendering logic that could benefit from memoization to prevent unnecessary re-renders.
+import { useMemo } from "react"; const LinksCell: React.FC<LinksCellProps> = ({ releaseJobTrigger }) => { const jobQ = api.job.config.byId.useQuery(releaseJobTrigger?.job.id ?? "", { enabled: releaseJobTrigger != null, refetchInterval: 5_000, }); const job = jobQ.data; - const linksMetadata = job?.job.metadata.find( - (m) => m.key === String(ReservedMetadataKey.Links), - ); + const linksMetadata = useMemo(() => + job?.job.metadata.find( + (m) => m.key === String(ReservedMetadataKey.Links), + ), + [job?.job.metadata] + );
225-229: Improve type safety for job status.Instead of type assertion, consider validating the status value.
+const isJobStatus = (status: string): status is JobStatus => { + return Object.values(JobStatus).includes(status as JobStatus); +}; job={{ id: releaseJobTrigger.job.id, - status: releaseJobTrigger.job.status as JobStatus, + status: isJobStatus(releaseJobTrigger.job.status) + ? releaseJobTrigger.job.status + : JobStatus.UNKNOWN, }}
266-274: Enhance keyboard accessibility for table rows.The table rows should be keyboard accessible since they're interactive.
<TableRow key={releaseJobTrigger?.id ?? release.id} onClick={() => router.push(/* ... */) } - className="cursor-pointer" + className="cursor-pointer focus:outline-none focus:ring-2 focus:ring-primary" + role="link" + tabIndex={0} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + router.push(/* ... */); + } + }} >Also applies to: 324-332
packages/api/src/router/job.ts (1)
391-391: Document the reason for limiting to 50 records.The query includes a hard-coded limit of 50 records. Consider adding a comment explaining this limitation or make it configurable via input parameters.
- .limit(50) + // Limit to 50 records to prevent excessive data loading in the UI + .limit(input.limit ?? 50)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/TableRow.tsx(1 hunks)packages/api/src/router/job.ts(3 hunks)packages/validators/src/auth/index.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/TableRow.tsx
[error] 386-400: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (6)
packages/validators/src/auth/index.ts (1)
77-78: LGTM! The new environment permissions are well-integrated.
The new permissions follow the established naming patterns and are properly integrated into the predefined roles through the existing filtering logic.
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/TableRow.tsx (2)
104-107:
Add error handling for JSON.parse.
The JSON parsing operation could throw an exception if the metadata value is malformed.
const links =
linksMetadata != null
- ? (JSON.parse(linksMetadata.value) as Record<string, string>)
+ ? (() => {
+ try {
+ return JSON.parse(linksMetadata.value) as Record<string, string>;
+ } catch (error) {
+ console.error("Failed to parse links metadata:", error);
+ return null;
+ }
+ })()
: null;386-400: 🛠️ Refactor suggestion
Remove unnecessary Fragment wrapper.
The Fragment wrapping the mapped child rows is unnecessary since it contains only one child.
<CollapsibleContent asChild>
- <>
{releaseJobTriggers.map((trigger, idx) => {
if (idx === 0) return null;
return (
<ReleaseJobTriggerChildRow
release={release}
releaseJobTrigger={trigger}
environment={environment}
deployment={deployment}
resource={resource}
key={trigger.id}
/>
);
})}
- </>
</CollapsibleContent>Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 386-400: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
packages/api/src/router/job.ts (3)
309-335: LGTM! Well-structured permission checks.
The authorization implementation follows security best practices by:
- Checking all required permissions independently
- Using parallel execution with Promise.all
- Ensuring all permissions are granted before proceeding
362-364: Handle cases where no release channel is associated.
The code might result in undefined releaseChannelId if both env.releaseChannels and env.policy?.environmentPolicyReleaseChannels are empty arrays.
385-388: Review the OR condition in the where clause.
The condition could include releases that have no associated releaseJobTrigger, which might not be the intended behavior.
| {releasesWithTriggers.map((release) => { | ||
| return ( | ||
| <ReleaseRows | ||
| release={release} | ||
| environment={environment} | ||
| deployment={deployment} | ||
| resource={resource} | ||
| key={release.id} | ||
| /> | ||
| ); | ||
| })} |
| const UUID_REGEX = | ||
| /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; |
| deploymentId == null || environmentId == null || resourceId == null | ||
| ? null | ||
| : encodeURIComponent( | ||
| `${deploymentId}${DELIMITER}${environmentId}${DELIMITER}${resourceId}`, |
There was a problem hiding this comment.
| `${deploymentId}${DELIMITER}${environmentId}${DELIMITER}${resourceId}`, | |
| [deploymentId, environmentId, resourceId].join(DELIMITER), |
| const rc = releaseChannelId | ||
| ? await ctx.db.query.releaseChannel.findFirst({ | ||
| where: eq(releaseChannel.id, releaseChannelId), | ||
| }) | ||
| : undefined; |
There was a problem hiding this comment.
| const rc = releaseChannelId | |
| ? await ctx.db.query.releaseChannel.findFirst({ | |
| where: eq(releaseChannel.id, releaseChannelId), | |
| }) | |
| : undefined; | |
| const {filter} = releaseChannelId | |
| ? await ctx.db.query.releaseChannel.findFirst({ | |
| where: eq(releaseChannel.id, releaseChannelId), | |
| }) | |
| :{}; |
There was a problem hiding this comment.
ts does not like this
| const deploymentQ = api.deployment.byId.useQuery(deploymentId ?? "", { | ||
| enabled: isOpen, | ||
| }); | ||
| const deployment = deploymentQ.data; |
There was a problem hiding this comment.
| const deploymentQ = api.deployment.byId.useQuery(deploymentId ?? "", { | |
| enabled: isOpen, | |
| }); | |
| const deployment = deploymentQ.data; | |
| const {data: deployment, ...deploymentQ} = api.deployment.byId.useQuery(deploymentId ?? "", { | |
| enabled: isOpen, | |
| }); |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/api/src/router/job.ts (2)
357-366: Consider handling potential race conditionsThe code retrieves the release channel in two steps: first getting the ID, then querying for the channel. This could lead to inconsistencies if the channel is deleted between queries.
Consider using a single query with a join:
- const releaseChannelId = - env.releaseChannels.at(0)?.channelId ?? - env.policy?.environmentPolicyReleaseChannels.at(0)?.channelId; - - const rc = releaseChannelId - ? await ctx.db.query.releaseChannel.findFirst({ - where: eq(releaseChannel.id, releaseChannelId), - }) - : undefined; + const rc = await ctx.db.query.releaseChannel.findFirst({ + where: eq(releaseChannel.id, + env.releaseChannels.at(0)?.channelId ?? + env.policy?.environmentPolicyReleaseChannels.at(0)?.channelId ?? '' + ), + });
368-401: Consider adding pagination supportThe query has a hard limit of 50 records. For better scalability and user experience:
- Make the limit configurable via input parameters
- Add offset-based or cursor-based pagination
- Return total count for UI pagination controls
Example implementation:
.input( z.object({ deploymentId: z.string().uuid(), environmentId: z.string().uuid(), resourceId: z.string().uuid(), + limit: z.number().int().min(1).max(100).default(50), + offset: z.number().int().min(0).default(0), }), )- .limit(50) + .limit(input.limit) + .offset(input.offset)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/ReleaseTable.tsx(1 hunks)packages/api/src/router/job.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/ReleaseTable.tsx
🧰 Additional context used
📓 Learnings (1)
packages/api/src/router/job.ts (2)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#237
File: packages/api/src/router/job.ts:382-390
Timestamp: 2024-11-27T23:18:59.456Z
Learning: In the codebase, releases are not scoped to a specific resource.
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#237
File: packages/api/src/router/job.ts:362-365
Timestamp: 2024-11-27T23:18:42.055Z
Learning: In the file `packages/api/src/router/job.ts`, the function `releaseMatchesCondition` returns undefined if the `filter` parameter is null. This behavior ensures that when constructing the query with `and(...)`, the condition is omitted, allowing the query to function correctly even if there is no release channel associated with the environment.
🔇 Additional comments (2)
packages/api/src/router/job.ts (2)
Line range hint 30-54: LGTM: Imports are properly organized
The new imports are necessary for the added functionality and are correctly imported from their respective modules.
318-329: LGTM: Comprehensive authorization check
The authorization check properly verifies permissions for all three entities (deployment, environment, and resource) using the correct permission types.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/useDeploymentResourceDrawer.ts (1)
16-41: Optimize the validation logicThe validation logic is thorough but could be more efficient and maintainable.
Consider this refactoring:
const { deploymentId, environmentId, resourceId } = useMemo(() => { + const nullResult = { deploymentId: null, environmentId: null, resourceId: null }; + if (deploymentResourceId == null) - return { deploymentId: null, environmentId: null, resourceId: null }; + return nullResult; const [rawDeploymentId, rawEnvironmentId, rawResourceId] = decodeURIComponent(deploymentResourceId).split(DELIMITER); + + const idSchema = z.string().uuid(); + const isValidUUID = (id: string) => idSchema.safeParse(id).success; + if ( rawDeploymentId == null || rawEnvironmentId == null || rawResourceId == null || + !isValidUUID(rawDeploymentId) || + !isValidUUID(rawEnvironmentId) || + !isValidUUID(rawResourceId) ) - return { deploymentId: null, environmentId: null, resourceId: null }; - - if ( - !z.string().uuid().safeParse(rawDeploymentId).success || - !z.string().uuid().safeParse(rawEnvironmentId).success || - !z.string().uuid().safeParse(rawResourceId).success - ) - return { deploymentId: null, environmentId: null, resourceId: null }; + return nullResult; return { deploymentId: rawDeploymentId, environmentId: rawEnvironmentId, resourceId: rawResourceId, }; }, [deploymentResourceId]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/useDeploymentResourceDrawer.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx
🔇 Additional comments (4)
apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/useDeploymentResourceDrawer.ts (4)
1-11: LGTM: Imports and constants are well-structured
The imports are appropriate, and the constants are well-defined. The delimiter choice is URL-safe.
12-15: LGTM: Clean hook setup with URL-based state management
Good use of query parameters for maintaining shareable state in URLs.
63-69: LGTM: Clean and complete hook return value
The return structure provides all necessary values and follows React hooks best practices.
50-62:
Add UUID validation in setter method
The setter should validate UUIDs before updating the state to maintain data consistency.
Add validation as previously suggested:
const setDeploymentEnvResourceId = (
deploymentId: string | null,
environmentId: string | null,
resourceId: string | null,
) => {
+ const idSchema = z.string().uuid();
+ if (deploymentId && !idSchema.safeParse(deploymentId).success) {
+ console.warn('Invalid deployment ID format');
+ return;
+ }
+ if (environmentId && !idSchema.safeParse(environmentId).success) {
+ console.warn('Invalid environment ID format');
+ return;
+ }
+ if (resourceId && !idSchema.safeParse(resourceId).success) {
+ console.warn('Invalid resource ID format');
+ return;
+ }
setParams({
[param]:
deploymentId == null || environmentId == null || resourceId == null
? null
: encodeURIComponent(
[deploymentId, environmentId, resourceId].join(DELIMITER),
),
});
};
Summary by CodeRabbit
New Features
DeploymentResourceDrawercomponent for managing deployment resources.ReleaseTablecomponent to display associated releases in a structured format.TableRowcomponents for rendering rows in the release table.DeployButtonto accept custom styling via an optionalclassNameprop.JobDropdownMenu.EnvironmentGetfor authorization checks.byDeploymentEnvAndResourcefor querying job triggers based on deployment, environment, and resource IDs.Bug Fixes
Documentation