ENG-490 NextJS endpoints to access the sync functions #230
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughTwo new API routes are introduced for managing synchronization tasks via Supabase. The first route allows proposing and querying sync tasks by function and target, supporting GET, POST, and OPTIONS methods. The second route enables updating the sync task status for a specific function, target, and worker using a POST method, with input validation and error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Route
participant Supabase
Client->>API_Route: POST /sync_task/{fn}/{target} (propose task)
API_Route->>Supabase: Call propose_sync_task RPC
Supabase-->>API_Route: Response
API_Route-->>Client: Normalized API response
Client->>API_Route: GET /sync_task/{fn}/{target} (query task)
API_Route->>Supabase: Query sync_info table
Supabase-->>API_Route: Sync info record
API_Route-->>Client: API response
Client->>API_Route: POST /sync_task/{fn}/{target}/{worker} (update status)
API_Route->>Supabase: Call end_sync_task RPC
Supabase-->>API_Route: Response
API_Route-->>Client: Success indicator
Possibly related PRs
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![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 (4)
apps/website/app/api/supabase/sync_task/[fn]/[target]/route.ts (2)
38-47: Improve error message clarity.The error message "Missing some worker" is unclear and could be more specific.
- "Missing some worker", + "Worker field is required",
55-59: Clarify the null response handling logic.The comment and logic handling null responses could benefit from more explanation about why this transformation is necessary.
Consider adding a more descriptive comment:
- // next not happy about null as a response + // NextJS responses cannot handle null values, convert to boolean success indicatorapps/website/app/api/supabase/sync_task/[fn]/[target]/[worker]/route.ts (2)
19-24: Maintain consistency with error message format.The error message format is inconsistent with the similar validation in the other route file. Consider using the original
targetvalue instead oftargetNfor clarity.- asPostgrestFailure(`${targetN} is not a number`, "type"), + asPostgrestFailure(`${target} is not a number`, "type"),
40-43: Clarify the 204 status code transformation.The logic for transforming a 204 status to 200 with
data: truecould benefit from a comment explaining why this transformation is necessary.Consider adding a comment:
+ // Transform 204 No Content to 200 OK with success indicator for API consistency if (response.status === 204) { response.data = true; response.status = 200; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/website/app/api/supabase/sync_task/[fn]/[target]/[worker]/route.ts(1 hunks)apps/website/app/api/supabase/sync_task/[fn]/[target]/route.ts(1 hunks)
🔇 Additional comments (4)
apps/website/app/api/supabase/sync_task/[fn]/[target]/route.ts (2)
17-20: LGTM! Clean default configuration setup.The sync defaults are well-defined with reasonable timeout and task interval values.
68-88: LGTM! Well-implemented GET handler with proper validation.The GET handler correctly validates the target parameter and uses
maybeSingle()for querying a single record, which is appropriate for this use case.apps/website/app/api/supabase/sync_task/[fn]/[target]/[worker]/route.ts (2)
25-32: LGTM! Robust enum validation using database types.The task status validation properly uses the database-generated enum types, ensuring type safety and preventing invalid status values.
34-39: Verify the end_sync_task RPC function exists in the database.Ensure that the
end_sync_taskstored procedure is properly defined in your Supabase database with the expected parameters.#!/bin/bash # Description: Search for references to end_sync_task RPC function in the codebase # Expected: Find database migration or function definition files rg -A 10 -B 5 "end_sync_task"
apps/website/app/api/supabase/sync_task/[fn]/[target]/[worker]/route.ts
Outdated
Show resolved
Hide resolved
mdroidian
left a comment
mdroidian
left a comment
There was a problem hiding this comment.
Make sure to format with Prettier.
Also, have you tested these? If so, could you indicate this in the body of the PR or (preferably) a screenshot of the successful test.
apps/website/app/api/supabase/sync_task/[fn]/[target]/[worker]/route.ts
Outdated
Show resolved
Hide resolved
|
My vscode prettier config was non-operational, for some reason; fixed. |
maparent
force-pushed
the
ENG-490-sync-wrappers
branch
from
fde67b9 to
f12620b
Compare
maparent
force-pushed
the
ENG-490-sync-wrappers
branch
from
f12620b to
6699bf6
Compare
maparent
force-pushed
the
ENG-490-sync-wrappers
branch
from
6699bf6 to
1d58982
Compare
maparent
force-pushed
the
ENG-490-sync-wrappers
branch
from
1d58982 to
cd39dc1
Compare
This should allow people to use the task syncing functions through the Vercel endpoints.
Summary by CodeRabbit
Tested with