refactor: Introduce deployment version endpoints in api v1 #442
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces multiple new OpenAPI specification files and route handlers for managing deployment version channels, deployment versions, environments, and jobs. New endpoints using POST, PATCH, and DELETE methods have been defined or updated, incorporating middleware for authentication, authorization, and body parsing. Enhancements include additional properties in request schemas (e.g., Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant M as Middleware (auth, parsing)
participant H as Deployment Channel Handler
participant DB as Database
C->>M: Send POST /v1/deployment-version-channels
M->>H: Forward validated request
H->>DB: Insert new channel or update conflict record
DB-->>H: Return result (success/error)
H-->>C: Respond with JSON (200, 409, 500)
sequenceDiagram
participant C as Client
participant M as Middleware (auth, parsing)
participant H as Deployment Version PATCH Handler
participant DB as Database
C->>M: Send PATCH /v1/deployment-versions/{deploymentVersionId}
M->>H: Forward validated payload
H->>DB: Update deployment version & metadata
DB-->>H: Return operation outcome
H-->>C: Respond with updated version JSON or error
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
✨ 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
|
adityachoudhari26
changed the title
adityachoudhari26
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (15)
apps/webservice/src/app/api/v1/deployment-version-channels/route.ts (1)
28-45: Refine Error Handling Strategy.Currently, the entire error object is returned in the 500 response. This can risk exposing sensitive stack traces or internal details. Consider returning a sanitized error message or logging the detailed error separately.
.catch((error) => { - return NextResponse.json({ error }, { status: 500 }); + console.error("Deployment version channel insert error:", error); + return NextResponse.json( + { error: "Internal server error" }, + { status: 500 } + ); });apps/webservice/src/app/api/v1/jobs/[jobId]/route.ts (1)
104-108: Preserving Existing Metadata (Optional).At line 106, replacing
metadatawith an empty object discards potential metadata fromrow.deployment_version. If any existing metadata is desired, consider merging or preserving it. Otherwise, this is acceptable if no prior metadata is expected.- ? { ...row.deployment_version, metadata: {} } + ? { ...row.deployment_version, metadata: row.deployment_version.metadata ?? {} }apps/webservice/src/app/api/v1/deployments/[deploymentId]/deployment-version-channels/name/[name]/route.ts (1)
10-45: Check Affected Row Count on Deletion.After performing the delete operation, consider checking if any row was actually deleted. You might return a 404 if no matching record was found, improving client feedback.
try { const result = await ctx.db .delete(schema.deploymentVersionChannel) .where( and( eq(schema.deploymentVersionChannel.deploymentId, params.deploymentId), eq(schema.deploymentVersionChannel.name, params.name), ), ); + if (result.rowCount === 0) { + return NextResponse.json({ error: "Not found" }, { status: 404 }); + } return NextResponse.json( { message: "Deployment version channel deleted" }, { status: 200 }, ); } catch {apps/webservice/src/app/api/v1/deployment-versions/openapi.ts (1)
56-69: Consider adding required fields to 409 response schemaThe error response schema for the 409 conflict status doesn't specify required fields, unlike other error responses in the codebase. For consistency, consider adding the required fields as shown below:
properties: { error: { type: "string" }, id: { type: "string" }, }, + required: ["error", "id"],apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/openapi.ts (2)
52-61: Add error response definitionsThe current specification only defines a success (200) response. Consider adding error response definitions for common scenarios like 404 (not found), 401 (unauthorized), 403 (forbidden), and 500 (server error) to match the pattern used in other API specifications.
responses: { "200": { description: "OK", content: { "application/json": { schema: { $ref: "#/components/schemas/DeploymentVersion" }, }, }, }, + "404": { + description: "Deployment version not found", + content: { + "application/json": { + schema: { + type: "object", + properties: { error: { type: "string" } }, + required: ["error"], + }, + }, + }, + }, + "500": { + description: "Failed to update deployment version", + content: { + "application/json": { + schema: { + type: "object", + properties: { error: { type: "string" } }, + required: ["error"], + }, + }, + }, + }, },
5-65: Consider adding security configurationSimilar to the previous file, this OpenAPI specification doesn't include a security configuration. If authentication is required for updating deployment versions, consider adding security:
export const openapi: Swagger.SwaggerV3 = { // existing code... paths: { "/v1/deployment-versions/{deploymentVersionId}": { patch: { // existing code... + security: [{ bearerAuth: [] }], }, }, }, + components: { + securitySchemes: { bearerAuth: { type: "http", scheme: "bearer" } }, + }, };apps/webservice/src/app/api/v1/deployments/[deploymentId]/deployment-version-channels/name/[name]/openapi.ts (1)
3-81: Consider adding security configurationThe OpenAPI specification doesn't include security requirements. If authentication is needed for deleting deployment version channels, consider adding a security scheme:
export const openapi: Swagger.SwaggerV3 = { // existing code... paths: { "/v1/deployments/{deploymentId}/deployment-version-channels/name/{name}": { delete: { // existing code... + security: [{ bearerAuth: [] }], }, }, }, + components: { + securitySchemes: { bearerAuth: { type: "http", scheme: "bearer" } }, + }, };Otherwise, this is a well-structured OpenAPI spec with comprehensive response definitions for success and error cases.
apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/route.ts (3)
22-24: Consider validating metadata structure further.While this schema allows any string value in metadata, you might want to add constraints like maximum length or stricter formatting if there's a risk of receiving overly large or malformed data.
51-70: Be mindful of metadata key removals.This logic updates or creates new metadata keys but does not remove keys that were deleted from the request. If you need full synchronization of metadata, consider removing old keys not present in the request.
71-90: Refactor nested .then calls for clarity.Chaining multiple .then calls introduces nesting and reduces readability. Converting these calls to async/await can simplify error handling and improve maintainability.
- createReleaseJobTriggers(ctx.db, "version_updated") - .causedById(user.id) - .filter(isPassingChannelSelectorPolicy) - .versions([deploymentVersionId]) - .then(createJobApprovals) - .insert() - .then((releaseJobTriggers) => { - dispatchReleaseJobTriggers(ctx.db) - .releaseTriggers(releaseJobTriggers) - .filter(isPassingAllPolicies) - .then(cancelOldReleaseJobTriggersOnJobDispatch) - .dispatch(); - }) - .then(() => logger.info(...)); + const releaseJobTriggers = await createReleaseJobTriggers(ctx.db, "version_updated") + .causedById(user.id) + .filter(isPassingChannelSelectorPolicy) + .versions([deploymentVersionId]) + .then(createJobApprovals) + .insert(); + + await dispatchReleaseJobTriggers(ctx.db) + .releaseTriggers(releaseJobTriggers) + .filter(isPassingAllPolicies) + .then(cancelOldReleaseJobTriggersOnJobDispatch) + .dispatch(); + + logger.info(...);apps/webservice/src/app/api/v1/deployment-versions/route.ts (2)
31-36: Unify schema validation with PATCH.Consider centralizing shared fields (e.g., metadata) in a common schema or utility function to reduce duplicated logic between POST and PATCH routes.
112-141: Convert .then chains to async/await.Rewriting these chained calls using async/await reduces complexity and makes error handling more straightforward.
openapi.v1.json (3)
63-221: Endpoint "/v1/deployment-version-channels" Definition.
This new endpoint for creating a deployment version channel is comprehensively defined—with clear requirements on the request body (including required properties such asdeploymentId,name, andversionSelector), detailed response schemas for both success and various error cases, and correct application of bearer authentication. Consider adding example values or further descriptions (e.g., forversionSelector) to enhance the clarity of its intended usage.
3472-3474: Integration of "deploymentVersion" in JobWithTrigger Schema.
The newdeploymentVersionproperty has been added and correctly references the newly definedDeploymentVersionschema. For improved documentation, consider adding a brief description to explain its purpose within the context of a job trigger.
3710-3753: New "DeploymentVersion" Schema Definition.
TheDeploymentVersionschema is well-structured and includes the appropriate required properties (id,name,tag,config,jobAgentConfig,deploymentId,createdAt, andmetadata). To further enhance the specification, adding descriptive text for fields liketagandmetadatacan help clarify their expected values and usage.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/webservice/src/app/api/v1/deployment-version-channels/openapi.ts(1 hunks)apps/webservice/src/app/api/v1/deployment-version-channels/route.ts(1 hunks)apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/openapi.ts(1 hunks)apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/route.ts(1 hunks)apps/webservice/src/app/api/v1/deployment-versions/openapi.ts(1 hunks)apps/webservice/src/app/api/v1/deployment-versions/route.ts(1 hunks)apps/webservice/src/app/api/v1/deployments/[deploymentId]/deployment-version-channels/name/[name]/openapi.ts(1 hunks)apps/webservice/src/app/api/v1/deployments/[deploymentId]/deployment-version-channels/name/[name]/route.ts(1 hunks)apps/webservice/src/app/api/v1/environments/openapi.ts(1 hunks)apps/webservice/src/app/api/v1/environments/route.ts(2 hunks)apps/webservice/src/app/api/v1/jobs/[jobId]/openapi.ts(1 hunks)apps/webservice/src/app/api/v1/jobs/[jobId]/route.ts(3 hunks)apps/webservice/src/app/api/v1/openapi.ts(1 hunks)openapi.v1.json(5 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/api/v1/jobs/[jobId]/openapi.tsapps/webservice/src/app/api/v1/environments/openapi.tsapps/webservice/src/app/api/v1/jobs/[jobId]/route.tsapps/webservice/src/app/api/v1/environments/route.tsapps/webservice/src/app/api/v1/deployments/[deploymentId]/deployment-version-channels/name/[name]/route.tsapps/webservice/src/app/api/v1/deployment-version-channels/route.tsapps/webservice/src/app/api/v1/openapi.tsapps/webservice/src/app/api/v1/deployment-versions/route.tsapps/webservice/src/app/api/v1/deployments/[deploymentId]/deployment-version-channels/name/[name]/openapi.tsapps/webservice/src/app/api/v1/deployment-version-channels/openapi.tsapps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/openapi.tsapps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/route.tsapps/webservice/src/app/api/v1/deployment-versions/openapi.ts
🧬 Code Definitions (5)
apps/webservice/src/app/api/v1/jobs/[jobId]/route.ts (1)
packages/db/src/schema/deployment-version.ts (1)
deploymentVersion(106-131)
apps/webservice/src/app/api/v1/deployment-version-channels/openapi.ts (4)
apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/openapi.ts (1)
openapi(5-65)apps/webservice/src/app/api/v1/deployment-versions/openapi.ts (1)
openapi(5-74)apps/webservice/src/app/api/v1/deployments/[deploymentId]/deployment-version-channels/name/[name]/openapi.ts (1)
openapi(3-81)apps/webservice/src/app/api/v1/openapi.ts (1)
openapi(3-336)
apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/openapi.ts (2)
apps/webservice/src/app/api/v1/deployment-versions/openapi.ts (1)
openapi(5-74)apps/webservice/src/app/api/v1/openapi.ts (1)
openapi(3-336)
apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/route.ts (1)
apps/webservice/src/app/api/v1/jobs/[jobId]/route.ts (1)
PATCH(163-174)
apps/webservice/src/app/api/v1/deployment-versions/openapi.ts (3)
apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/openapi.ts (1)
openapi(5-65)apps/webservice/src/app/api/v1/deployment-version-channels/openapi.ts (1)
openapi(3-116)apps/webservice/src/app/api/v1/openapi.ts (1)
openapi(3-336)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (linux/amd64)
- GitHub Check: Typecheck
- GitHub Check: Lint
🔇 Additional comments (21)
apps/webservice/src/app/api/v1/environments/openapi.ts (1)
50-55: Properly extends request body schema with new property.The addition of the
deploymentVersionChannelsproperty to the environment creation request body is well-structured and consistent with the existing schema design pattern. This new property will allow clients to specify deployment version channels when creating environments.apps/webservice/src/app/api/v1/jobs/[jobId]/openapi.ts (1)
18-20: Good addition of deploymentVersion reference.The addition of the
deploymentVersionproperty to theJobWithTriggerschema properly references the newDeploymentVersionschema. This enhances the job definition by providing access to detailed deployment version information.apps/webservice/src/app/api/v1/openapi.ts (1)
122-143: Well-structured DeploymentVersion schema.The new
DeploymentVersionschema is properly defined with appropriate types and formats for each property. It follows the same pattern as other schema definitions in this file, maintaining consistency in the API design.apps/webservice/src/app/api/v1/environments/route.ts (2)
19-19: Correct schema extension for deploymentVersionChannels.Adding
deploymentVersionChannelsto the body schema as an array of strings matches the OpenAPI specification and properly extends the request validation.
40-43: Good implementation of combined channel handling.The code correctly combines both
releaseChannelsanddeploymentVersionChannelsusing the spread operator, allowing the query to consider both types of channels. This is an elegant solution that maintains backward compatibility while adding new functionality.apps/webservice/src/app/api/v1/deployment-version-channels/route.ts (2)
1-12: Consistent Setup and Imports.All imports and declarations appear valid and logically organized. The use of typed imports and clear separation of responsibilities looks good.
25-27: Clear Separation ofversionSelector.Extracting
versionSelectorbefore passing it to the database insertion call improves clarity. This approach helps keep the main insert flow concise.apps/webservice/src/app/api/v1/jobs/[jobId]/route.ts (3)
115-115: Good Structuring of the Job Execution Object.Adding
deploymentVersionto thejeobject centralizes relevant details for the job. This improves readability and discoverability of the version data.
121-122: Conditional Fetch for Approval Details.The short-circuit check on
je.deploymentVersion?.id && policyIdis a neat approach, ensuring the DB call occurs only when necessary.
142-143: Renaming Tagged Field toversion.Renaming the
tagproperty toversionfor thereleaseobject is reasonable if consumers expect a property namedversion. Ensure downstream code is updated accordingly.apps/webservice/src/app/api/v1/deployments/[deploymentId]/deployment-version-channels/name/[name]/route.ts (1)
1-2: Imports and Setup Look Good.Imports for database operations, authentication, and authorization are standard and well-structured.
apps/webservice/src/app/api/v1/deployment-version-channels/openapi.ts (1)
1-116: Well-structured OpenAPI specification with complete documentationThe OpenAPI specification for deployment version channels is comprehensive, with all required components:
- Properly defined request body schema with required fields
- Thorough response definitions for success and error cases
- Appropriate security scheme using bearer authentication
- Consistent formatting and structure
This follows best practices for API documentation and aligns well with other API specs in the codebase.
apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/route.ts (3)
26-39: Middleware chaining is well-structured.Chaining
authn,parseBody, andauthzin a linear flow enhances maintainability and readability while ensuring proper checks at each stage.
92-98: General error handling is consistent.Catching and logging exceptions before returning a 500 response is a robust approach. This helps maintain clarity for debugging and user feedback.
44-49:❓ Verification inconclusive
Verify partial update behavior.
Using
.set(body)might overwrite unspecified fields with undefined or null if they aren't provided, depending on howSCHEMA.updateDeploymentVersionis structured. Please confirm this aligns with the intended partial update logic.
🏁 Script executed:
#!/bin/bash # Searching for usage of updateDeploymentVersion schema to confirm partial or full update handling ast-grep --pattern $'updateDeploymentVersion($_)' || rg "updateDeploymentVersion"Length of output: 50
Partial Update Behavior: Manual Verification Required
The executed script did not return any indications regarding the handling of unspecified fields. Please manually verify that the use of
.set(body)in this update:
- Ensures that fields not provided in
bodyare left unchanged rather than being overwritten toundefinedornull.- Aligns with the intended partial update logic specified by the schema (i.e., the behavior expected from
updateDeploymentVersion).Review the schema configuration and the underlying implementation of how partial updates are handled to confirm that this behavior is correct.
apps/webservice/src/app/api/v1/deployment-versions/route.ts (5)
38-47: Middleware usage looks solid.Authentication, body parsing, and permission checks are consistently applied, which helps maintain a clean, modular API structure.
53-59: Validation for version name is appropriate.Returning an HTTP 400 if
nameortagis missing ensures proper handling of invalid inputs early.
72-90: Upsert logic is well-implemented.Merging or creating deployment versions on conflict streamlines usage. The use of
buildConflictUpdateColumnsis clear and follows best practices for upserts.
91-111: Consistent metadata upsert.Similar to the PATCH route, metadata is updated or inserted, but not removed. This is acceptable if partial metadata is desired. Otherwise, consider a cleanup approach.
147-159: Zod error handling is nicely handled.Differentiating validation errors (400) from other exceptions (500) makes API responses more transparent for clients.
openapi.v1.json (1)
4068-4071: Updated Security Scheme to "bearerAuth".
The security scheme has been updated to use HTTP bearer authentication, which aligns well with the new endpoints and modern API security practices. Ensure that the corresponding backend authentication logic is updated accordingly to validate bearer tokens.
| export const openapi: Swagger.SwaggerV3 = { | ||
| openapi: "3.0.0", | ||
| info: { | ||
| title: "Ctrlplane API", | ||
| version: "1.0.0", | ||
| }, | ||
| paths: { | ||
| "/v1/deployment-versions": { | ||
| post: { | ||
| summary: "Upserts a deployment version", | ||
| operationId: "upsertDeploymentVersion", | ||
| requestBody: { | ||
| required: true, | ||
| content: { | ||
| "application/json": { | ||
| schema: { | ||
| type: "object", | ||
| properties: { | ||
| tag: { type: "string" }, | ||
| deploymentId: { type: "string" }, | ||
| createdAt: { type: "string", format: "date-time" }, | ||
| name: { type: "string" }, | ||
| config: { type: "object", additionalProperties: true }, | ||
| jobAgentConfig: { | ||
| type: "object", | ||
| additionalProperties: true, | ||
| }, | ||
| status: { | ||
| type: "string", | ||
| enum: Object.values(DeploymentVersionStatus), | ||
| }, | ||
| message: { type: "string" }, | ||
| metadata: { | ||
| type: "object", | ||
| additionalProperties: { type: "string" }, | ||
| }, | ||
| }, | ||
| required: ["tag", "deploymentId"], | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| responses: { | ||
| "200": { | ||
| description: "OK", | ||
| content: { | ||
| "application/json": { | ||
| schema: { $ref: "#/components/schemas/DeploymentVersion" }, | ||
| }, | ||
| }, | ||
| }, | ||
| "409": { | ||
| description: "Deployment version already exists", | ||
| content: { | ||
| "application/json": { | ||
| schema: { | ||
| type: "object", | ||
| properties: { | ||
| error: { type: "string" }, | ||
| id: { type: "string" }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider adding security configuration
Unlike the deployment-version-channels OpenAPI specification, this file doesn't include a security configuration. If authentication is required for this endpoint, consider adding a security scheme and security requirement:
export const openapi: Swagger.SwaggerV3 = {
// existing code...
paths: {
"/v1/deployment-versions": {
post: {
// existing code...
+ security: [{ bearerAuth: [] }],
},
},
},
+ components: {
+ securitySchemes: { bearerAuth: { type: "http", scheme: "bearer" } },
+ },
};🏁 Script executed:
#!/bin/bash
# Check if other API endpoints use security configurations
grep -r "security: \[\{ bearerAuth: \[\] \}\]" apps/webservice/src/app/api/Length of output: 107
I'm going to re-check whether other API endpoints in our repository include a security configuration. Please run the following script to search for any occurrences of "security:" and "bearerAuth" in the API directory:
Based on the output of these commands, there isn’t immediate evidence of a shared security configuration like that in the commented diff. However, if authentication is required for this endpoint, please consider adding a security configuration similar to the example diff below. If the omission is intentional (for example, if authentication is handled elsewhere), please double-check for consistency.
Action Required: Include Security Configuration if Authentication Is Intended
It appears the OpenAPI spec for the deployment-versions endpoint (in apps/webservice/src/app/api/v1/deployment-versions/openapi.ts) doesn’t include any security configuration. If this endpoint should be secured, consider updating it like so:
export const openapi: Swagger.SwaggerV3 = {
openapi: "3.0.0",
info: {
title: "Ctrlplane API",
version: "1.0.0",
},
paths: {
"/v1/deployment-versions": {
post: {
summary: "Upserts a deployment version",
operationId: "upsertDeploymentVersion",
requestBody: { ... },
responses: { ... },
+ security: [{ bearerAuth: [] }],
},
},
},
+ components: {
+ securitySchemes: { bearerAuth: { type: "http", scheme: "bearer" } },
+ },
};If authentication is meant to be applied here, please add the configuration. Otherwise, if the lack of security is deliberate, confirm that this aligns with our overall API security strategy.
🏁 Script executed:
#!/bin/bash
echo "Searching for 'security:' keyword in API endpoints..."
rg -F "security:" apps/webservice/src/app/api/
echo "Searching for 'bearerAuth' usage in API endpoints..."
rg -F "bearerAuth" apps/webservice/src/app/api/Length of output: 1263
Security Configuration Consistency — Add Missing Security Schema
The endpoint at apps/webservice/src/app/api/v1/deployment-versions/openapi.ts currently does not include a security configuration, unlike similar endpoints. Our repository shows that endpoints in the following files have the security configuration already set up:
apps/webservice/src/app/api/v1/release-channels/openapi.tsapps/webservice/src/app/api/v1/relationship/resource-to-resource/openapi.tsapps/webservice/src/app/api/v1/deployment-version-channels/openapi.ts
If this endpoint is meant to be authenticated, please add a matching security configuration. For example:
export const openapi: Swagger.SwaggerV3 = {
openapi: "3.0.0",
info: {
title: "Ctrlplane API",
version: "1.0.0",
},
paths: {
"/v1/deployment-versions": {
post: {
summary: "Upserts a deployment version",
operationId: "upsertDeploymentVersion",
requestBody: {
required: true,
content: {
"application/json": {
schema: {
// schema details...
},
},
},
},
responses: {
"200": {
description: "OK",
content: {
"application/json": {
schema: { $ref: "#/components/schemas/DeploymentVersion" },
},
},
},
"409": {
description: "Deployment version already exists",
content: {
"application/json": {
schema: {
type: "object",
properties: {
error: { type: "string" },
id: { type: "string" },
},
},
},
},
},
},
+ security: [{ bearerAuth: [] }],
},
},
},
+ components: {
+ securitySchemes: { bearerAuth: { type: "http", scheme: "bearer" } },
+ },
};If—including authentication for this endpoint—is intentional per our API design, then please add the above configuration for consistency. Otherwise, ensure that the absence of the security configuration is clearly documented and aligned with our overall security strategy.
📝 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.
| export const openapi: Swagger.SwaggerV3 = { | |
| openapi: "3.0.0", | |
| info: { | |
| title: "Ctrlplane API", | |
| version: "1.0.0", | |
| }, | |
| paths: { | |
| "/v1/deployment-versions": { | |
| post: { | |
| summary: "Upserts a deployment version", | |
| operationId: "upsertDeploymentVersion", | |
| requestBody: { | |
| required: true, | |
| content: { | |
| "application/json": { | |
| schema: { | |
| type: "object", | |
| properties: { | |
| tag: { type: "string" }, | |
| deploymentId: { type: "string" }, | |
| createdAt: { type: "string", format: "date-time" }, | |
| name: { type: "string" }, | |
| config: { type: "object", additionalProperties: true }, | |
| jobAgentConfig: { | |
| type: "object", | |
| additionalProperties: true, | |
| }, | |
| status: { | |
| type: "string", | |
| enum: Object.values(DeploymentVersionStatus), | |
| }, | |
| message: { type: "string" }, | |
| metadata: { | |
| type: "object", | |
| additionalProperties: { type: "string" }, | |
| }, | |
| }, | |
| required: ["tag", "deploymentId"], | |
| }, | |
| }, | |
| }, | |
| }, | |
| responses: { | |
| "200": { | |
| description: "OK", | |
| content: { | |
| "application/json": { | |
| schema: { $ref: "#/components/schemas/DeploymentVersion" }, | |
| }, | |
| }, | |
| }, | |
| "409": { | |
| description: "Deployment version already exists", | |
| content: { | |
| "application/json": { | |
| schema: { | |
| type: "object", | |
| properties: { | |
| error: { type: "string" }, | |
| id: { type: "string" }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }; | |
| export const openapi: Swagger.SwaggerV3 = { | |
| openapi: "3.0.0", | |
| info: { | |
| title: "Ctrlplane API", | |
| version: "1.0.0", | |
| }, | |
| paths: { | |
| "/v1/deployment-versions": { | |
| post: { | |
| summary: "Upserts a deployment version", | |
| operationId: "upsertDeploymentVersion", | |
| requestBody: { | |
| required: true, | |
| content: { | |
| "application/json": { | |
| schema: { | |
| type: "object", | |
| properties: { | |
| tag: { type: "string" }, | |
| deploymentId: { type: "string" }, | |
| createdAt: { type: "string", format: "date-time" }, | |
| name: { type: "string" }, | |
| config: { type: "object", additionalProperties: true }, | |
| jobAgentConfig: { | |
| type: "object", | |
| additionalProperties: true, | |
| }, | |
| status: { | |
| type: "string", | |
| enum: Object.values(DeploymentVersionStatus), | |
| }, | |
| message: { type: "string" }, | |
| metadata: { | |
| type: "object", | |
| additionalProperties: { type: "string" }, | |
| }, | |
| }, | |
| required: ["tag", "deploymentId"], | |
| }, | |
| }, | |
| }, | |
| }, | |
| responses: { | |
| "200": { | |
| description: "OK", | |
| content: { | |
| "application/json": { | |
| schema: { $ref: "#/components/schemas/DeploymentVersion" }, | |
| }, | |
| }, | |
| }, | |
| "409": { | |
| description: "Deployment version already exists", | |
| content: { | |
| "application/json": { | |
| schema: { | |
| type: "object", | |
| properties: { | |
| error: { type: "string" }, | |
| id: { type: "string" }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| security: [{ bearerAuth: [] }], | |
| }, | |
| }, | |
| }, | |
| components: { | |
| securitySchemes: { bearerAuth: { type: "http", scheme: "bearer" } }, | |
| }, | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/webservice/src/app/api/v1/releases/[releaseId]/route.ts (1)
47-47: Consider documenting the fallback behaviorThe fallback from tag to version is implemented correctly, but it might be helpful to add a comment explaining this fallback behavior for future developers.
- const tag = body.tag ?? body.version; + // Use tag if available, otherwise fall back to version field + const tag = body.tag ?? body.version;packages/db/src/schema/deployment-version.ts (1)
267-275: Consider refactoring duplicate condition building logicThe
buildTagConditionfunction duplicates the logic frombuildVersionConditionsince both operate on the tag field. Consider refactoring to reduce duplication.-const buildTagCondition = (cond: TagCondition): SQL => { - if (cond.operator === ColumnOperator.Equals) - return eq(deploymentVersion.tag, cond.value); - if (cond.operator === ColumnOperator.StartsWith) - return ilike(deploymentVersion.tag, `${cond.value}%`); - if (cond.operator === ColumnOperator.EndsWith) - return ilike(deploymentVersion.tag, `%${cond.value}`); - return ilike(deploymentVersion.tag, `%${cond.value}%`); -}; +// Generic function to build conditions for string columns +const buildStringColumnCondition = ( + column: typeof deploymentVersion.tag, + operator: ColumnOperator, + value: string +): SQL => { + if (operator === ColumnOperator.Equals) + return eq(column, value); + if (operator === ColumnOperator.StartsWith) + return ilike(column, `${value}%`); + if (operator === ColumnOperator.EndsWith) + return ilike(column, `%${value}`); + return ilike(column, `%${value}%`); +}; + +const buildVersionCondition = (cond: VersionCondition): SQL => { + return buildStringColumnCondition(deploymentVersion.tag, cond.operator, cond.value); +}; + +const buildTagCondition = (cond: TagCondition): SQL => { + return buildStringColumnCondition(deploymentVersion.tag, cond.operator, cond.value); +};
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/webservice/src/app/api/v1/deployment-version-channels/route.ts(1 hunks)apps/webservice/src/app/api/v1/releases/[releaseId]/route.ts(2 hunks)packages/db/src/schema/deployment-version.ts(2 hunks)packages/validators/src/releases/conditions/comparison-condition.ts(3 hunks)packages/validators/src/releases/conditions/index.ts(1 hunks)packages/validators/src/releases/conditions/release-condition.ts(4 hunks)packages/validators/src/releases/conditions/tag-condition.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webservice/src/app/api/v1/deployment-version-channels/route.ts
🧰 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.
packages/validators/src/releases/conditions/index.tspackages/validators/src/releases/conditions/tag-condition.tspackages/validators/src/releases/conditions/comparison-condition.tspackages/validators/src/releases/conditions/release-condition.tsapps/webservice/src/app/api/v1/releases/[releaseId]/route.tspackages/db/src/schema/deployment-version.ts
🧬 Code Definitions (4)
packages/validators/src/releases/conditions/comparison-condition.ts (1)
packages/validators/src/releases/conditions/tag-condition.ts (2)
tagCondition(5-9)TagCondition(11-11)
packages/validators/src/releases/conditions/release-condition.ts (4)
packages/validators/src/releases/conditions/comparison-condition.ts (2)
ComparisonCondition(30-41)comparisonCondition(13-28)packages/validators/src/conditions/metadata-condition.ts (2)
MetadataCondition(56-56)metadataCondition(48-54)packages/validators/src/conditions/index.ts (2)
VersionCondition(42-42)versionCondition(36-40)packages/validators/src/releases/conditions/tag-condition.ts (2)
TagCondition(11-11)tagCondition(5-9)
apps/webservice/src/app/api/v1/releases/[releaseId]/route.ts (1)
packages/db/src/schema/release.ts (1)
release(42-55)
packages/db/src/schema/deployment-version.ts (2)
packages/validators/src/releases/conditions/tag-condition.ts (1)
TagCondition(11-11)packages/validators/src/releases/conditions/release-condition.ts (1)
DeploymentVersionCondition(15-20)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Typecheck
- GitHub Check: build (linux/amd64)
- GitHub Check: Lint
🔇 Additional comments (12)
packages/validators/src/releases/conditions/index.ts (1)
3-3: Looks good - introducing tag condition exportThe added export for tag-condition.js follows the same pattern as existing exports, appropriately integrating the new tag condition functionality.
packages/validators/src/releases/conditions/tag-condition.ts (1)
1-11: LGTM: Tag condition schema implementationThe implementation is clean and follows the project's patterns for defining Zod schemas and exporting types. The schema appropriately validates tag conditions with the required properties.
apps/webservice/src/app/api/v1/releases/[releaseId]/route.ts (2)
23-26: Appropriate addition of version field to schemaAdding the optional version field to the patch schema aligns with the PR objectives for the deployment version endpoints.
50-50: Looks good - spreading body and setting tagThe update to include the tag variable in the set operation ensures the correct value is used regardless of which field was provided in the request.
packages/db/src/schema/deployment-version.ts (2)
6-9: Appropriate import of TagCondition typeThe import statement is correctly updated to include the TagCondition type from the validators package.
284-285: Good implementation of tag condition handlingThe condition handling for tag type is correctly implemented in the buildCondition function.
packages/validators/src/releases/conditions/release-condition.ts (3)
9-9: Well-integrated: TagCondition imports and type additionsThe TagCondition type and schema have been properly imported and integrated into the existing DeploymentVersionCondition union type and deploymentVersionCondition Zod schema. These changes follow the established pattern for condition types in the system.
Also applies to: 13-13, 20-20, 27-27
45-45: Good addition: DeploymentVersionConditionType enum updatedThe Tag entry is correctly added to the DeploymentVersionConditionType enum, which is necessary for the condition type checking throughout the system.
101-104: Well-implemented: Type guard and validationThe isTagCondition type guard function follows the same pattern as other type guards in the file. The additional validation in isValidDeploymentVersionCondition correctly ensures that tag values have a length greater than zero, maintaining consistent validation across different condition types.
Also applies to: 114-114
packages/validators/src/releases/conditions/comparison-condition.ts (3)
8-8: Imported tag condition types correctly.The additions of imports for both the
TagConditiontype and thetagConditionZod schema from the tag-condition module align with the existing pattern used for other conditions.Also applies to: 11-11
24-24: Successfully added tag condition to the union of valid conditions.Adding
tagConditionto thez.unionarray properly integrates tag-based conditions into the validation logic, which extends the functionality to support tag filtering alongside existing condition types.
39-39: Properly updated the ComparisonCondition type definition.The type definition now correctly includes
TagConditionin the union type for the conditions array, maintaining type consistency with the Zod schema implementation above.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation