feat: Adding patch endpoint for deployment #382
Conversation
WalkthroughThis pull request introduces a new PATCH operation to update existing deployments. The changes include updating the OpenAPI specifications (both in TypeScript and JSON formats) to define the new endpoint and its associated schemas, parameters, and responses. Additionally, a corresponding PATCH route handler has been implemented with authentication, authorization, and error handling to process update requests for deployments. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant R as Route Handler
participant DB as Database
C->>R: PATCH /deployments/{deploymentId} (with update data)
R->>R: Authenticate & Authorize (DeploymentUpdate)
R->>DB: Fetch deployment by ID
DB-->>R: Deployment found / not found
alt Deployment exists
R->>DB: Update deployment
DB-->>R: Updated deployment info
R-->>C: 200 OK with updated data
else Deployment not found
R-->>C: 404 Not Found error
end
Possibly Related PRs
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 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: 3
🧹 Nitpick comments (3)
apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts (3)
82-86: Consider using consistent authorization patternThe authorization implementation in the PATCH route uses an async function with await on params, unlike the GET and DELETE methods. While this works, it introduces inconsistency in the codebase pattern.
Consider aligning with the approach used in other routes for better code consistency:
- authz(async ({ can, extra: { params } }) => { - return can - .perform(Permission.DeploymentUpdate) - .on({ type: "deployment", id: (await params).deploymentId }); - }), + authz(({ can, extra: { params } }) => + can + .perform(Permission.DeploymentUpdate) + .on({ type: "deployment", id: params.deploymentId }), + ),
88-89: Review type definition for paramsUnlike GET and DELETE routes that use a direct string type for deploymentId, the PATCH route uses a Promise wrapped type. This creates inconsistency with other route handlers.
Consider adapting the type to match other route handlers for consistency:
- .handle<{ db: Tx }, { params: Promise<{ deploymentId: string }> }>( + .handle<{ db: Tx }, { params: { deploymentId: string } }>(
117-117: Redundant error object in loggingThe error logging includes the error object twice, which is redundant.
- logger.error("Failed to update deployment", { error: error }); + logger.error("Failed to update deployment", { error });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webservice/src/app/api/v1/deployments/[deploymentId]/openapi.ts(1 hunks)apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts(1 hunks)openapi.v1.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict en...
**/*.{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/api/v1/deployments/[deploymentId]/route.tsapps/webservice/src/app/api/v1/deployments/[deploymentId]/openapi.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (linux/amd64)
- GitHub Check: Typecheck
- GitHub Check: Lint
🔇 Additional comments (1)
apps/webservice/src/app/api/v1/deployments/[deploymentId]/openapi.ts (1)
91-144:✅ Verification successful
PATCH operation schema configuration looks good
The OpenAPI specification for the PATCH operation is well-structured with appropriate parameters, request body, and response definitions.
Verify that the
UpdateDeploymentschema referenced in the request body is defined in your schema components:
🏁 Script executed:
#!/bin/bash # Search for the UpdateDeployment schema definition in the codebase rg "UpdateDeployment" --type tsLength of output: 468
PATCH Operation Schema Verified:
UpdateDeploymentSchema ExistsThe
UpdateDeploymentschema is defined inpackages/db/src/schema/deployment.tsand referenced correctly in the PATCH operation configuration inapps/webservice/src/app/api/v1/deployments/[deploymentId]/openapi.ts. No issues found.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts (2)
90-94: Simplify deploymentId extractionIf you keep the Promise-based params, consider simplifying the extraction of deploymentId with object destructuring:
- const deploymentId = (await params).deploymentId; + const { deploymentId } = await params;
116-117: Simplify error logging syntaxThe error object doesn't need to be wrapped in an additional object.
- logger.error("Failed to update deployment", { error: error }); + logger.error("Failed to update deployment", { error });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict en...
**/*.{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/api/v1/deployments/[deploymentId]/route.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
🔇 Additional comments (2)
apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts (2)
93-94: Missing request body validationThe request body is extracted but there's no validation of the input data, which could lead to security issues if invalid or malicious data is provided.
Consider adding validation for the request body using a schema validator or a manual check of required fields and types before proceeding with the database update.
107-113: Direct database update without field filteringThe implementation directly sets the entire request body to the database update without filtering fields, which could potentially allow updating fields that shouldn't be updatable (e.g., sensitive or system fields).
Add field filtering to ensure only allowed fields can be updated:
- .update(SCHEMA.deployment) - .set(body) + .update(SCHEMA.deployment) + .set({ + name: body.name, + description: body.description, + // Only include fields that should be updatable + })
| authz(async ({ can, extra: { params } }) => { | ||
| return can | ||
| .perform(Permission.DeploymentUpdate) | ||
| .on({ type: "deployment", id: (await params).deploymentId }); | ||
| }), | ||
| ) | ||
| .handle<{ db: Tx }, { params: Promise<{ deploymentId: string }> }>( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent deploymentId handling between endpoints
The authz middleware and handler type definitions in the PATCH endpoint handle params as a Promise, while GET and DELETE access it directly. This inconsistency could lead to confusion and potential bugs.
- authz(async ({ can, extra: { params } }) => {
- return can
- .perform(Permission.DeploymentUpdate)
- .on({ type: "deployment", id: (await params).deploymentId });
- }),
+ authz(({ can, extra: { params } }) =>
+ can
+ .perform(Permission.DeploymentUpdate)
+ .on({ type: "deployment", id: params.deploymentId }),
+ ),And update the handler type definition accordingly:
- .handle<{ db: Tx }, { params: Promise<{ deploymentId: string }> }>(
+ .handle<{ db: Tx }, { params: { deploymentId: string } }>(📝 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.
| authz(async ({ can, extra: { params } }) => { | |
| return can | |
| .perform(Permission.DeploymentUpdate) | |
| .on({ type: "deployment", id: (await params).deploymentId }); | |
| }), | |
| ) | |
| .handle<{ db: Tx }, { params: Promise<{ deploymentId: string }> }>( | |
| authz(({ can, extra: { params } }) => | |
| can | |
| .perform(Permission.DeploymentUpdate) | |
| .on({ type: "deployment", id: params.deploymentId }), | |
| ), | |
| ) | |
| .handle<{ db: Tx }, { params: { deploymentId: string } }>( |
There was a problem hiding this comment.
This was specifically done because In Next.js 15, the params prop in dynamic routes is handled as a Promise within async contexts.
Otherwise I get this error.
@ctrlplane/webservice:dev: Error: Route "/api/v1/deployments/[deploymentId]" used `params.deploymentId`. `params` should be awaited before using its properties. Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis
@ctrlplane/webservice:dev: at <unknown> (src/app/api/v1/deployments/[deploymentId]/route.ts:85:45)
@ctrlplane/webservice:dev: at <unknown> (src/app/api/v1/auth.ts:32:28)
@ctrlplane/webservice:dev: at next (src/app/api/v1/middleware.ts:46:15)
@ctrlplane/webservice:dev: at authn (src/app/api/v1/auth.ts:20:9)
@ctrlplane/webservice:dev: 83 | can
@ctrlplane/webservice:dev: 84 | .perform(Permission.DeploymentUpdate)
@ctrlplane/webservice:dev: > 85 | .on({ type: "deployment", id: params.deploymentId }),
@ctrlplane/webservice:dev: | ^
@ctrlplane/webservice:dev: 86 | ),
@ctrlplane/webservice:dev: 87 | )
@ctrlplane/webservice:dev: 88 | .handle<{ db: Tx }, { params: { deploymentId: string } }>(
@ctrlplane/webservice:dev: Error: Route "/api/v1/deployments/[deploymentId]" used `params.deploymentId`. `params` should be awaited before using its properties. Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis
@ctrlplane/webservice:dev: at <unknown> (src/app/api/v1/deployments/[deploymentId]/route.ts:91:36)
@ctrlplane/webservice:dev: at next (src/app/api/v1/middleware.ts:44:17)
@ctrlplane/webservice:dev: at <unknown> (src/app/api/v1/auth.ts:43:13)
@ctrlplane/webservice:dev: 89 | async ({ db, req }, { params }) => {
@ctrlplane/webservice:dev: 90 | try {
@ctrlplane/webservice:dev: > 91 | const deploymentId = params.deploymentId;
@ctrlplane/webservice:dev: | ^
@ctrlplane/webservice:dev: 92 |
@ctrlplane/webservice:dev: 93 | const body = await req.json();
@ctrlplane/webservice:dev: 94 |
If you have another solution, I am open to it. But I tried reverting both parts, and couldn't get this to work fully without both the await on the authz and the Promise on the params.
There was a problem hiding this comment.
🧩 Analysis chain
@zacharyblasczyk Thank you for the explanation! You're absolutely right about this Next.js 15 behavior change. In Next.js 15, route handlers must await dynamic route parameters in async contexts before accessing their properties.
Your implementation of handling params as a Promise is the correct approach given this constraint. However, I notice that the GET and DELETE endpoints haven't been updated to the same pattern yet - they might work now but could potentially face the same issue in certain scenarios.
For consistency across all endpoints, you might consider updating the GET and DELETE handlers to follow the same pattern:
// For GET and DELETE endpoints:
authz(async ({ can, extra: { params } }) =>
can
.perform(Permission.DeploymentGet/Delete)
.on({ type: "deployment", id: (await params).deploymentId }),
),And update their handler type definitions to match. This would make all three endpoints consistent while complying with Next.js 15's requirement.
ʕ •ᴥ•ʔ
🌐 Web query:
Next.js 15 dynamic route params as Promise in async contexts
💡 Result:
Next.js 15 introduces significant changes to how dynamic route parameters (params) are handled, requiring developers to adopt asynchronous patterns when working with routing data. Here's a breakdown of the key changes and implementation strategies:
Core Changes in Next.js 15
Dynamic route parameters now return Promises instead of direct values[5][8]. This enables better server-side optimizations and prepares for future features like Partial Prerendering (PPR)[8]. Attempting to access params synchronously will throw warnings/errors:
// ❌ Old approach (v14 and earlier)
function Page({ params }) {
return <div>{params.slug}</div> // Error in v15
}Implementation Patterns
Server Components
export default async function Page({ params }) {
const { slug } = await params // ✅ Proper async access
return <div>Post: {slug}</div>
}Client Components
'use client'
import { use } from 'react'
function ClientPage({ paramsPromise }) {
const { slug } = use(paramsPromise) // ✅ React hook for promises
return <div>{slug}</div>
}| Component Type | Access Method | Example |
|---|---|---|
| Server | await params |
const { id } = await params |
| Client | use(paramsPromise) |
const params = use(paramsPromise) |
TypeScript Integration
Next.js 15 provides enhanced type safety for dynamic routes. Use helper types with generateStaticParams:
export async function generateStaticParams() {
return [{ slug: 'post-1' }, { slug: 'post-2' }]
}
export default function Page({
params,
}: {
params: Promise<{ slug: string }>
}) {
// Type-safe params access
}Migration Strategies
- Codemod Automation:
npx @next/codemod@canary next-async-request-api . - Manual Updates:
- Convert synchronous
paramsaccess toawait params - Update Client Components to use React's
use()hook
- Convert synchronous
- Error Handling:
- Address
@next-codemod-errorcomments - Use
@next-codemod-ignorefor special cases[5]
- Address
Advanced Configuration
Control static generation with generateStaticParams:
export async function generateStaticParams() {
const posts = await fetchPosts()
return posts.map(post => ({ slug: post.slug }))
}
export const dynamicParams = false // Block unmapped routes[10]These changes enable Next.js to optimize both static generation and dynamic rendering while maintaining backward compatibility. Developers should prioritize migrating to async patterns to avoid deprecation warnings and ensure future compatibility[7][8].
Citations:
- 1: https://dev.to/adrianbailador/dynamic-routes-and-parameter-passing-in-nextjs-2l7e
- 2: https://community.auth0.com/t/error-in-nextjs-15/151480
- 3: https://dev.to/asross311/a-nextjs-15-dynamic-route-helper-type-139p
- 4: https://nextjs.org/docs/pages/building-your-application/routing/dynamic-routes
- 5: https://nextjs.org/docs/messages/sync-dynamic-apis
- 6: https://believemy.com/en/r/solving-the-params-should-be-awaited-error-on-nextjs-15
- 7: https://nextjs.org/docs/app/building-your-application/routing/dynamic-routes
- 8: https://nextjs.org/blog/next-15
- 9: Upgrading to Next v15.0.0 throws a directly accessed param warning vercel/next.js#71690
- 10: https://devanddeliver.com/blog/frontend/next-js-15-dynamic-routes-and-static-site-generation-ssg
- 11: https://www.youtube.com/watch?v=k9g6aVLH3p4
- 12: https://www.youtube.com/watch?v=qPsY4AKFlnM
- 13: Next 15: Dynamic routes vercel/next.js#72887
@zacharyblasczyk You're absolutely correct in your implementation! Next.js 15 indeed requires awaiting dynamic route parameters in async contexts before accessing their properties. Your approach with handling the params as a Promise and using (await params).deploymentId is exactly what Next.js 15 requires.
Based on the web search results, this is a significant architectural change in Next.js 15, where dynamic route parameters now return Promises instead of direct values. The error message you're seeing is the expected behavior when trying to access params synchronously in an async context.
For consistency across your codebase, you might want to eventually update the GET and DELETE endpoints to follow the same pattern, as they could potentially encounter similar issues in certain scenarios:
// For GET and DELETE endpoints:
authz(async ({ can, extra: { params } }) =>
can
.perform(Permission.DeploymentGet) // or DeploymentDelete
.on({ type: "deployment", id: (await params).deploymentId }),
),Next.js also provides a codemod to help automate these changes:
npx @next/codemod@canary next-async-request-api .Your current implementation for the PATCH endpoint is the correct approach, and I recommend keeping it as is.
ʕ •ᴥ•ʔ
✏️ Learnings added
Learnt from: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#382
File: apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts:82-88
Timestamp: 2025-03-16T19:41:44.019Z
Learning: In Next.js 15, params in dynamic routes must be awaited before accessing their properties in async contexts. For example, `params.deploymentId` should be accessed as `(await params).deploymentId` in async functions.
Learnt from: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#382
File: apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts:82-88
Timestamp: 2025-03-16T19:41:44.019Z
Learning: In Next.js 15, dynamic route parameters (params) return Promises instead of direct values in async contexts. When accessing properties like `params.deploymentId` in an async function, use `(await params).deploymentId` to avoid the "params should be awaited before using its properties" error. This applies to API routes, page components, and other async contexts.
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: 0
🧹 Nitpick comments (1)
openapi.v1.json (1)
2967-2978: Additional Deployment Properties Update
New properties—retryCount(integer),timeout(integer, nullable), andresourceFilter(object, nullable with additional properties)—have been added to the Deployment schema. These changes appear appropriate for capturing advanced configuration details. Please confirm that they are intended to be optional and that their types align with your business logic.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webservice/src/app/api/v1/openapi.ts(2 hunks)openapi.v1.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webservice/src/app/api/v1/openapi.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
🔇 Additional comments (2)
openapi.v1.json (2)
117-188: PATCH Operation for Updating a Deployment
The newpatchendpoint is clearly defined with a proper summary, operationId (updateDeployment), the required path parameter (deploymentIdwith UUID validation), and a request body that references theUpdateDeploymentschema. The responses for status codes 200, 404, and 500 are consistently structured. Please verify that the error response formats (with an"error"property) are aligned with your API’s overall error handling strategy.
2989-3005:❓ Verification inconclusive
Examination of the
UpdateDeploymentSchema Composition
TheUpdateDeploymentschema is defined using anallOfcomposition that references the existingDeploymentschema while also allowing additional properties. However, note that the baseDeploymentschema has several required fields (such as"name","slug","description","systemId", and"jobAgentConfig"). UsingallOfin this way will inherit those required constraints, which may conflict with the intent of having all update fields optional except for"id". Please verify whether this behavior is desired; if not, consider constructing a schema that explicitly marks all fields (other than"id") as optional to truly support partial updates.
I'll run a script to inspect the definition of the
Deploymentschema inopenapi.v1.jsonto verify whether it includes the required fields noted in the review comment (like"name","slug","description","systemId", and"jobAgentConfig"). This will help determine if the use ofallOfinUpdateDeploymentinadvertently inherits these required constraints.
🏁 Script executed:
#!/bin/bash # Extract the "Deployment" schema definition along with context from the OpenAPI file. rg -A 50 '"Deployment": {' openapi.v1.jsonLength of output: 138
Action Required: Verify Inherited Required Fields in UpdateDeployment Schema
The current “UpdateDeployment” schema uses anallOfcomposition to incorporate the base “Deployment” schema while adding flexibility by allowing additional properties. However, if the “Deployment” schema does indeed mark several fields (e.g.,"name","slug","description","systemId","jobAgentConfig") as required, then these constraints will be inherited by “UpdateDeployment”—which contradicts the intent of making update fields optional (besides"id").
- Please confirm that the “Deployment” schema (located at
#/components/schemas/Deploymentwithinopenapi.v1.json) includes these required fields. You might use a command like:jq '.components.schemas.Deployment.required' openapi.v1.json- If the required constraints from “Deployment” are not desired for update operations, consider refactoring the “UpdateDeployment” schema to explicitly mark fields (other than
"id") as optional.
apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts
Outdated
Show resolved
Hide resolved
| jobAgentConfig: { type: "object", additionalProperties: true }, | ||
| retryCount: { type: "integer" }, | ||
| timeout: { type: "integer", nullable: true }, | ||
| resourceFilter: { |
There was a problem hiding this comment.
I think we are refactoring this to resource selector?
There was a problem hiding this comment.
Yah, I think for now I omit it because of that.
openapi.v1.json
Outdated
| "type": "integer", | ||
| "nullable": true | ||
| }, | ||
| "resourceFilter": { |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
openapi.v1.json (1)
3132-3148: Consider refining the UpdateDeployment schema definition.The UpdateDeployment schema correctly extends the Deployment schema, making fields optional for updates. However, there are some redundancies and potential issues:
additionalProperties: trueappears twice (lines 3141 and 3147)- The schema requires the "id" field, but in a PATCH request, the ID is typically provided in the URL path, not the request body
Consider this improved definition:
"UpdateDeployment": { "type": "object", "description": "Schema for updating a deployment (all fields optional)", "allOf": [ { "$ref": "#/components/schemas/Deployment" }, { "type": "object", - "additionalProperties": true + "properties": { + "name": { "type": "string" }, + "slug": { "type": "string" }, + "description": { "type": "string" }, + "jobAgentId": { "type": "string", "format": "uuid", "nullable": true }, + "jobAgentConfig": { "type": "object", "additionalProperties": true }, + "retryCount": { "type": "integer" }, + "timeout": { "type": "integer", "nullable": true } + } } ], - "required": [ - "id" - ], "additionalProperties": true },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts(1 hunks)apps/webservice/src/app/api/v1/openapi.ts(2 hunks)openapi.v1.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webservice/src/app/api/v1/openapi.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict en...
**/*.{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/api/v1/deployments/[deploymentId]/route.ts
🧠 Learnings (1)
apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts (2)
Learnt from: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#382
File: apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts:82-88
Timestamp: 2025-03-16T19:41:44.129Z
Learning: In Next.js 15, dynamic route parameters (params) return Promises instead of direct values in async contexts. When accessing properties like `params.deploymentId` in an async function, use `(await params).deploymentId` to avoid the "params should be awaited before using its properties" error. This applies to API routes, page components, and other async contexts.
Learnt from: zacharyblasczyk
PR: ctrlplanedev/ctrlplane#382
File: apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts:82-88
Timestamp: 2025-03-16T19:41:44.129Z
Learning: In Next.js 15, params in dynamic routes must be awaited before accessing their properties in async contexts. For example, `params.deploymentId` should be accessed as `(await params).deploymentId` in async functions.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
🔇 Additional comments (8)
apps/webservice/src/app/api/v1/deployments/[deploymentId]/route.ts (6)
82-87: Good implementation of async parameter handling for Next.js 15.Your implementation correctly handles the dynamic route parameters as Promises in Next.js 15, as required by the framework. The async function and await on params is the correct approach.
91-91: Clean parameter extraction using object destructuring.The destructuring of deploymentId from the awaited params is clean and follows Next.js 15 best practices.
95-103: Proper request validation with detailed error response.The implementation correctly validates the request body against a schema and returns a detailed error response when validation fails. This is a security best practice that helps prevent invalid data from affecting the database.
105-115: Good existence check before update operation.Checking if the deployment exists before attempting to update it prevents potential errors and provides a clear 404 response to clients when the resource is not found.
117-123: Direct database update without field filtering.The implementation directly sets the entire request body to the database update without filtering fields, which could potentially allow updating fields that shouldn't be updatable (e.g., sensitive or system fields).
Add field filtering to ensure only allowed fields can be updated:
- .update(SCHEMA.deployment) - .set(body) + .update(SCHEMA.deployment) + .set({ + name: body.name, + description: body.description, + // Only include fields that should be updatable + })
125-131: Comprehensive error handling with logging.The error handling implementation catches exceptions, logs the error details, and returns an appropriate 500 response without exposing sensitive error information to clients.
openapi.v1.json (2)
117-188: Well-structured PATCH endpoint definition.The PATCH operation is properly defined with all required components - summary, operationId, parameters, request body, and responses. The response codes (200, 404, 500) align with the implementation in the route handler.
3114-3121: Proper schema update for new optional fields.The additions to the Deployment schema for retryCount and timeout are well-defined with appropriate types. Making timeout nullable is a good choice for optional fields.
Summary by CodeRabbit