feat(artist): add CRUD endpoints for artists#51
Conversation
Implement a complete CRUD API for managing artists in the catalog. Changes: - Add GET /v2/artist/:id endpoint for retrieving artist details - Add POST /v2/artist endpoint for creating new artists (auth required) - Add PATCH /v2/artist/:id endpoint for updating artist metadata (auth required) - Add DELETE /v2/artist/:id endpoint for soft-deleting artists (auth required) - Add Zod schemas for request/response validation in packages/core - Add E2E tests covering all endpoints with 15 test cases
|
Warning Rate limit exceeded
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 55 minutes and 23 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR implements complete CRUD functionality for artists by adding four REST endpoints ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/handlers/catalog/artist/create.ts`:
- Around line 1-33: The file is not formatted to the project's code style; run
the repository formatter on the new handler to match CI expectations and commit
the result. Specifically, format this file containing artistCreateHandler (which
uses authMiddleware and calls artistService.create) with the project's
configured formatter (e.g., run the configured Prettier/format script or your
editor's format command), re-run lint/tests, and stage the formatted changes so
the updated file passes the Test and Lint check.
In `@apps/backend/tests/artist.test.ts`:
- Line 1: The file's import line ("import { afterAll, beforeAll, describe,
expect, test } from \"bun:test\";") is not formatted per the project's
configured formatter; run the repository's formatter (e.g., the project's format
script or configured tool) on this test file, apply the resulting changes, and
commit them so the file matches the formatter output.
- Around line 16-20: The test teardown in afterAll currently removes sessions
and users but not artists, causing leaked artist rows; update the afterAll block
to also delete artists by adding an awaited prisma.artist.deleteMany() call
(e.g., call await prisma.artist.deleteMany(); alongside await
prisma.session.deleteMany() and await prisma.user.deleteMany()) before awaiting
prisma.$disconnect() so all created artist rows are cleaned up after the suite.
In `@packages/core/src/modules/catalog/artist/dto.ts`:
- Around line 24-26: ArtistResponseSchema currently only omits deletedAt but
still exposes userId (and possibly the user relation) which leaks account
association; update ArtistResponseSchema (and ensure ArtistDetailsResponseSchema
if it aliases it) to omit userId and, if ArtistSchema exposes a user relation,
omit user as well by changing the .omit(...) call to include userId and user so
public GET /v2/artist/:id responses never include those fields.
In `@packages/core/src/modules/catalog/artist/repository.ts`:
- Around line 45-62: The update() and softDelete() repository methods currently
use id-only where clauses (in methods update and softDelete), which permits a
race where a concurrent soft-delete slips in; change their Prisma update where
filters to include active-record predicate deletedAt: null (e.g., where: { id,
deletedAt: null }) so the write only succeeds on non-deleted rows, or
alternatively refactor to a single atomic repository operation that both checks
deletedAt is null and performs the update/soft-delete in one call (preserve
usage of update(id: ArtistId, ...) and softDelete(id: ArtistId, ...) signatures
and the client variable tx ?? this.prisma).
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 290f7798-44dc-4b32-adc7-bdf26cde8d7d
📒 Files selected for processing (15)
apps/backend/src/handlers/catalog/artist/create.tsapps/backend/src/handlers/catalog/artist/delete.tsapps/backend/src/handlers/catalog/artist/get.tsapps/backend/src/handlers/catalog/artist/index.tsapps/backend/src/handlers/catalog/artist/update.tsapps/backend/src/handlers/index.tsapps/backend/src/index.tsapps/backend/tests/artist.test.tspackages/core/src/modules/catalog/artist/container.tspackages/core/src/modules/catalog/artist/dto.tspackages/core/src/modules/catalog/artist/index.tspackages/core/src/modules/catalog/artist/repository.interface.tspackages/core/src/modules/catalog/artist/repository.tspackages/core/src/modules/catalog/artist/service.tspackages/core/src/modules/catalog/index.ts
| afterAll(async () => { | ||
| await prisma.session.deleteMany(); | ||
| await prisma.user.deleteMany(); | ||
| await prisma.$disconnect(); | ||
| }); |
There was a problem hiding this comment.
Clean up artists in afterAll as well.
This suite creates artist rows but only removes them in beforeAll, so later test suites sharing the same database can observe leaked state.
Proposed cleanup
afterAll(async () => {
await prisma.session.deleteMany();
+ await prisma.artist.deleteMany();
await prisma.user.deleteMany();
await prisma.$disconnect();
});📝 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.
| afterAll(async () => { | |
| await prisma.session.deleteMany(); | |
| await prisma.user.deleteMany(); | |
| await prisma.$disconnect(); | |
| }); | |
| afterAll(async () => { | |
| await prisma.session.deleteMany(); | |
| await prisma.artist.deleteMany(); | |
| await prisma.user.deleteMany(); | |
| await prisma.$disconnect(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/tests/artist.test.ts` around lines 16 - 20, The test teardown in
afterAll currently removes sessions and users but not artists, causing leaked
artist rows; update the afterAll block to also delete artists by adding an
awaited prisma.artist.deleteMany() call (e.g., call await
prisma.artist.deleteMany(); alongside await prisma.session.deleteMany() and
await prisma.user.deleteMany()) before awaiting prisma.$disconnect() so all
created artist rows are cleaned up after the suite.
| export const ArtistResponseSchema = ArtistSchema.omit({ deletedAt: true }); | ||
|
|
||
| export const ArtistDetailsResponseSchema = ArtistResponseSchema; |
There was a problem hiding this comment.
Do not expose userId in public artist responses.
ArtistResponseSchema only strips deletedAt, but the Artist model also includes userId. Since GET /v2/artist/:id is public, this can leak the account/user association for an artist unless the service manually removes it everywhere.
🛡️ Proposed response schema tightening
-export const ArtistResponseSchema = ArtistSchema.omit({ deletedAt: true });
+export const ArtistResponseSchema = ArtistSchema.omit({
+ deletedAt: true,
+ userId: true,
+});If ArtistSchema also exposes the user relation, omit that here as well.
📝 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 ArtistResponseSchema = ArtistSchema.omit({ deletedAt: true }); | |
| export const ArtistDetailsResponseSchema = ArtistResponseSchema; | |
| export const ArtistResponseSchema = ArtistSchema.omit({ | |
| deletedAt: true, | |
| userId: true, | |
| }); | |
| export const ArtistDetailsResponseSchema = ArtistResponseSchema; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/modules/catalog/artist/dto.ts` around lines 24 - 26,
ArtistResponseSchema currently only omits deletedAt but still exposes userId
(and possibly the user relation) which leaks account association; update
ArtistResponseSchema (and ensure ArtistDetailsResponseSchema if it aliases it)
to omit userId and, if ArtistSchema exposes a user relation, omit user as well
by changing the .omit(...) call to include userId and user so public GET
/v2/artist/:id responses never include those fields.
| async update(id: ArtistId, input: UpdateArtistRequestDto, tx?: TxClient) { | ||
| const client = tx ?? this.prisma; | ||
|
|
||
| return transformPrismaResult( | ||
| await client.artist.update({ | ||
| where: { id }, | ||
| data: input, | ||
| omit: { deletedAt: true }, | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| async softDelete(id: ArtistId, tx?: TxClient) { | ||
| const client = tx ?? this.prisma; | ||
| await client.artist.update({ | ||
| where: { id }, | ||
| data: { deletedAt: new Date() }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether artist write operations are guarded by deletedAt/null checks.
# Expected: current output shows update writes using id-only where clauses.
rg -n -C4 'client\.artist\.update\(|where:\s*\{\s*id\s*\}|deletedAt' packages/core/src/modules/catalog/artist/repository.ts packages/core/src/modules/catalog/artist/service.tsRepository: project-cvsa/cvsa
Length of output: 3984
🏁 Script executed:
cat packages/core/src/modules/catalog/artist/service.tsRepository: project-cvsa/cvsa
Length of output: 1664
Add active-record predicates to update and softDelete writes.
The update() and softDelete() methods use id-only where clauses, creating a race condition. The service layer checks existence via getById() (which includes deletedAt: null), then calls the repository write unconditionally. A concurrent delete between these steps causes the original request to succeed against an already soft-deleted record. Add deletedAt: null to both where clauses, or combine the existence check and write into a single atomic repository operation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/modules/catalog/artist/repository.ts` around lines 45 - 62,
The update() and softDelete() repository methods currently use id-only where
clauses (in methods update and softDelete), which permits a race where a
concurrent soft-delete slips in; change their Prisma update where filters to
include active-record predicate deletedAt: null (e.g., where: { id, deletedAt:
null }) so the write only succeeds on non-deleted rows, or alternatively
refactor to a single atomic repository operation that both checks deletedAt is
null and performs the update/soft-delete in one call (preserve usage of
update(id: ArtistId, ...) and softDelete(id: ArtistId, ...) signatures and the
client variable tx ?? this.prisma).
1 participant
Changes
Implement a complete CRUD API for managing artists in the catalog.
Related