Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA new DELETE endpoint for artist accounts is introduced with multi-layer request validation, authorization checks, and cascading deletion logic that removes artist-account associations before deleting the artist account if no remaining links exist. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Route as Route Handler<br/>(DELETE /api/artists/[id])
participant Validator as Request Validator
participant Business as Business Logic<br/>(deleteArtist)
participant DB as Supabase Database
Client->>Route: DELETE /api/artists/{id}
Route->>Validator: validateDeleteArtistRequest(request, id)
Validator->>Validator: validateAccountParams(id)
Validator->>Validator: validateAuthContext(request)
Validator->>DB: selectAccounts(artistId)
DB-->>Validator: artist record (or null)
Validator->>Validator: checkAccountArtistAccess(requesterAccountId, artistId)
Validator-->>Route: { artistId, requesterAccountId }
Route->>Business: deleteArtist({ artistId, requesterAccountId })
Business->>DB: deleteAccountArtistId(accountId, artistId)
DB-->>Business: deleted rows
Business->>DB: getAccountArtistIds({ artistIds: [artistId] })
DB-->>Business: remaining associations
alt No remaining associations
Business->>DB: deleteAccountById(artistId)
DB-->>Business: true
end
Business-->>Route: artistId
Route-->>Client: { success: true, artistId } (200)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 9 files
Confidence score: 3/5
- Potential data loss risk in
lib/artists/deleteArtist.ts: treating an emptygetAccountArtistIdsresult as “no remaining links” could hard-delete artists on transient DB errors - Style/consistency concerns exist but are likely non-blocking (route export pattern in
app/api/artists/[id]/route.ts, DELETE vs POST inlib/artists/deleteArtistHandler.ts, and oversized test inlib/artists/__tests__/deleteArtist.test.ts) - Score reflects a concrete user-impacting risk alongside several maintainability issues
- Pay close attention to
lib/artists/deleteArtist.ts- guard against deleting on query errors
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/api/artists/[id]/route.ts">
<violation number="1" location="app/api/artists/[id]/route.ts:10">
P1: Custom agent: **Module should export a single primary function whose name matches the filename**
This module exports multiple top-level functions (`OPTIONS` and `DELETE`) instead of a single primary export matching the filename `route`, violating the single-export module rule. Split the handlers into separate files or rename the primary export to match the filename.</violation>
</file>
<file name="lib/artists/deleteArtistHandler.ts">
<violation number="1" location="lib/artists/deleteArtistHandler.ts:7">
P2: Custom agent: **API Design Consistency and Maintainability**
Rule 1 requires POST for actions/mutations, but this new handler is explicitly for a DELETE mutation (`DELETE /api/artists/{id}`). Use POST for this mutation (or rename the endpoint/handler accordingly) to keep HTTP methods consistent with the API design rule.</violation>
</file>
<file name="lib/artists/__tests__/deleteArtist.test.ts">
<violation number="1" location="lib/artists/__tests__/deleteArtist.test.ts:1">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
This new test file is 126 lines long, exceeding the 100-line limit in the Clear Code Style rule. Split the tests into smaller focused files (e.g., access validation vs delete behavior) to stay under the limit.</violation>
</file>
<file name="lib/artists/deleteArtist.ts">
<violation number="1" location="lib/artists/deleteArtist.ts:60">
P2: `getAccountArtistIds` returns `[]` on query errors, but this code treats an empty result as “no remaining links” and deletes the artist account. A transient DB error would incorrectly hard-delete an artist. Consider surfacing errors (throw/return null) and aborting the delete when the lookup fails.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant API as Artist Route Handler
participant Auth as Auth Service
participant Logic as Artist Logic (deleteArtist)
participant DB as Supabase
Note over Client,DB: NEW: DELETE /api/artists/{id} Flow
Client->>API: DELETE /api/artists/{id}
API->>API: validateAccountParams(id)
API->>Auth: validateAuthContext(request)
alt Auth or Param Validation Fails
Auth-->>API: Auth Error (NextResponse)
API-->>Client: 401 Unauthorized / 400 Bad Request
else Auth Success
Auth-->>API: requesterAccountId
end
API->>Logic: NEW: deleteArtist(artistId, requesterAccountId)
Logic->>DB: selectAccounts(artistId)
alt Artist Not Found
DB-->>Logic: []
Logic-->>API: code: "not_found"
API-->>Client: 404 Not Found
end
Logic->>Logic: checkAccountArtistAccess(requesterId, artistId)
Logic->>DB: NEW: deleteAccountArtistId(requesterId, artistId)
alt No direct link found to delete
DB-->>Logic: []
Logic-->>API: code: "forbidden"
API-->>Client: 403 Unauthorized delete attempt
else Link Removed
DB-->>Logic: [deletedLinkRow]
end
Logic->>DB: getAccountArtistIds(artistId)
DB-->>Logic: remainingLinks[]
opt NEW: No links remain for artist
Logic->>DB: NEW: deleteAccountById(artistId)
Note right of DB: Cascade rules handle related data
DB-->>Logic: success
end
Logic-->>API: ok: true
API-->>Client: 200 OK { success: true, artistId }
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| * | ||
| * @returns A NextResponse with CORS headers. | ||
| */ | ||
| export async function OPTIONS() { |
There was a problem hiding this comment.
P1: Custom agent: Module should export a single primary function whose name matches the filename
This module exports multiple top-level functions (OPTIONS and DELETE) instead of a single primary export matching the filename route, violating the single-export module rule. Split the handlers into separate files or rename the primary export to match the filename.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/artists/[id]/route.ts, line 10:
<comment>This module exports multiple top-level functions (`OPTIONS` and `DELETE`) instead of a single primary export matching the filename `route`, violating the single-export module rule. Split the handlers into separate files or rename the primary export to match the filename.</comment>
<file context>
@@ -0,0 +1,30 @@
+ *
+ * @returns A NextResponse with CORS headers.
+ */
+export async function OPTIONS() {
+ return new NextResponse(null, {
+ status: 200,
</file context>
| import { validateDeleteArtistRequest } from "@/lib/artists/validateDeleteArtistRequest"; | ||
|
|
||
| /** | ||
| * Handler for DELETE /api/artists/{id}. |
There was a problem hiding this comment.
P2: Custom agent: API Design Consistency and Maintainability
Rule 1 requires POST for actions/mutations, but this new handler is explicitly for a DELETE mutation (DELETE /api/artists/{id}). Use POST for this mutation (or rename the endpoint/handler accordingly) to keep HTTP methods consistent with the API design rule.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/artists/deleteArtistHandler.ts, line 7:
<comment>Rule 1 requires POST for actions/mutations, but this new handler is explicitly for a DELETE mutation (`DELETE /api/artists/{id}`). Use POST for this mutation (or rename the endpoint/handler accordingly) to keep HTTP methods consistent with the API design rule.</comment>
<file context>
@@ -0,0 +1,79 @@
+import { validateDeleteArtistRequest } from "@/lib/artists/validateDeleteArtistRequest";
+
+/**
+ * Handler for DELETE /api/artists/{id}.
+ *
+ * Removes the authenticated account's direct link to an artist. If that link
</file context>
| artistIds: [artistId], | ||
| }); | ||
|
|
||
| if (remainingLinks.length === 0) { |
There was a problem hiding this comment.
P2: getAccountArtistIds returns [] on query errors, but this code treats an empty result as “no remaining links” and deletes the artist account. A transient DB error would incorrectly hard-delete an artist. Consider surfacing errors (throw/return null) and aborting the delete when the lookup fails.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/artists/deleteArtist.ts, line 60:
<comment>`getAccountArtistIds` returns `[]` on query errors, but this code treats an empty result as “no remaining links” and deletes the artist account. A transient DB error would incorrectly hard-delete an artist. Consider surfacing errors (throw/return null) and aborting the delete when the lookup fails.</comment>
<file context>
@@ -0,0 +1,68 @@
+ artistIds: [artistId],
+ });
+
+ if (remainingLinks.length === 0) {
+ await deleteAccountById(artistId);
+ }
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/supabase/accounts/deleteAccountById.ts (1)
1-1: Use the canonical Supabase client import path.Please switch the client import to
@/lib/supabase/serverClientfor consistency across Supabase helpers.Proposed change
-import supabase from "../serverClient"; +import supabase from "@/lib/supabase/serverClient";As per coding guidelines
lib/supabase/**/*.ts: “Supabase database functions should import from@/lib/supabase/serverClientand follow the documented pattern with JSDoc comments, error handling, and return types”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/supabase/accounts/deleteAccountById.ts` at line 1, The import in deleteAccountById.ts currently uses a relative path (import supabase from "../serverClient"); update it to import the canonical Supabase client from "@/lib/supabase/serverClient" and ensure the file follows the project Supabase helper pattern: include JSDoc for the exported function (e.g., deleteAccountById), proper error handling and typed return values consistent with other lib/supabase/**/*.ts helpers, and use the imported supabase instance from the new module name.lib/supabase/account_artist_ids/deleteAccountArtistId.ts (1)
1-1: Align server client import with Supabase conventions.Use
@/lib/supabase/serverClientinstead of a relative path to keep helper imports consistent.Proposed change
-import supabase from "../serverClient"; +import supabase from "@/lib/supabase/serverClient";As per coding guidelines
lib/supabase/**/*.ts: “Supabase database functions should import from@/lib/supabase/serverClientand follow the documented pattern with JSDoc comments, error handling, and return types”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/supabase/account_artist_ids/deleteAccountArtistId.ts` at line 1, Replace the relative import in deleteAccountArtistId.ts (currently importing supabase from "../serverClient") with the canonical alias import "@/lib/supabase/serverClient"; also ensure the file follows the documented Supabase helper pattern by adding/verifying JSDoc on the exported function (deleteAccountArtistId), consistent error handling around Supabase calls, and an explicit return type for the function so it matches other lib/supabase/*.ts utilities.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/artists/deleteArtist.ts`:
- Around line 31-37: The current read-then-delete using getAccountArtistIds(...)
and deleteAccountById(artistId) is racy; make the orphan-check and deletion
atomic by moving the logic into a single transactional DB operation or RPC.
Replace the two-step flow with one DB call (e.g., add a new function like
deleteAccountIfNoArtistLinks(accountId) or a transaction wrapper used by
deleteArtist) that rechecks the related links and performs DELETE only when none
exist (for example: a transaction or single SQL DELETE ... WHERE id = $1 AND NOT
EXISTS (SELECT 1 FROM account_artists WHERE artist_id = $1)). Ensure the new
function is used instead of calling getAccountArtistIds and deleteAccountById
separately to avoid concurrency races.
In `@lib/artists/validateDeleteArtistRequest.ts`:
- Around line 51-63: The current validation uses
checkAccountArtistAccess(requesterAccountId, artistId) which permits org-level
access; change it to enforce a direct account↔artist link by calling a
direct-link check (e.g., checkAccountArtistDirectLink(requesterAccountId,
artistId)) or extend checkAccountArtistAccess with a flag to requireDirect=true,
and if that direct-link check returns false return the same 403 JSON error
response (status:"error", error:"Unauthorized delete attempt") with CORS
headers; update any imports or helper usage in validateDeleteArtistRequest to
reference the direct-link helper name used.
---
Nitpick comments:
In `@lib/supabase/account_artist_ids/deleteAccountArtistId.ts`:
- Line 1: Replace the relative import in deleteAccountArtistId.ts (currently
importing supabase from "../serverClient") with the canonical alias import
"@/lib/supabase/serverClient"; also ensure the file follows the documented
Supabase helper pattern by adding/verifying JSDoc on the exported function
(deleteAccountArtistId), consistent error handling around Supabase calls, and an
explicit return type for the function so it matches other lib/supabase/*.ts
utilities.
In `@lib/supabase/accounts/deleteAccountById.ts`:
- Line 1: The import in deleteAccountById.ts currently uses a relative path
(import supabase from "../serverClient"); update it to import the canonical
Supabase client from "@/lib/supabase/serverClient" and ensure the file follows
the project Supabase helper pattern: include JSDoc for the exported function
(e.g., deleteAccountById), proper error handling and typed return values
consistent with other lib/supabase/**/*.ts helpers, and use the imported
supabase instance from the new module name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9e546dd6-23a3-45f9-aadd-012fc94d8539
⛔ Files ignored due to path filters (2)
lib/artists/__tests__/deleteArtistHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/artists/__tests__/validateDeleteArtistRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (6)
app/api/artists/[id]/route.tslib/artists/deleteArtist.tslib/artists/deleteArtistHandler.tslib/artists/validateDeleteArtistRequest.tslib/supabase/account_artist_ids/deleteAccountArtistId.tslib/supabase/accounts/deleteAccountById.ts
Comment on lines
+31
to
+37
| const remainingLinks = await getAccountArtistIds({ | ||
| artistIds: [artistId], | ||
| }); | ||
|
|
||
| if (remainingLinks.length === 0) { | ||
| await deleteAccountById(artistId); | ||
| } |
There was a problem hiding this comment.
Make orphan-check + account delete atomic.
Line 31 and Line 35-36 perform a read-then-delete sequence without transaction boundaries. Under concurrency, duplicate or stale decisions are possible. Move this into one atomic DB operation (transaction/RPC) so “delete if no remaining links” is evaluated and executed together.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/artists/deleteArtist.ts` around lines 31 - 37, The current
read-then-delete using getAccountArtistIds(...) and deleteAccountById(artistId)
is racy; make the orphan-check and deletion atomic by moving the logic into a
single transactional DB operation or RPC. Replace the two-step flow with one DB
call (e.g., add a new function like deleteAccountIfNoArtistLinks(accountId) or a
transaction wrapper used by deleteArtist) that rechecks the related links and
performs DELETE only when none exist (for example: a transaction or single SQL
DELETE ... WHERE id = $1 AND NOT EXISTS (SELECT 1 FROM account_artists WHERE
artist_id = $1)). Ensure the new function is used instead of calling
getAccountArtistIds and deleteAccountById separately to avoid concurrency races.
Comment on lines
+51
to
+63
| const hasAccess = await checkAccountArtistAccess(requesterAccountId, artistId); | ||
| if (!hasAccess) { | ||
| return NextResponse.json( | ||
| { | ||
| status: "error", | ||
| error: "Unauthorized delete attempt", | ||
| }, | ||
| { | ||
| status: 403, | ||
| headers: getCorsHeaders(), | ||
| }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Enforce direct-link ownership here, not broad artist access.
checkAccountArtistAccess allows organization-based access, but delete semantics require a removable direct link. This can pass validation and still fail in delete with a 500 path. Return 403 here when no direct account↔artist link exists.
Proposed change
-import { checkAccountArtistAccess } from "@/lib/artists/checkAccountArtistAccess";
+import { selectAccountArtistId } from "@/lib/supabase/account_artist_ids/selectAccountArtistId";
@@
- const hasAccess = await checkAccountArtistAccess(requesterAccountId, artistId);
- if (!hasAccess) {
+ const directLink = await selectAccountArtistId(requesterAccountId, artistId);
+ if (!directLink) {
return NextResponse.json(
{
status: "error",
error: "Unauthorized delete attempt",
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/artists/validateDeleteArtistRequest.ts` around lines 51 - 63, The current
validation uses checkAccountArtistAccess(requesterAccountId, artistId) which
permits org-level access; change it to enforce a direct account↔artist link by
calling a direct-link check (e.g.,
checkAccountArtistDirectLink(requesterAccountId, artistId)) or extend
checkAccountArtistAccess with a flag to requireDirect=true, and if that
direct-link check returns false return the same 403 JSON error response
(status:"error", error:"Unauthorized delete attempt") with CORS headers; update
any imports or helper usage in validateDeleteArtistRequest to reference the
direct-link helper name used.
sweetmantech
approved these changes
Apr 10, 2026
Contributor
Manual Testing ResultsTested the preview deployment: Endpoint:
|
| Test | Expected | Actual |
|---|---|---|
| OPTIONS preflight | DELETE allowed | ✅ 200 |
| No auth | 401 | ✅ 401 — "Exactly one of x-api-key or Authorization must be provided" |
| Invalid UUID format | 400 | ✅ 400 — "id must be a valid UUID" |
| Non-existent UUID with auth | 403 | ✅ 403 — "Unauthorized delete attempt" (good anti-enumeration) |
| Wrong ID type (artist record id) | 404 | ✅ 404 — "Artist not found" |
| Successful delete with account_id | 200 | ✅ 200 — {"success":true,"artistId":"..."} |
| Verify removal | Gone from list | ✅ Total dropped 46 → 45 |
Successfully deleted a throwaway test artist ('delete me') and confirmed it was removed from the artists list.
Note for the docs
The {id} path param is the account_id (artist's account UUID), not the artist record ID. Initially tried with the artist id field and got a 404. Worth clarifying in recoupable/docs#124 so consumers know which UUID to pass.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
sweetmantech
added a commit
that referenced
this pull request
Apr 10, 2026
feat: add artist delete endpoint (#421)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DELETE /api/artists/{id}with auth, access validation, and safe delete semanticsStack
Summary by cubic
Adds a DELETE /api/artists/{id} endpoint to unlink the requester from an artist and delete the artist only if no links remain. Centralizes validation and access checks for safe, predictable deletes with consistent CORS.
New Features
Refactors
Written for commit 2f45480. Summary will update on new commits.
Summary by CodeRabbit