feat: Handle inactive users #79
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughA new API endpoint for cleaning up pages tied to inactive subscriptions was added, leveraging a new Supabase function and updated type definitions. The database migration introduces a function to fetch such pages and modifies a foreign key constraint for cascading deletes. The webhook handler was enhanced to delete custom domains on page deletion. Type utilities and enums in the Supabase types package were refactored and extended. Additionally, a new scheduled job was added to the Vercel configuration for running the cleanup daily. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API (cleanupInactivePagesJob)
participant Supabase
Client->>API (cleanupInactivePagesJob): POST /api/billing/jobs/cleanup-inactive-pages
API->>Supabase: Call get_pages_with_inactive_subscriptions()
Supabase-->>API: Return list of inactive pages
alt Pages found
API->>Supabase: Delete pages by IDs
Supabase-->>API: Deletion result
API-->>Client: Respond with job ID, status, deleted count
else No pages found
API-->>Client: Respond with job ID, status, zero deleted
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/web/pages/api/billing/jobs/cleanup-inactive-pages.ts (1)
64-91: Excellent audit logging implementationThe comprehensive logging with unique job IDs provides a good audit trail for tracking deletions. The structured logging approach makes it easy to trace operations.
One minor improvement for error handling:
res.status(500).json({ error: { statusCode: 500, - message: err.message || "Internal server error", + message: process.env.NODE_ENV === "production" + ? "Internal server error" + : (err.message || "Internal server error"), }, });This prevents leaking potentially sensitive error details in production.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/pages/api/billing/jobs/cleanup-inactive-pages.ts(1 hunks)packages/supabase/migrations/17_handle_inactive_pages.sql(1 hunks)packages/supabase/types/index.ts(3 hunks)
🔇 Additional comments (4)
packages/supabase/migrations/17_handle_inactive_pages.sql (1)
1-10: Good approach for maintaining referential integrityAdding CASCADE DELETE to the foreign key constraint ensures audit logs are automatically cleaned up when pages are deleted. This prevents orphaned records and simplifies the cleanup process.
packages/supabase/types/index.ts (3)
9-14: Good addition for Supabase client configurationThe
__InternalSupabaseproperty enables automatic client instantiation with the correct Postgrest version, improving developer experience.
561-570: Type definition correctly matches the SQL functionThe function type accurately reflects the SQL function signature and return type from the migration file.
744-753: Useful addition for runtime enum accessThe
Constantsexport provides runtime access to enum values, which is helpful for validation and UI components.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/pages/api/pages/settings/webhook.ts (1)
24-41: LGTM with suggestions for enhanced security and error handling.The DELETE event handler correctly integrates with Vercel's API to clean up custom domains. The static analysis SSRF warning is likely a false positive since
custom_domaincomes from the database record, not direct user input.Consider these improvements for robustness:
if (type === "DELETE") { if (page_settings?.custom_domain) { + // Validate domain format for defense in depth + const domainRegex = /^[a-zA-Z0-9][a-zA-Z0-9-]{1,61}[a-zA-Z0-9]\.[a-zA-Z]{2,}$/; + if (!domainRegex.test(page_settings.custom_domain)) { + console.error("Invalid domain format:", page_settings.custom_domain); + return res.status(200).json({ ok: true }); + } + try { const response = await fetch( `https://api.vercel.com/v8/projects/${process.env.VERCEL_PAGES_PROJECT_ID}/domains/${page_settings.custom_domain}?teamId=${process.env.VERCEL_TEAM_ID}`, { headers: { Authorization: `Bearer ${process.env.VERCEL_AUTH_TOKEN}`, }, method: "DELETE", } - ).then((res) => res.json()); + ); + + if (!response.ok) { + console.error(`Vercel API error: ${response.status} ${response.statusText}`); + const errorData = await response.json(); + console.error("Vercel API error details:", errorData); + } else { + const data = await response.json(); + console.log("Response from Vercel API:", data); + } - console.log("Response from Vercel API:", response); } catch (error) { console.error("Error deleting custom domain:", error); } } }These changes add:
- Domain format validation to address potential security concerns
- Proper HTTP response status checking
- Better error logging for debugging
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/pages/api/pages/settings/webhook.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeQL
apps/web/pages/api/pages/settings/webhook.ts
[failure] 27-35: Server-side request forgery
The URL of this request depends on a user-provided value.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/supabase/migrations/17_handle_inactive_pages.sql (1)
13-47: Pagination & indexing were already requested – still unresolvedPrevious review highlighted that returning the entire result set with no
LIMIT / OFFSETand without supporting indexes can hurt production performance. The function body is unchanged, so the concern remains.
🧹 Nitpick comments (2)
packages/supabase/migrations/17_handle_inactive_pages.sql (2)
2-3: Make the migration idempotent by guarding the drop withIF EXISTSRunning this migration twice (e.g. on a restored staging DB) will fail because the constraint is already gone. Add
IF EXISTSto make the script safe to re-run.-ALTER TABLE page_audit_logs -DROP CONSTRAINT page_audit_logs_page_id_fkey; +ALTER TABLE page_audit_logs +DROP CONSTRAINT IF EXISTS page_audit_logs_page_id_fkey;
6-10: Protect the re-creation against name clashesIf a different migration has already introduced a cascade version of this FK,
ADD CONSTRAINTwill error. Consider the DO-block pattern to check first, or pick a new constraint name.
| { | ||
| "path": "/api/billing/jobs/cleanup-inactive-pages", | ||
| "schedule": "0 1 * * *" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Specify HTTP method to prevent side-effectful cleanup from defaulting to GET
Vercel cron invocations default to a GET request, which is semantically wrong for a mutation-type endpoint and can introduce caching or proxy side-effects. Declare the intended verb explicitly.
"path": "/api/billing/jobs/cleanup-inactive-pages",
+ "method": "POST",
"schedule": "0 1 * * *"Ensure the handler in pages/api/billing/jobs/cleanup-inactive-pages.ts is prepared to accept POST requests before applying the change.
🏁 Script executed:
#!/bin/bash
set -e
# Locate the cleanup-inactive-pages API handler file
handler_file=$(fd cleanup-inactive-pages.ts)
echo "Handler file found at: $handler_file"
# Inspect HTTP method checks in the handler
echo -e "\nChecking for HTTP method conditions (req.method) in the handler:"
grep -n -C2 "req.method" "$handler_file" || echo "No HTTP method checks found"
# Show the handler export signature for context
echo -e "\nHandler export signature:"
grep -n "export default" "$handler_file" | head -n 1Length of output: 864
Specify HTTP method for the cleanup-inactive-pages cron job
Your handler in apps/web/pages/api/billing/jobs/cleanup-inactive-pages.ts already enforces req.method === "POST" (returning 405 otherwise), so it’s safe—and recommended—to explicitly declare the HTTP verb in vercel.json to avoid a default GET invocation:
• File: apps/web/vercel.json
• Location: the block for /api/billing/jobs/cleanup-inactive-pages
{
"path": "/api/billing/jobs/cleanup-inactive-pages",
+ "method": "POST",
"schedule": "0 1 * * *"
}This ensures Vercel’s cron will invoke the endpoint with POST, matching your mutation-style handler.
📝 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.
| { | |
| "path": "/api/billing/jobs/cleanup-inactive-pages", | |
| "schedule": "0 1 * * *" | |
| } | |
| { | |
| "path": "/api/billing/jobs/cleanup-inactive-pages", | |
| "method": "POST", | |
| "schedule": "0 1 * * *" | |
| } |
🤖 Prompt for AI Agents
In apps/web/vercel.json around lines 7 to 10, the cron job for
/api/billing/jobs/cleanup-inactive-pages does not specify the HTTP method,
causing Vercel to default to GET. Since your handler requires POST, update the
configuration to explicitly include "method": "POST" in the job definition to
ensure the cron triggers a POST request matching your handler's expectations.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes