Skip to content

feat: add artist pin endpoint#424

Open
arpitgupta1214 wants to merge 4 commits intotestfrom
codex/artist-pin-endpoint-api
Open

feat: add artist pin endpoint#424
arpitgupta1214 wants to merge 4 commits intotestfrom
codex/artist-pin-endpoint-api

Conversation

@arpitgupta1214
Copy link
Copy Markdown
Collaborator

@arpitgupta1214 arpitgupta1214 commented Apr 10, 2026

Summary

  • add nested POST /api/artists/{id}/pin and DELETE /api/artists/{id}/pin
  • validate artist path params, auth, existence, and access before mutating pin state
  • upsert account_artist_ids so org-access artists can be pinned without an existing direct link row
  • reuse shared artist access validation in the delete validator to keep the flow DRY

Summary by CodeRabbit

  • New Features
    • Added artist pinning functionality, allowing authenticated users to pin and unpin artists via new API endpoints that accept POST requests to pin and DELETE requests to unpin.

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Apr 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
recoup-api Ready Ready Preview Apr 10, 2026 2:30pm

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 10, 2026

Warning

Rate limit exceeded

@arpitgupta1214 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 6 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 6 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9ed0d7e4-ec92-42a0-9432-74edb544e5dd

📥 Commits

Reviewing files that changed from the base of the PR and between 29dcca4 and 7b4bd9d.

⛔ Files ignored due to path filters (1)
  • lib/artists/__tests__/pinArtistHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (1)
  • lib/artists/pinArtistHandler.ts
📝 Walkthrough

Walkthrough

Introduces a new feature for pinning artists to an account via REST API endpoints (POST/DELETE at /api/artists/[id]/pin). The implementation includes a validation utility, handler function, database operations, and a new parameter to existing database functions to support the pin state.

Changes

Cohort / File(s) Summary
API Route & Handler
app/api/artists/[id]/pin/route.ts, lib/artists/pinArtistHandler.ts
New REST endpoint with OPTIONS, POST, and DELETE handlers; shared handler implements validation, database operation, CORS support, and error handling with appropriate HTTP status codes.
Validation Utilities
lib/artists/validateArtistAccessRequest.ts, lib/artists/validateDeleteArtistRequest.ts
New shared validation function that checks route params, authentication, artist existence, and access permissions; refactored existing deletion validator to reuse new validation logic and simplified error handling.
Business Logic
lib/artists/pinArtist.ts
New thin service layer that orchestrates the pin/unpin operation by delegating to database upsert function.
Database Layer
lib/supabase/account_artist_ids/upsertAccountArtistPin.ts, lib/supabase/account_artist_ids/updateAccountArtistPin.ts, lib/supabase/account_artist_ids/insertAccountArtistId.ts
Added upsert logic for artist pins (select-then-update/insert pattern); new update function for pinned state; extended insert function to optionally set initial pinned value.

Sequence Diagram

sequenceDiagram
    actor Client
    participant API as POST/DELETE /api/artists/[id]/pin
    participant Handler as pinArtistHandler
    participant Validator as validateArtistAccessRequest
    participant Service as pinArtist
    participant DB as Supabase (upsert)
    
    Client->>API: POST/DELETE request
    API->>Handler: pinArtistHandler(request, params, isPinned)
    Handler->>Validator: validateArtistAccessRequest(request, id)
    Validator->>Validator: validate route params & auth
    Validator->>DB: selectAccounts(artistId) [check exists]
    Validator->>DB: checkAccountArtistAccess() [authorize]
    Validator-->>Handler: ValidatedArtistAccessRequest or NextResponse
    
    alt Validation Failed
        Handler-->>API: NextResponse (error)
        API-->>Client: 400/403/404 JSON error
    else Validation Passed
        Handler->>Service: pinArtist(artistId, pinned, requesterAccountId)
        Service->>DB: upsertAccountArtistPin()
        DB->>DB: selectAccountArtistId()
        alt Row Exists
            DB->>DB: updateAccountArtistPin()
        else Row Not Found
            DB->>DB: insertAccountArtistId()
        end
        DB-->>Service: Promise<void>
        Service-->>Handler: resolve/reject
        Handler-->>API: NextResponse.json({ success: true, ... })
        API-->>Client: 200 JSON response
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • sweetmantech

Poem

📌 A pin feature blooms with grace,
Validation guards each access place,
From route to database, clean and tight,
Reusable patterns shine so bright! ✨

🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Solid & Clean Code ⚠️ Warning Import statements use relative paths instead of project alias convention; violates documented repository guidelines for @/lib/supabase imports. Update import statements in updateAccountArtistPin.ts and insertAccountArtistId.ts from '../serverClient' to '@/lib/supabase/serverClient' to align with repository conventions.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/artist-pin-endpoint-api

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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (5)
lib/supabase/account_artist_ids/upsertAccountArtistPin.ts (1)

1-1: Use the canonical Supabase client import path for lib/supabase/*.

Please import from @/lib/supabase/serverClient here to keep Supabase access modules consistent with the repository pattern.

As per coding guidelines: "lib/supabase/**/*.ts: Supabase database functions should import from @/lib/supabase/serverClient..."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/supabase/account_artist_ids/upsertAccountArtistPin.ts` at line 1, Replace
the non-canonical import in upsertAccountArtistPin.ts: change the module import
that currently reads import supabase from "../serverClient" to use the
repository canonical path import from "@/lib/supabase/serverClient"; update the
import statement at the top of the file where supabase is referenced (the module
containing the upsertAccountArtistPin function) so all Supabase access in
lib/supabase/* follows the project's canonical serverClient path.
lib/artists/validatePinArtistBody.ts (2)

14-18: Prefer schema-inferred types over a manually duplicated request interface.

Please export an inferred type from pinArtistBodySchema and compose ValidatedPinArtistRequest from it, so schema and TypeScript stay in sync.

Based on learnings: "Applies to lib/**/validate*.ts : Create validate functions in validate<EndpointName>Body.ts or validate<EndpointName>Query.ts files that export both the schema and inferred TypeScript type."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/artists/validatePinArtistBody.ts` around lines 14 - 18, Replace the
manually duplicated ValidatedPinArtistRequest with a schema-inferred type:
export the pinArtistBodySchema from this module (or import it if defined
elsewhere) and derive a TypeScript type using z.infer<typeof
pinArtistBodySchema>, then compose ValidatedPinArtistRequest from that inferred
type (and add requesterAccountId if it’s not part of the schema). Update the
export so ValidatedPinArtistRequest is defined from the inferred schema type to
keep schema and TS types in sync (referencing pinArtistBodySchema and
ValidatedPinArtistRequest in validatePinArtistBody.ts).

26-88: Split this validator into smaller helpers.

This function is doing multiple concerns (body parse/validation, auth, existence, authorization, response shaping) and exceeds the 20-line guideline. Extracting reusable error-response and access-check helpers will improve maintainability.

As per coding guidelines: "**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php}: Flag functions longer than 20 lines..." and "**/*.{ts,tsx}: Extract shared logic into reusable utilities following DRY principle."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/artists/validatePinArtistBody.ts` around lines 26 - 88, The
validatePinArtistBody function is doing multiple responsibilities; split it into
small helpers: extract the body parsing/validation into a
parseAndValidatePinArtistBody(request) helper that returns typed data or
throws/returns a standard error payload, move the repeated NextResponse JSON
creation into an errorResponse(payload, status) helper that attaches
getCorsHeaders(), and move existence and permission checks into
ensureArtistExists(artistId) and ensureAccountHasAccess(requesterAccountId,
artistId) helpers that return booleans or standardized error responses; update
validatePinArtistBody to call parseAndValidatePinArtistBody,
validateAuthContext, ensureArtistExists, and ensureAccountHasAccess and only
assemble the final validated object ({ artistId, pinned, requesterAccountId })
or return errorResponse to preserve current behavior and headers.
lib/artists/pinArtist.ts (1)

2-2: Decouple domain logic from request-validator types.

pinArtist importing ValidatedPinArtistRequest from validatePinArtistBody ties business logic to transport validation. Consider moving the params type to a neutral lib/artists/*types* module (or local type) to keep boundaries clean.

As per coding guidelines: "lib/**/*.ts: Apply Single Responsibility Principle (SRP): one exported function per file; each file should do one thing well."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/artists/pinArtist.ts` at line 2, The pinArtist file currently imports the
transport-level type ValidatedPinArtistRequest from validatePinArtistBody which
couples domain logic to request validation; extract that type into a neutral
type container (e.g., a new lib/artists/types.ts or a local PinArtistParams
type) and update pinArtist to import/use that new type instead of
validatePinArtistBody's type, then change validatePinArtistBody to re-export or
import the shared type; ensure the exported function pinArtist signature uses
the new neutral type and remove the direct import of ValidatedPinArtistRequest
from validatePinArtistBody.
lib/artists/pinArtistHandler.ts (1)

14-48: Refactor handler into smaller units for readability and reuse.

This handler currently exceeds the 20-line guideline and mixes validation branching, success response construction, and error response construction. Extracting success/error response helpers would reduce duplication and simplify the flow.

As per coding guidelines: "**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php}: Flag functions longer than 20 lines..." and "app/api/**/*.ts: DRY: Extract common validation and error handling logic."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/artists/pinArtistHandler.ts` around lines 14 - 48, The pinArtistHandler
function should be split into smaller helpers: create a validate-and-extract
step using validatePinArtistBody (already called), and move response
construction into two reusable helpers (e.g., buildPinSuccessResponse and
buildErrorResponse or respondSuccess/respondError) that encapsulate
NextResponse.json with getCorsHeaders; update pinArtistHandler to call
validatePinArtistBody, await pinArtist(validated), then return the success
helper (including artistId and pinned) and catch errors to return the error
helper (using error instanceof Error ? error.message : "Internal server error");
keep function logic identical but reduced to orchestration only and reuse the
helpers across other handlers.
🤖 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/pinArtistHandler.ts`:
- Around line 37-41: In pinArtistHandler, stop returning raw internal errors in
the NextResponse.json body; replace the dynamic error.message with a generic
client-facing message like "Internal server error" and log the full error
details server-side (e.g., via console.error or your logger) using the existing
error variable so operators can debug without leaking DB/provider internals;
ensure the response shape (status / error) remains consistent and only exposes
non-sensitive text.

In `@lib/artists/validatePinArtistBody.ts`:
- Around line 32-39: The current response always returns the first Zod issue
path under the key missing_fields even for type/format errors; update the
NextResponse.json payload in validatePinArtistBody (where result.error.issues
and firstError are used) to return a more accurate field such as error_fields
(or conditional missing_fields only when firstError.code === "invalid_type" or
=== "missing") and include the issue code/type and path (e.g., { status:
"error", error_fields: firstError.path, error_code: firstError.code, error:
firstError.message }) so clients can distinguish missing-field errors from other
validation failures.

In `@lib/supabase/account_artist_ids/upsertAccountArtistPin.ts`:
- Around line 25-32: The upsert in upsertAccountArtistPin.ts uses onConflict:
"account_id,artist_id" against the account_artist_ids table but there is no
unique constraint/index on (account_id, artist_id), so add a unique constraint
on those two columns in your Supabase migration (e.g., create unique index or
constraint for account_artist_ids(account_id, artist_id) with a clear name) so
the upsert can use onConflict, or alternatively change the upsert logic in
upsertAccountArtistPin to perform an insert-then-update flow without relying on
onConflict.

---

Nitpick comments:
In `@lib/artists/pinArtist.ts`:
- Line 2: The pinArtist file currently imports the transport-level type
ValidatedPinArtistRequest from validatePinArtistBody which couples domain logic
to request validation; extract that type into a neutral type container (e.g., a
new lib/artists/types.ts or a local PinArtistParams type) and update pinArtist
to import/use that new type instead of validatePinArtistBody's type, then change
validatePinArtistBody to re-export or import the shared type; ensure the
exported function pinArtist signature uses the new neutral type and remove the
direct import of ValidatedPinArtistRequest from validatePinArtistBody.

In `@lib/artists/pinArtistHandler.ts`:
- Around line 14-48: The pinArtistHandler function should be split into smaller
helpers: create a validate-and-extract step using validatePinArtistBody (already
called), and move response construction into two reusable helpers (e.g.,
buildPinSuccessResponse and buildErrorResponse or respondSuccess/respondError)
that encapsulate NextResponse.json with getCorsHeaders; update pinArtistHandler
to call validatePinArtistBody, await pinArtist(validated), then return the
success helper (including artistId and pinned) and catch errors to return the
error helper (using error instanceof Error ? error.message : "Internal server
error"); keep function logic identical but reduced to orchestration only and
reuse the helpers across other handlers.

In `@lib/artists/validatePinArtistBody.ts`:
- Around line 14-18: Replace the manually duplicated ValidatedPinArtistRequest
with a schema-inferred type: export the pinArtistBodySchema from this module (or
import it if defined elsewhere) and derive a TypeScript type using
z.infer<typeof pinArtistBodySchema>, then compose ValidatedPinArtistRequest from
that inferred type (and add requesterAccountId if it’s not part of the schema).
Update the export so ValidatedPinArtistRequest is defined from the inferred
schema type to keep schema and TS types in sync (referencing pinArtistBodySchema
and ValidatedPinArtistRequest in validatePinArtistBody.ts).
- Around line 26-88: The validatePinArtistBody function is doing multiple
responsibilities; split it into small helpers: extract the body
parsing/validation into a parseAndValidatePinArtistBody(request) helper that
returns typed data or throws/returns a standard error payload, move the repeated
NextResponse JSON creation into an errorResponse(payload, status) helper that
attaches getCorsHeaders(), and move existence and permission checks into
ensureArtistExists(artistId) and ensureAccountHasAccess(requesterAccountId,
artistId) helpers that return booleans or standardized error responses; update
validatePinArtistBody to call parseAndValidatePinArtistBody,
validateAuthContext, ensureArtistExists, and ensureAccountHasAccess and only
assemble the final validated object ({ artistId, pinned, requesterAccountId })
or return errorResponse to preserve current behavior and headers.

In `@lib/supabase/account_artist_ids/upsertAccountArtistPin.ts`:
- Line 1: Replace the non-canonical import in upsertAccountArtistPin.ts: change
the module import that currently reads import supabase from "../serverClient" to
use the repository canonical path import from "@/lib/supabase/serverClient";
update the import statement at the top of the file where supabase is referenced
(the module containing the upsertAccountArtistPin function) so all Supabase
access in lib/supabase/* follows the project's canonical serverClient path.
🪄 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: 795dbe69-c486-4a31-b72a-876bb2ec587e

📥 Commits

Reviewing files that changed from the base of the PR and between fc97310 and 133c984.

⛔ Files ignored due to path filters (3)
  • lib/artists/__tests__/pinArtistHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/artists/__tests__/validatePinArtistBody.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/account_artist_ids/__tests__/upsertAccountArtistPin.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (5)
  • app/api/artists/pin/route.ts
  • lib/artists/pinArtist.ts
  • lib/artists/pinArtistHandler.ts
  • lib/artists/validatePinArtistBody.ts
  • lib/supabase/account_artist_ids/upsertAccountArtistPin.ts

Comment on lines +32 to +39
const firstError = result.error.issues[0];

return NextResponse.json(
{
status: "error",
missing_fields: firstError.path,
error: firstError.message,
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

missing_fields is inaccurate for non-missing validation errors.

This always reports the first issue path as missing_fields, even for type/format failures (e.g., invalid UUID). That can mislead API clients.

Suggested payload adjustment
-    const firstError = result.error.issues[0];
+    const firstError = result.error.issues[0];
+    const validationIssues = result.error.issues.map(issue => ({
+      path: issue.path,
+      message: issue.message,
+    }));

     return NextResponse.json(
       {
         status: "error",
-        missing_fields: firstError.path,
         error: firstError.message,
+        validation_issues: validationIssues,
       },
📝 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.

Suggested change
const firstError = result.error.issues[0];
return NextResponse.json(
{
status: "error",
missing_fields: firstError.path,
error: firstError.message,
},
const firstError = result.error.issues[0];
const validationIssues = result.error.issues.map(issue => ({
path: issue.path,
message: issue.message,
}));
return NextResponse.json(
{
status: "error",
error: firstError.message,
validation_issues: validationIssues,
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/artists/validatePinArtistBody.ts` around lines 32 - 39, The current
response always returns the first Zod issue path under the key missing_fields
even for type/format errors; update the NextResponse.json payload in
validatePinArtistBody (where result.error.issues and firstError are used) to
return a more accurate field such as error_fields (or conditional missing_fields
only when firstError.code === "invalid_type" or === "missing") and include the
issue code/type and path (e.g., { status: "error", error_fields:
firstError.path, error_code: firstError.code, error: firstError.message }) so
clients can distinguish missing-field errors from other validation failures.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 8 files

Confidence score: 3/5

  • There is a concrete regression risk in lib/artists/pinArtistHandler.ts: calling validatePinArtistBody outside the try means async auth/DB errors can bypass normal error handling and produce inconsistent failure responses.
  • The concern in lib/artists/validatePinArtistBody.ts is mostly maintainability (mixed responsibilities and naming mismatch), which is less likely to break behavior immediately but increases future change risk.
  • Given the medium severity/high confidence findings, this is mergeable with caution after tightening error-path handling.
  • Pay close attention to lib/artists/pinArtistHandler.ts and lib/artists/validatePinArtistBody.ts - ensure thrown validation/auth/query errors are consistently caught and responsibility boundaries are clear.
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="lib/artists/validatePinArtistBody.ts">

<violation number="1" location="lib/artists/validatePinArtistBody.ts:55">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**

This function handles four concerns (body parsing, auth, entity existence, access control) but is named `validatePinArtistBody`, violating single-responsibility design. The `selectAccounts` existence check and `checkAccountArtistAccess` authorization belong in `pinArtistHandler`, consistent with how `validateCreateArtistBody` and `validateCreateWorkspaceBody` limit themselves to Zod parsing + auth.</violation>
</file>

<file name="lib/artists/pinArtistHandler.ts">

<violation number="1" location="lib/artists/pinArtistHandler.ts:15">
P2: Move the `validatePinArtistBody` call inside the `try` block. It performs multiple async operations (auth check, DB queries) that can throw, and an unhandled exception here would bypass the error handler, returning no CORS headers and no structured error body.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant Client
    participant Route as API Route (/api/artists/pin)
    participant Handler as pinArtistHandler
    participant Val as validatePinArtistBody
    participant Auth as validateAuthContext
    participant DB as Supabase (PostgreSQL)

    Note over Client,DB: NEW: Artist Pinning Flow

    Client->>Route: POST /api/artists/pin { artistId, pinned }
    Route->>Handler: pinArtistHandler(request)
    Handler->>Val: validatePinArtistBody(request)
    
    Val->>Val: NEW: Zod Schema Validation
    alt Invalid Schema
        Val-->>Handler: 400 Bad Request
    else Valid Schema
        Val->>Auth: validateAuthContext(request)
        alt Auth Failed
            Auth-->>Val: 401 Unauthorized
            Val-->>Handler: 401 Response
        else Auth Success
            Auth-->>Val: requesterAccountId
            
            Val->>DB: selectAccounts(artistId)
            alt Artist Missing
                DB-->>Val: empty array
                Val-->>Handler: 404 Not Found
            else Artist Exists
                Val->>DB: checkAccountArtistAccess(requesterAccountId, artistId)
                alt No Access
                    DB-->>Val: false
                    Val-->>Handler: 403 Forbidden
                else Has Access
                    DB-->>Val: true
                    Val-->>Handler: ValidatedParams
                end
            end
        end
    end

    alt Validation Success
        Handler->>DB: NEW: upsertAccountArtistPin(accountId, artistId, pinned)
        Note right of DB: NEW: Performs upsert on 'account_artist_ids'<br/>ON CONFLICT (account_id, artist_id)
        DB-->>Handler: Success
        Handler-->>Route: 200 OK { success: true }
        Route-->>Client: 200 OK + CORS Headers
    else Validation Failed
        Handler-->>Route: Error Response
        Route-->>Client: Error Response + CORS Headers
    end
Loading

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 8 files (changes from recent commits).

Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 8 files (changes from recent commits).

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="lib/supabase/account_artist_ids/upsertAccountArtistPin.ts">

<violation number="1" location="lib/supabase/account_artist_ids/upsertAccountArtistPin.ts:27">
P1: This select-then-insert/update pattern introduces a TOCTOU race condition. Two concurrent pin requests for the same account/artist pair can both see no existing row and both attempt an INSERT, causing one to fail with a unique-constraint violation. The previous single `upsert` call with `onConflict` was atomic and immune to this. Consider restoring an atomic upsert or wrapping the new logic in a serializable transaction / handling the duplicate-key error in the INSERT path.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

artistId,
pinned,
}: UpsertAccountArtistPinParams): Promise<void> {
const existingRow = await selectAccountArtistId(accountId, artistId);
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: This select-then-insert/update pattern introduces a TOCTOU race condition. Two concurrent pin requests for the same account/artist pair can both see no existing row and both attempt an INSERT, causing one to fail with a unique-constraint violation. The previous single upsert call with onConflict was atomic and immune to this. Consider restoring an atomic upsert or wrapping the new logic in a serializable transaction / handling the duplicate-key error in the INSERT path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/supabase/account_artist_ids/upsertAccountArtistPin.ts, line 27:

<comment>This select-then-insert/update pattern introduces a TOCTOU race condition. Two concurrent pin requests for the same account/artist pair can both see no existing row and both attempt an INSERT, causing one to fail with a unique-constraint violation. The previous single `upsert` call with `onConflict` was atomic and immune to this. Consider restoring an atomic upsert or wrapping the new logic in a serializable transaction / handling the duplicate-key error in the INSERT path.</comment>

<file context>
@@ -22,16 +24,12 @@ export async function upsertAccountArtistPin({
-    },
-    { onConflict: "account_id,artist_id" },
-  );
+  const existingRow = await selectAccountArtistId(accountId, artistId);
 
-  if (error) {
</file context>
Fix with Cubic

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
lib/artists/validateDeleteArtistRequest.ts (1)

20-35: Consider using getCorsHeaders() directly for consistency.

The current approach copies headers from the original response (validated.headers), which works but introduces an implicit dependency on validateArtistAccessRequest's header implementation. For consistency with other error responses in the codebase and clearer intent:

♻️ Suggested refinement
+import { getCorsHeaders } from "@/lib/networking/getCorsHeaders";
+
 // ... in the function body:
     return NextResponse.json(
       {
         status: "error",
         error: "Unauthorized delete attempt",
       },
       {
         status: 403,
-        headers: validated.headers,
+        headers: getCorsHeaders(),
       },
     );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/artists/validateDeleteArtistRequest.ts` around lines 20 - 35, Replace
copying headers from the returned NextResponse (validated.headers) with a direct
call to getCorsHeaders() when returning the 403 JSON in
validateDeleteArtistRequest: detect the unauthorized case (validated instanceof
NextResponse and validated.status === 403) and return NextResponse.json(...)
with headers set to getCorsHeaders() instead of using validated.headers to keep
header behavior consistent with other error responses and remove the implicit
dependency on validateArtistAccessRequest's headers.
🤖 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/supabase/account_artist_ids/updateAccountArtistPin.ts`:
- Line 1: Update the import in updateAccountArtistPin.ts to use the project
alias instead of a relative path by importing serverClient from
"@/lib/supabase/serverClient" (replace the existing import of supabase from
"../serverClient"); also ensure the exported function in this file follows the
repository pattern—add JSDoc, explicit TypeScript return types, and proper error
handling around Supabase calls (use the serverClient instance) so the function
matches other lib/supabase/**/*.ts implementations.

---

Nitpick comments:
In `@lib/artists/validateDeleteArtistRequest.ts`:
- Around line 20-35: Replace copying headers from the returned NextResponse
(validated.headers) with a direct call to getCorsHeaders() when returning the
403 JSON in validateDeleteArtistRequest: detect the unauthorized case (validated
instanceof NextResponse and validated.status === 403) and return
NextResponse.json(...) with headers set to getCorsHeaders() instead of using
validated.headers to keep header behavior consistent with other error responses
and remove the implicit dependency on validateArtistAccessRequest's headers.
🪄 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: 7cecc88f-c9c9-4933-9ebf-e6d6f2df8b62

📥 Commits

Reviewing files that changed from the base of the PR and between 133c984 and 29dcca4.

⛔ Files ignored due to path filters (5)
  • lib/artists/__tests__/pinArtistHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/artists/__tests__/validateArtistAccessRequest.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/account_artist_ids/__tests__/insertAccountArtistId.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/account_artist_ids/__tests__/updateAccountArtistPin.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/supabase/account_artist_ids/__tests__/upsertAccountArtistPin.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (8)
  • app/api/artists/[id]/pin/route.ts
  • lib/artists/pinArtist.ts
  • lib/artists/pinArtistHandler.ts
  • lib/artists/validateArtistAccessRequest.ts
  • lib/artists/validateDeleteArtistRequest.ts
  • lib/supabase/account_artist_ids/insertAccountArtistId.ts
  • lib/supabase/account_artist_ids/updateAccountArtistPin.ts
  • lib/supabase/account_artist_ids/upsertAccountArtistPin.ts
✅ Files skipped from review due to trivial changes (2)
  • lib/artists/pinArtist.ts
  • lib/supabase/account_artist_ids/upsertAccountArtistPin.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/artists/pinArtistHandler.ts

@@ -0,0 +1,24 @@
import supabase from "../serverClient";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use the required Supabase client import path.

Line 1 should import serverClient via the project alias, not a relative path, to stay consistent with the repository convention.

Suggested 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/serverClient and follow the documented pattern with JSDoc comments, error handling, and return types”.

📝 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.

Suggested change
import supabase from "../serverClient";
import supabase from "@/lib/supabase/serverClient";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/supabase/account_artist_ids/updateAccountArtistPin.ts` at line 1, Update
the import in updateAccountArtistPin.ts to use the project alias instead of a
relative path by importing serverClient from "@/lib/supabase/serverClient"
(replace the existing import of supabase from "../serverClient"); also ensure
the exported function in this file follows the repository pattern—add JSDoc,
explicit TypeScript return types, and proper error handling around Supabase
calls (use the serverClient instance) so the function matches other
lib/supabase/**/*.ts implementations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant