feat: persist swagger schemas - #84
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds Supabase-backed schema persistence for authenticated users, a POST API route for saving schemas, server-side restoration in the locale page, and initial-schema wiring through ChangesSchema persistence flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Home
participant SchemaServerService
participant Supabase
participant SwaggerEditor
participant SchemaService
Home->>SchemaServerService: restoreSchemaForCurrentUser()
SchemaServerService->>Supabase: query authenticated user's schema
Supabase-->>SchemaServerService: saved content and format
Home->>SwaggerEditor: pass initialSchema
SwaggerEditor->>SchemaService: save schema
SchemaService->>Home: return save response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/app/api/schema/route.test.ts (1)
44-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test for the
result === 'error'→ 500 branch.Tests cover 200/400/401 but not the 500 case handled in the route.
✅ Suggested additional test
it('returns unauthorized when there is no authenticated user', async () => { mockSaveSchemaForCurrentUser.mockResolvedValue('unauthorized'); const response = await POST(createRequest({ content: 'openapi: 3.0.0', format: 'yaml' })); expect(response.status).toBe(401); }); + + it('returns a server error when saving fails', async () => { + mockSaveSchemaForCurrentUser.mockResolvedValue('error'); + + const response = await POST(createRequest({ content: 'openapi: 3.0.0', format: 'yaml' })); + + expect(response.status).toBe(500); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/schema/route.test.ts` around lines 44 - 51, Add a test alongside the existing POST route cases in schema route.test.ts that configures mockSaveSchemaForCurrentUser to resolve to 'error', invokes POST with a valid schema request, and asserts the response status is 500.src/services/schema-server-service.test.ts (1)
35-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the authenticated-but-Supabase-error branches.
Tests cover success and unauthenticated paths for both functions, but not the case where the user is authenticated and Supabase returns an
error(e.g., RLS denial or DB failure) onmaybeSingle()/upsert(). Those branches (error || !isSavedSchema(data)anderror ? 'error' : 'saved') are untested.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/schema-server-service.test.ts` around lines 35 - 87, The tests for restoreSchemaForCurrentUser and saveSchemaForCurrentUser cover success and unauthenticated cases but omit authenticated Supabase-error paths. Add tests with an authenticated user where mockMaybeSingle returns an error and where mockUpsert returns an error, asserting the restore function returns null and the save function returns 'error', respectively.src/services/schema-server-service.ts (1)
7-56: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSupabase failures are silently swallowed with no logging or exception guarding.
Every Supabase call here (
auth.getUser(),select(),upsert()) only checks theerrorfield and falls back tonull/'error'without ever logging — production save/restore failures (RLS denials, network errors, misconfig) become invisible. There's also notry/catcharoundcreateClient()/getUser(), so an unexpected thrown exception (vs. a returned error object) would propagate uncaught into the Server Component render path (page.tsx) instead of degrading gracefully.♻️ Proposed fix: log errors and guard against thrown exceptions
async function getCurrentUserId() { - const supabase = await createClient(); - const { data, error } = await supabase.auth.getUser(); - - if (error || !data.user) { - return { supabase, userId: null }; - } - - return { supabase, userId: data.user.id }; + const supabase = await createClient(); + + try { + const { data, error } = await supabase.auth.getUser(); + + if (error || !data.user) { + if (error) console.error('Failed to resolve current user', error); + return { supabase, userId: null }; + } + + return { supabase, userId: data.user.id }; + } catch (unexpectedError) { + console.error('Unexpected error resolving current user', unexpectedError); + return { supabase, userId: null }; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/schema-server-service.ts` around lines 7 - 56, Update restoreSchemaForCurrentUser, saveSchemaForCurrentUser, and getCurrentUserId to log Supabase errors with relevant operation context, and wrap createClient(), auth.getUser(), select(), and upsert() flows in try/catch guards. Preserve the existing null, unauthorized, and 'error' fallback results while ensuring both returned Supabase errors and thrown exceptions are logged and do not escape the Server Component render path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/api/schema/route.ts`:
- Around line 1-22: Update POST to validate the parsed payload with
validateSwaggerSchema after isSavedSchema and before saveSchemaForCurrentUser;
reject invalid OpenAPI content with the existing 400 invalid-payload response,
while preserving the current authorization and save-error handling.
---
Nitpick comments:
In `@src/app/api/schema/route.test.ts`:
- Around line 44-51: Add a test alongside the existing POST route cases in
schema route.test.ts that configures mockSaveSchemaForCurrentUser to resolve to
'error', invokes POST with a valid schema request, and asserts the response
status is 500.
In `@src/services/schema-server-service.test.ts`:
- Around line 35-87: The tests for restoreSchemaForCurrentUser and
saveSchemaForCurrentUser cover success and unauthenticated cases but omit
authenticated Supabase-error paths. Add tests with an authenticated user where
mockMaybeSingle returns an error and where mockUpsert returns an error,
asserting the restore function returns null and the save function returns
'error', respectively.
In `@src/services/schema-server-service.ts`:
- Around line 7-56: Update restoreSchemaForCurrentUser,
saveSchemaForCurrentUser, and getCurrentUserId to log Supabase errors with
relevant operation context, and wrap createClient(), auth.getUser(), select(),
and upsert() flows in try/catch guards. Preserve the existing null,
unauthorized, and 'error' fallback results while ensuring both returned Supabase
errors and thrown exceptions are logged and do not escape the Server Component
render 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 Plus
Run ID: 79f5c3f0-8d4e-4445-9239-ab432ad1bf07
📒 Files selected for processing (14)
src/app/[locale]/page.test.tsxsrc/app/[locale]/page.tsxsrc/app/api/schema/route.test.tssrc/app/api/schema/route.tssrc/components/swagger-control/swagger-control.test.tsxsrc/components/swagger-editor/swagger-editor.test.tsxsrc/components/swagger-editor/swagger-editor.tsxsrc/services/schema-persistence.tssrc/services/schema-server-service.test.tssrc/services/schema-server-service.tssrc/services/schema-service.tssrc/utils/hooks/use-swagger.test.tssrc/utils/hooks/use-swagger.tssupabase/schema-persistence.sql
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/app/api/schema/route.test.ts (1)
52-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider asserting the call args, not just the status code.
This test only checks
response.status. Adding an assertion thatmockSaveSchemaForCurrentUserwas called with the expected payload would strengthen coverage of the unauthorized branch, consistent with the success test at Line 26.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/schema/route.test.ts` around lines 52 - 58, Strengthen the unauthorized test by asserting that mockSaveSchemaForCurrentUser was called with the expected schema payload and format, matching the argument assertion used in the success test while retaining the existing 401 status assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/app/api/schema/route.test.ts`:
- Around line 52-58: Strengthen the unauthorized test by asserting that
mockSaveSchemaForCurrentUser was called with the expected schema payload and
format, matching the argument assertion used in the success test while retaining
the existing 401 status assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: adb71ea2-b65d-41b8-b68c-547e9024c17c
📒 Files selected for processing (4)
src/app/api/schema/route.test.tssrc/app/api/schema/route.tssrc/services/schema-server-service.test.tssrc/services/schema-server-service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/api/schema/route.ts
- src/services/schema-server-service.ts
Summary
Closes #44
Adds schema persistence for authenticated users in the Swagger Editor.
What Changed
SwaggerEditorasinitialSchema.useSwaggerto avoid empty editor flicker after mount.useSwagger./api/schemaPOST route for saving schemas.json/yaml) instead of only parsed JSON.public.schemastable and RLS own-user policies.Affected Files
src/app/[locale]/page.tsxsrc/app/[locale]/page.test.tsxsrc/app/api/schema/route.tssrc/app/api/schema/route.test.tssrc/components/swagger-control/swagger-control.test.tsxsrc/components/swagger-editor/swagger-editor.tsxsrc/components/swagger-editor/swagger-editor.test.tsxsrc/services/schema-persistence.tssrc/services/schema-server-service.tssrc/services/schema-server-service.test.tssrc/services/schema-service.tssrc/utils/hooks/use-swagger.tssrc/utils/hooks/use-swagger.test.tssupabase/schema-persistence.sqlValidation
npm testnpm run lintnpm run buildgit diff --checkManual Check Required
supabase/schema-persistence.sqlin Supabase.Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests