Repository navigation
feat(canvas): custom subdomain selection (Pro+) - #1727
Conversation
- Add canChooseSubdomain to plan definitions (Pro+ only) - Backend: changePublishSubdomain() validates, checks uniqueness, re-renders all published pages under new prefix, cleans up old artifacts - API: PATCH /api/drives/[driveId]/subdomain with tier-gating - UI: Subdomain card on Domains & Publishing settings - Test: verify canChooseSubdomain gating across all tiers
|
Warning Review limit reached
More reviews will be available in 10 minutes and 6 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds drive publish subdomain management across the API, publishing workflow, plan limits, and dashboard settings. The new route reads and updates subdomains, the publish layer migrates published artifacts, and plan data now exposes subdomain eligibility. ChangesDrive publish subdomain management
Sequence Diagram(s)sequenceDiagram
participant SubdomainCard
participant SubdomainRoute
participant changePublishSubdomain
participant publishedStorage
SubdomainCard->>SubdomainRoute: GET /api/drives/[driveId]/subdomain
SubdomainRoute->>SubdomainCard: subdomain + canChange
SubdomainCard->>SubdomainRoute: PATCH subdomain
SubdomainRoute->>changePublishSubdomain: changePublishSubdomain(driveId, subdomain, userId)
changePublishSubdomain->>publishedStorage: republishDriveCanonical(...)
changePublishSubdomain->>publishedStorage: clearPublishedPrefix(previousPrefix)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cafdf0eeec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await db.update(drives).set({ publishSubdomain: normalized }).where(eq(drives.id, driveId)); | ||
|
|
||
| // Re-render all published pages under the new subdomain prefix. | ||
| await republishDriveCanonical(driveId, userId); |
There was a problem hiding this comment.
Require successful migration before clearing old artifacts
When any published page fails to re-render during a subdomain change (for example an asset rewrite or publish-bucket upload error), republishDriveCanonical only logs the per-page failure and returns a count, but this caller ignores that result and proceeds to clear the old prefix. In that scenario the API reports success while some pages have no artifact under the new subdomain and the old working artifacts have been deleted, breaking the published site for those pages.
Useful? React with 👍 / 👎.
| return NextResponse.json({ error: 'Only drive owners and admins can change the subdomain' }, { status: 403 }); | ||
| } | ||
|
|
||
| if (!(await canChooseSubdomain(driveId))) { |
There was a problem hiding this comment.
Initialize the new plan limit before gating Pro users
For Pro and Founder drive owners this gate still rejects the request because the new limits.canChooseSubdomain value is not set on those plan objects, so getPlan(...).limits.canChooseSubdomain evaluates to undefined. That makes the advertised Pro+ feature available only to Business owners unless every tier gets an explicit value before this check is used.
Useful? React with 👍 / 👎.
| </> | ||
| ) : ( | ||
| <p className="text-sm text-muted-foreground"> | ||
| Custom subdomain selection is a Pro feature. <Link href="/dashboard/settings/billing" className="text-blue-500 hover:underline">Upgrade</Link> to choose your own subdomain. |
There was a problem hiding this comment.
Point the upgrade CTA at an existing settings route
For free users who click this Upgrade link, the target path appears to be a dead route: I checked the app routes and billing/plan pages live under /settings/billing and /settings/plan, with no /dashboard/settings/billing route or redirect. This sends the intended upgrade flow to a 404 instead of the billing UI.
Useful? React with 👍 / 👎.
- add canChooseSubdomain for free/pro/founder tiers - fix missing publish-page imports (and/ne/clearPublishedPrefix) - abort migration cleanup when republish count is incomplete - map update unique-violation to 409 - fix plans.test bracket structure - fix free-tier upgrade CTA route to /settings/billing
|
AIDD Reviewer pass completed and blockers addressed. Applied in commit
This should clear the CI blockers and satisfy the high-priority review comments. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@apps/web/src/app/dashboard/`[driveId]/settings/domains/page.tsx:
- Around line 35-38: The subdomain settings UI is hard-coding the publish domain
instead of using the configured host, so update the SubdomainResponse and the
related domain settings flow to return and consume the actual publish host (for
example via url or publishHost) rather than reconstructing URLs with
pagespace.site. Use the existing domain settings components and API handling
around SubdomainResponse and the PATCH response to render the server-provided
host everywhere the publish URL is shown, including the configured card and any
success/response text.
In `@apps/web/src/lib/canvas/publish-page.ts`:
- Around line 503-505: The cleanup in publishPage can delete a prefix that may
already have been reclaimed by another drive once oldSubdomain is released.
Update the oldSubdomain cleanup flow in publishPage/clearPublishedPrefix so the
subdomain stays reserved until cleanup finishes, or guard it with a
per-subdomain lock or ownership check before deleting artifacts. Make sure the
logic around oldSubdomain and clearPublishedPrefix only removes artifacts proven
to belong to the current drive.
- Around line 477-497: The publishSubdomain update is committed in the
db.update(drives) step before republishing is confirmed, so a partial failure
leaves the drive pointing at an incomplete migration. In publish-page.ts, adjust
the publish flow around db.update, publishedPages.findMany, and
republishDriveCanonical so the subdomain change is only committed after
republishing succeeds, or add rollback/cleanup to restore the previous subdomain
when the refreshed count does not match. Keep the unique violation handling in
place and ensure the migration path leaves the drive in a consistent state on
any error.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 09f392d4-7c5a-4071-ab83-859927185e5e
📒 Files selected for processing (5)
apps/web/src/app/api/drives/[driveId]/subdomain/route.tsapps/web/src/app/dashboard/[driveId]/settings/domains/page.tsxapps/web/src/lib/canvas/publish-page.tsapps/web/src/lib/subscription/__tests__/plans.test.tsapps/web/src/lib/subscription/plans.ts
… safety - return publishHost from subdomain API and use it in settings UI - stop hard-coding pagespace.site in subdomain card preview/links - add rollback of drives.publishSubdomain when migration fails - avoid old-prefix deletion race by removing prefix cleanup step
|
Addressed latest review comments in commit What changed:
Files updated:
|
What
Pro+ users can now change their published canvas site's subdomain from the auto-allocated slug to a custom one via Drive Settings -> Domains & Publishing.
Changes
plans.tscanChooseSubdomaintoPlanDefinition.limits(false: free, true: pro/founder/business)publish-page.tschangePublishSubdomain(): validates subdomain, checks uniqueness, updatesdrives.publishSubdomain, re-renders all published pages under new prefix, regenerates site files, cleans up old artifactsapi/drives/[driveId]/subdomain/route.tsdomains/page.tsxplans.test.tscanChooseSubdomaingating across all tiersTier gating
Migration behavior
When a user changes their subdomain:
drives.publishSubdomainupdated in DBrepublishDriveCanonical)clearPublishedPrefix(best-effort)Security
Notes
tsc --noEmitor tests in sandbox (no node_modules). CI should catch any type errors.Summary by CodeRabbit
New Features
Bug Fixes
Tests