feat: add drum layouts table and service #48
Conversation
Implements drum layouts feature with: - Database migration for drum_layouts table - Zod schema with layout_format containing drumCounts - DrumLayoutsRepo service layer - S3 handler functions for drum layout uploads - API endpoints: submit, complete, get, list, delete, download - TODO placeholder for file parsing/validation Closes #47 Co-authored-by: bitnimble <bitnimble@users.noreply.github.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Introduces “drum layouts” as a first-class asset type (DB-backed record + S3-backed zip), mirroring the existing maps upload/publish flow and adding API endpoints for listing, fetching, submitting, deleting, and downloading drum layouts.
Changes:
- Adds
drum_layoutstable and Zapatos typings for persistence. - Adds drum layout Zod schemas +
DrumLayoutsRepofor DB/S3 lifecycle management. - Adds S3 helper functions and new Next.js API routes for drum layout CRUD + upload flow.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| supabase/migrations/20260111004023_add_drum_layouts.sql | Creates drum_layouts table and enables RLS. |
| src/services/zapatos/schema.d.ts | Adds Zapatos table typings/exports for drum_layouts. |
| src/services/server_context.ts | Wires DrumLayoutsRepo into the server context. |
| src/services/maps/s3_handler.ts | Adds drum-layout-specific S3 keying + upload/delete/promote helpers. |
| src/services/drum_layouts/drum_layouts_repo.ts | Implements drum layout repo logic (find/get/create/delete/validate + error map). |
| src/services/db/id_gen.ts | Adds IdDomain.DRUM_LAYOUTS for ID generation. |
| src/schema/drum_layouts.ts | Adds Zod schemas for drum layouts + API request/response shapes. |
| src/app/api/drum-layouts/submit/route.ts | Adds “begin upload” endpoint that mints presigned URL + placeholder record. |
| src/app/api/drum-layouts/submit/complete/actions.ts | Adds “upload complete” server action to process/validate/publish. |
| src/app/api/drum-layouts/route.ts | Adds listing endpoint. |
| src/app/api/drum-layouts/[id]/route.ts | Adds get-by-id endpoint. |
| src/app/api/drum-layouts/[id]/delete/route.ts | Adds delete endpoint with uploader authorization. |
| src/app/api/drum-layouts/[id]/download/route.ts | Adds download redirect endpoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // For now, just return placeholder validated data | ||
| const validatedData = { | ||
| name: 'Placeholder Layout', | ||
| description: null as string | null, | ||
| layoutFormat: { drumCounts: {} } as DrumLayoutFormat, | ||
| imageFile: null as { buffer: Buffer; filename: string } | null, | ||
| }; | ||
|
|
||
| // Upload image if present | ||
| let imagePath: string | null = null; | ||
| if (validatedData.imageFile) { | ||
| const uploadResult = await uploadDrumLayoutImage( | ||
| id, | ||
| validatedData.imageFile.buffer, | ||
| validatedData.imageFile.filename, | ||
| true | ||
| ); | ||
| if (!uploadResult.success) { | ||
| return uploadResult; | ||
| } | ||
| imagePath = uploadResult.value; | ||
| } | ||
|
|
||
| const now = new Date(); | ||
| const pool = await getDbPool(); | ||
| try { | ||
| const insertedLayout = await db | ||
| .upsert( | ||
| 'drum_layouts', | ||
| snakeCaseKeys({ | ||
| id, | ||
| visibility: MapVisibility.PUBLIC, | ||
| validity: MapValidity.VALID, | ||
| submissionDate: now, | ||
| name: validatedData.name, | ||
| description: validatedData.description, | ||
| uploader, | ||
| imagePath, | ||
| layoutFormat: validatedData.layoutFormat, | ||
| }), | ||
| ['id'] | ||
| ) | ||
| .run(pool); | ||
|
|
||
| await promoteTempDrumLayoutFiles(id); | ||
|
|
||
| return { | ||
| success: true, | ||
| value: DrumLayout.parse({ | ||
| ...camelCaseKeys(insertedLayout), | ||
| layoutFormat: DrumLayoutFormat.parse(insertedLayout.layout_format), | ||
| }), | ||
| }; | ||
| } catch (e) { | ||
| return { success: false, errors: [wrapError(e, DbError.UNKNOWN_DB_ERROR)] }; | ||
| } |
There was a problem hiding this comment.
validateUploadedDrumLayout currently overwrites the user-provided name/metadata with hard-coded placeholder values ('Placeholder Layout') and then publishes the layout as PUBLIC/VALID. This will result in incorrect public data and makes it hard to distinguish real uploads from the TODO placeholder. Prefer keeping the existing DB name/description (from the submit request) and leaving layout_format empty/partial until real parsing is implemented, or return a validation error until the TODO is implemented.
| // For now, just return placeholder validated data | |
| const validatedData = { | |
| name: 'Placeholder Layout', | |
| description: null as string | null, | |
| layoutFormat: { drumCounts: {} } as DrumLayoutFormat, | |
| imageFile: null as { buffer: Buffer; filename: string } | null, | |
| }; | |
| // Upload image if present | |
| let imagePath: string | null = null; | |
| if (validatedData.imageFile) { | |
| const uploadResult = await uploadDrumLayoutImage( | |
| id, | |
| validatedData.imageFile.buffer, | |
| validatedData.imageFile.filename, | |
| true | |
| ); | |
| if (!uploadResult.success) { | |
| return uploadResult; | |
| } | |
| imagePath = uploadResult.value; | |
| } | |
| const now = new Date(); | |
| const pool = await getDbPool(); | |
| try { | |
| const insertedLayout = await db | |
| .upsert( | |
| 'drum_layouts', | |
| snakeCaseKeys({ | |
| id, | |
| visibility: MapVisibility.PUBLIC, | |
| validity: MapValidity.VALID, | |
| submissionDate: now, | |
| name: validatedData.name, | |
| description: validatedData.description, | |
| uploader, | |
| imagePath, | |
| layoutFormat: validatedData.layoutFormat, | |
| }), | |
| ['id'] | |
| ) | |
| .run(pool); | |
| await promoteTempDrumLayoutFiles(id); | |
| return { | |
| success: true, | |
| value: DrumLayout.parse({ | |
| ...camelCaseKeys(insertedLayout), | |
| layoutFormat: DrumLayoutFormat.parse(insertedLayout.layout_format), | |
| }), | |
| }; | |
| } catch (e) { | |
| return { success: false, errors: [wrapError(e, DbError.UNKNOWN_DB_ERROR)] }; | |
| } | |
| // Until proper parsing/validation is implemented, treat this as a validation failure | |
| await this.setValidity(id, MapValidity.INVALID); | |
| return { | |
| success: false, | |
| errors: [ValidateDrumLayoutError.INVALID_FORMAT], | |
| }; |
|
|
||
| await promoteTempDrumLayoutFiles(id); | ||
|
|
There was a problem hiding this comment.
The result of promoteTempDrumLayoutFiles(id) is ignored. If S3 promotion fails, this function will still return success after publishing the DB row, leaving the DB pointing at assets that were not promoted (and the temp assets possibly orphaned). Capture the result and return/rollback on failure.
| const deleted = await db.deletes('drum_layouts', { id }).run(pool); | ||
| if (deleted.length === 0) { | ||
| return { success: false, errors: [{ type: DeleteDrumLayoutError.MISSING_DRUM_LAYOUT }] }; | ||
| } | ||
| await Promise.all([deleteDrumLayoutFiles(id, true), deleteDrumLayoutFiles(id, false)]); | ||
| return { success: true, value: undefined }; | ||
| } catch (e) { |
There was a problem hiding this comment.
deleteDrumLayout awaits deleteDrumLayoutFiles(...) but discards the returned Results and always returns success after the DB delete. Since the function’s error type includes S3Error, consider propagating S3 deletion failures (or at least logging/returning a failure) so callers don’t assume assets were removed when they weren’t.
| async validateUploadedDrumLayout( | ||
| opts: ProcessDrumLayoutOpts | ||
| ): PromisedResult< | ||
| DrumLayout, | ||
| S3Error | DbError | CreateDrumLayoutError | ValidateDrumLayoutError |
There was a problem hiding this comment.
The drum layouts upload/validation flow is new functionality but there are no Jest tests (similar to src/services/maps/tests/*) covering the happy path and cleanup/failure paths. Adding tests around the submit/complete flow would help prevent regressions.
| snakeCaseKeys({ | ||
| id, | ||
| visibility: MapVisibility.PUBLIC, | ||
| validity: MapValidity.VALID, | ||
| submissionDate: now, | ||
| name: validatedData.name, | ||
| description: validatedData.description, | ||
| uploader, | ||
| imagePath, | ||
| layoutFormat: validatedData.layoutFormat, | ||
| }), |
There was a problem hiding this comment.
snakeCaseKeys is applied to an object that includes layoutFormat (a JSONB payload). With snakecase-keys@^5.4.6, keys are transformed deeply by default, so layoutFormat: { drumCounts: ... } will be stored as { drum_counts: ... }, causing DrumLayoutFormat.parse(...) to fail when reading from the DB. Avoid deep snake-casing for JSONB fields (e.g., pass { deep: false } and map DB column names explicitly, or exclude layoutFormat from the transformation).
| snakeCaseKeys({ | |
| id, | |
| visibility: MapVisibility.PUBLIC, | |
| validity: MapValidity.VALID, | |
| submissionDate: now, | |
| name: validatedData.name, | |
| description: validatedData.description, | |
| uploader, | |
| imagePath, | |
| layoutFormat: validatedData.layoutFormat, | |
| }), | |
| snakeCaseKeys( | |
| { | |
| id, | |
| visibility: MapVisibility.PUBLIC, | |
| validity: MapValidity.VALID, | |
| submissionDate: now, | |
| name: validatedData.name, | |
| description: validatedData.description, | |
| uploader, | |
| imagePath, | |
| layoutFormat: validatedData.layoutFormat, | |
| }, | |
| { deep: false } | |
| ), |
| async getDrumLayout(id: string): PromisedResult<DrumLayout, GetDrumLayoutError> { | ||
| const { pool } = await getServerContext(); | ||
| try { | ||
| const layout = await db | ||
| .selectOne('drum_layouts', { id, visibility: MapVisibility.PUBLIC }) | ||
| .run(pool); |
There was a problem hiding this comment.
getDrumLayout filters by visibility: MapVisibility.PUBLIC, but this API is used by the upload-complete flow to look up the just-created placeholder record (which is created with visibility: HIDDEN). This will make dbLayoutResult fail for new uploads, skipping the uploader/previousValidity checks and allowing incorrect cleanup (and potentially allowing another logged-in user to complete/claim an upload if they learn the id). Introduce an internal lookup that can read hidden rows for server-side workflows (or add an includeHidden flag) and use it from the submit/complete action.
| async getDrumLayout(id: string): PromisedResult<DrumLayout, GetDrumLayoutError> { | |
| const { pool } = await getServerContext(); | |
| try { | |
| const layout = await db | |
| .selectOne('drum_layouts', { id, visibility: MapVisibility.PUBLIC }) | |
| .run(pool); | |
| async getDrumLayout( | |
| id: string, | |
| opts?: { includeHidden?: boolean } | |
| ): PromisedResult<DrumLayout, GetDrumLayoutError> { | |
| const { pool } = await getServerContext(); | |
| try { | |
| const where: db.WhereableForTable<'drum_layouts'> = { id }; | |
| if (!opts?.includeHidden) { | |
| (where as any).visibility = MapVisibility.PUBLIC; | |
| } | |
| const layout = await db.selectOne('drum_layouts', where).run(pool); |
| await Promise.all( | ||
| imageKeys.map((key) => | ||
| s3Move(key, key.replace(drumLayoutImagePrefix(id, true), drumLayoutImagePrefix(id, false))) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
promoteTempDrumLayoutFiles moves image objects via Promise.all(imageKeys.map(key => s3Move(...))) but never checks whether any of the s3Move operations failed (they return a Result). This can silently succeed while leaving some images in the temp prefix. Collect the results and return an S3_WRITE_ERROR if any move fails.
| await Promise.all( | |
| imageKeys.map((key) => | |
| s3Move(key, key.replace(drumLayoutImagePrefix(id, true), drumLayoutImagePrefix(id, false))) | |
| ) | |
| ); | |
| const imageMoveResults = await Promise.all( | |
| imageKeys.map((key) => | |
| s3Move(key, key.replace(drumLayoutImagePrefix(id, true), drumLayoutImagePrefix(id, false))) | |
| ) | |
| ); | |
| const failedMove = imageMoveResults.find((result) => !result.success); | |
| if (failedMove) { | |
| return failedMove; | |
| } |
| const { drumLayoutsRepo } = await getServerContext(); | ||
| const result = await drumLayoutsRepo.getDrumLayout(id); | ||
| if (result.success === false) { | ||
| return new NextResponse('Drum layout not found', { status: 404 }); | ||
| } | ||
|
|
||
| const filename = sanitizeForDownload(result.value.name); | ||
| redirect( | ||
| `${getEnvVars().publicS3BaseUrl}/drumLayouts/${result.value.id}.zip?title=${filename}.zip` | ||
| ); |
There was a problem hiding this comment.
Unlike the maps download endpoint, this route does not increment download_count in the DB. Since drum_layouts has a download_count column, this endpoint should likely mirror /api/maps/[id]/download by incrementing the count before redirecting.
| CREATE TABLE drum_layouts ( | ||
| _id serial, | ||
| id varchar(16) primary key, |
There was a problem hiding this comment.
The migration doesn’t qualify the table with the public schema (e.g., CREATE TABLE "public"."drum_layouts" ...). Other migrations here consistently reference "public", and relying on search_path can create the table in an unexpected schema in some environments.
| download_count int not null default 0 | ||
| ); | ||
|
|
||
| alter table drum_layouts enable row level security; |
There was a problem hiding this comment.
For consistency with other migrations, qualify the table name here as well (e.g., alter table "public"."drum_layouts" enable row level security).
Summary
drum_layoutsdatabase table with id, name, description, image_path, layout_format (jsonb), visibility, validity, uploader, and download_count columnsDrumLayoutFormatcontainingdrumCounts(Record<string, number>)DrumLayoutsReposervice with find, get, create, delete, and validate methods/api/drum-layouts/submit,/api/drum-layouts/submit/complete,/api/drum-layouts/[id],/api/drum-layouts,/api/drum-layouts/[id]/delete,/api/drum-layouts/[id]/downloadCloses #47
Notes
validateUploadedDrumLayoutmethod has a TODO placeholder for actual file parsing - this needs to be implemented to unzip and extract layout metadata🤖 Generated with Claude Code