Api delete pre-check

[jsbroks]

Add pre-delete existence checks for deployments, environments, relationship rules, and systems to ensure items exist before sending delete events and to use actual entity data in event payloads.

---

 

---

Note

Low Risk
Low risk: adds pre-delete fetches and reuses fetched data in delete events; behavior change is limited to returning proper 404s and emitting richer payloads for existing resources.

Overview
Adds pre-delete fetch/validation for deployments, environments, relationship-rules, and systems delete endpoints so missing resources return a proper not-found error instead of emitting a delete event.

Updates delete event emission (sendGoEvent) to use the real fetched entity payload (e.g., deployment, environmentResponse.data, relationshipRuleResponse.data, systemResponse.data.system) rather than placeholder/empty fields.

^{Written by Cursor Bugbot for commit db6f9ad. This will update automatically on new commits. Configure here.}

Summary by CodeRabbit

Bug Fixes

• Deletion endpoints now validate resource existence before removal and return appropriate error responses when resources are not found
• Deletion event payloads now include actual resource data instead of placeholder information, ensuring accurate event records

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[cursor]

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
_{Learn more about Cursor Agents}

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR enhances deletion handlers across four resource types (deployments, environments, relationship-rules, and systems) by adding pre-fetch validation and populating deletion events with actual resource data instead of empty placeholder objects.

Changes

Cohort / File(s)	Summary
Deletion Handler Improvements apps/api/src/routes/v1/workspaces/deployments.ts, apps/api/src/routes/v1/workspaces/environments.ts, apps/api/src/routes/v1/workspaces/relationship-rules.ts, apps/api/src/routes/v1/workspaces/systems.ts	All deletion handlers now fetch the target resource via API client before emitting deletion events. Existence validation is performed with appropriate 404 error handling. Event payloads are updated to use the actual fetched resource data instead of empty or placeholder objects.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Handler as Delete Handler
    participant APIClient as API Client
    participant EventBus as Event Emitter
    participant DB as Database

    rect rgba(200, 150, 100, 0.5)
    Note over Client,DB: Before: Direct deletion without pre-fetch
    Client->>Handler: DELETE /resource/{id}
    Handler->>EventBus: Emit deleted event (empty data)
    Handler->>Client: 202/204 response
    end

    rect rgba(100, 150, 200, 0.5)
    Note over Client,DB: After: Pre-fetch before deletion
    Client->>Handler: DELETE /resource/{id}
    Handler->>APIClient: GET /resource/{id}
    APIClient->>DB: Query resource
    DB-->>APIClient: Return resource data
    APIClient-->>Handler: Return resource response
    alt Resource found
        Handler->>EventBus: Emit deleted event (real data)
        Handler->>Client: 202/204 response
    else Resource not found
        Handler->>Client: 404 ApiError
    end
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• ctrlplanedev/ctrlplane#255: Main PR refactoring delete handlers to pre-fetch resources and emit deletion events with real resource data, aligning with this PR's consistent pattern across multiple resource types.

Poem

🐰 Before deletion's final call,
We fetch the resource, one and all,
No empty shells in our events,
Just truthful data, making sense!
Real resources now we send, hop hop ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Api delete pre-check' clearly and concisely summarizes the main objective of the PR—adding pre-deletion existence checks to API delete handlers.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch cursor/api-delete-pre-check-6348

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

apps/api/src/routes/v1/workspaces/deployments.ts (2)

139-139: ⚠️ Potential issue | 🟡 Minor

Remove debug console.log statement.

This appears to be a leftover debug artifact that should not be committed.

🧹 Proposed fix

   const { body } = req;
 
-  console.log(body);
-
   const existingDeploymentResponse = await existingDeploymentById(

---

147-164: ⚠️ Potential issue | 🟠 Major

Missing resourceSelector validation on the create path.

When deployment == null, a DeploymentCreated event is emitted without validating the resourceSelector. However, postDeployment (line 117-118) validates the selector before creating. This inconsistency could allow invalid selectors through the upsert-create path.

🛡️ Proposed fix to add validation on the create path

   if (deployment == null) {
+    const isValid = await validResourceSelector(body.resourceSelector);
+    if (!isValid) throw new ApiError("Invalid resource selector", 400);
+
     await sendGoEvent({
       workspaceId,
       eventType: Event.DeploymentCreated,

🧹 Nitpick comments (1)

apps/api/src/routes/v1/workspaces/systems.ts (1)

75-85: Consider consistent error handling pattern.

The pre-fetch errors throw ApiError (propagated by asyncHandler), but sendGoEvent errors are caught and return a generic 500. Other delete handlers in this PR let errors propagate naturally. Consider removing the try/catch for consistency and to preserve error details.

♻️ Proposed refactor for consistent error handling

-  try {
-    await sendGoEvent({
-      workspaceId,
-      eventType: Event.SystemDeleted,
-      timestamp: Date.now(),
-      data: systemResponse.data.system,
-    });
-  } catch {
-    res.status(500).json({ message: "Failed to delete system" });
-    return;
-  }
+  await sendGoEvent({
+    workspaceId,
+    eventType: Event.SystemDeleted,
+    timestamp: Date.now(),
+    data: systemResponse.data.system,
+  });
 
   res.status(204).json({ message: "System deleted successfully" });