feat: implement delete for resource by identifier#767
feat: implement delete for resource by identifier#767jsbroks merged 3 commits intoctrlplanedev:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a DELETE endpoint to remove a resource by identifier, updates OpenAPI spec and typings, implements a route handler that resolves the resource, emits a ResourceDeleted event, returns 204 on success, and introduces OpenAPI helpers Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as API Server
participant Service as Resource Service
participant Events as Event Bus
Client->>API: DELETE /v1/workspaces/{workspaceId}/resources/identifier/{identifier}
API->>Service: resolveResource(workspaceId, identifier)
alt resource found
Service-->>API: resource
API->>Service: deleteResource(resource.id)
Service-->>API: deletion confirmation
API->>Events: emit ResourceDeleted
Events-->>API: ack
API-->>Client: 204 No Content
else not found
Service-->>API: not found
API-->>Client: 404 ErrorResponse
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/api/openapi/openapi.json`:
- Around line 5650-5682: Update the OpenAPI operation with operationId
"deleteResourceByIdentifier" to include documented error responses that match
runtime behavior: add 401 (Unauthorized) when auth fails, 404 (Not Found) when
the resource/ workspaceId or identifier is missing, and 500 (Internal Server
Error) for unexpected server errors; reference your existing error schema (e.g.,
ErrorResponse or ApiError) in each response's content/application/json schema
and keep descriptions concise; ensure the operation still includes the existing
204 response and retains path parameters "workspaceId" and "identifier".
In `@apps/api/src/routes/v1/workspaces/resources.ts`:
- Around line 116-144: In updateVariablesForResource, the handler currently
sends a 204 with no content which violates the OpenAPI contract; change the
final response to return HTTP 202 with a JSON body containing the updated
variables as described by the spec (use the request body `body` as the updated
variables payload and include any expected fields such as the resource id from
`resourceResponse.data.id` if the spec requires it), i.e. replace the
res.status(204).end() with a 202 JSON response that returns the updated
variables object after the sendGoEvent call.
- Around line 8-38: The CEL decoding can throw URIError causing a 500; wrap the
decode logic in a try/catch inside listResources: when cel is a string attempt
the decodeURIComponent(...) and if it throws, throw a new ApiError with a clear
validation message (e.g., "Invalid cel query") and status 400; keep the decoded
value in the existing decodedCel variable and leave the rest of the request flow
unchanged so malformed CEL returns 400 instead of bubbling as a 500.
* adds the ability to delete a resource by identifier over the api fix: remove commented code fix: update openapi to match responses * update openapi to match not found response fix: add 500 error handling * adds 500 error handling for deleteResrouceByIdentifier endpoint
5114098 to
0e793fd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/api/src/routes/v1/workspaces/resources.ts`:
- Around line 106-111: The GET call using getClientFor(workspaceId).GET is
passing limit and offset at the wrong level so they are ignored; move the query
object under params (i.e. params.query) so the SDK receives query params. Update
the call that builds the request for
"/v1/workspaces/{workspaceId}/resources/{resourceIdentifier}/variables" (the
code referencing workspaceId, resourceIdentifier, limit, offset) to nest the
query ({ limit, offset }) inside params as params.query instead of as a sibling.
- Around line 153-182: The handler getReleaseTargetForResourceInDeployment is
destructuring req.params.resourceIdentifier but the Express route registers the
param as :identifier, so resourceIdentifier will be undefined; fix by reading
req.params.identifier (or rename the route param to resourceIdentifier) and use
that value (e.g., const resourceIdentifier = req.params.identifier) before
encoding and calling getClientFor, and apply the same parameter-name alignment
to the other identifier-based handlers mentioned (the ones around the other
release-target/deployment routes) so route params and handler params match
consistently.
Summary by CodeRabbit
New Features
Style