-
Notifications
You must be signed in to change notification settings - Fork 225
fix: only show upgrade prompt to org owners, not regular users (#815) #865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: only show upgrade prompt to org owners, not regular users (#815) #865
Conversation
…ebot-dev#815) Root cause: The layout's billing check wraps ALL users in UpgradeGuard when the subscription is expired, redirecting everyone to the upgrade page. Non-owner users cannot perform billing actions, so showing them the upgrade page is confusing. Fix: Hoist the membership variable from the auth block and check the user's role at the billing guard. Only org owners are redirected to the upgrade page. Non-owners see a message asking them to contact their organization owner.
WalkthroughThis change modifies subscription expiry handling in the domain layout component to restrict upgrade prompts to organization owners only. Non-owners now receive an in-page "Subscription Expired" message instead of the upgrade guard popup, while unauthenticated and other flows remain unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/web/src/app/`[domain]/layout.tsx:
- Around line 188-201: The current expired-subscription UI renders
LogoutEscapeHatch and an owner-contact message even for anonymous visitors;
update the else branch in layout.tsx to (1) only render <LogoutEscapeHatch> when
session is present (guard it with a session check) and (2) change the copy based
on whether session/membership is null—e.g., show "This organization's
subscription has expired." or other non-actionable text for anonymous users and
keep the "contact your organization owner" text for authenticated members;
locate the logic around session, membership and LogoutEscapeHatch to apply these
conditional checks and text variants.
🧹 Nitpick comments (1)
packages/web/src/app/[domain]/layout.tsx (1)
66-70: Consider simplifying the type annotation.This
Awaited<ReturnType<typeof prisma.userToOrg.findUnique<{…}>>>encoding is fragile—if the query shape changes, the standalone type must be updated in lockstep. Using Prisma's generated types (e.g.,Prisma.UserToOrgGetPayload<{ include: { user: true } }> | null) or simply letting TypeScript infer the type with a two-step pattern would be cleaner:♻️ Suggested simplification
- // Hoist membership so it's available for the billing owner check below (`#815`) - let membership: Awaited<ReturnType<typeof prisma.userToOrg.findUnique<{ - where: { orgId_userId: { orgId: string; userId: string } }; - include: { user: true }; - }>>> = null; + // Hoist membership so it's available for the billing owner check below (`#815`) + let membership: Prisma.UserToOrgGetPayload<{ include: { user: true } }> | null = null;This requires importing
Prismafrom@prisma/client. Alternatively, just use the inferred type from the query result if theincludeshape is stable.#!/bin/bash # Check what Prisma types are available for UserToOrg rg -n "UserToOrg" --type=ts -g '!node_modules/**' -l
| } else { | ||
| return ( | ||
| <div className="min-h-screen flex items-center justify-center p-6"> | ||
| <LogoutEscapeHatch className="absolute top-0 right-0 p-6" /> | ||
| <div className="text-center max-w-md"> | ||
| <h2 className="text-xl font-semibold mb-2">Subscription Expired</h2> | ||
| <p className="text-muted-foreground"> | ||
| Your organization's subscription has expired or is inactive. | ||
| Please contact your organization owner to renew the subscription. | ||
| </p> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anonymous users with expired subscriptions see a logout escape hatch and an owner-contact message.
When anonymous access is enabled and the subscription is expired, unauthenticated visitors (session === null, membership === null) land in this else branch. They'll see the LogoutEscapeHatch (which may be a no-op if it checks session internally—worth verifying) and a message to "contact your organization owner," which isn't actionable for anonymous visitors.
Consider either:
- Guarding
LogoutEscapeHatchwithsessionhere, and - Adjusting the copy for unauthenticated users (e.g., "This organization's subscription has expired.").
#!/bin/bash
# Check if LogoutEscapeHatch renders conditionally based on session
fd "logoutEscapeHatch" --type f --exec cat {}🤖 Prompt for AI Agents
In `@packages/web/src/app/`[domain]/layout.tsx around lines 188 - 201, The current
expired-subscription UI renders LogoutEscapeHatch and an owner-contact message
even for anonymous visitors; update the else branch in layout.tsx to (1) only
render <LogoutEscapeHatch> when session is present (guard it with a session
check) and (2) change the copy based on whether session/membership is null—e.g.,
show "This organization's subscription has expired." or other non-actionable
text for anonymous users and keep the "contact your organization owner" text for
authenticated members; locate the logic around session, membership and
LogoutEscapeHatch to apply these conditional checks and text variants.
Summary
Fixes #815.
Root cause: The layout's billing check at
packages/web/src/app/[domain]/layout.tsxwraps ALL users inUpgradeGuardwhen the subscription is expired or inactive. This redirects every user — including non-owners — to the upgrade page. Non-owner users cannot perform billing actions, so showing them the upgrade prompt is misleading.Fix: Check the user's organization role before redirecting to the upgrade page. Only org owners see the upgrade redirect. Non-owners see a static message asking them to contact their organization owner.
Changes
packages/web/src/app/[domain]/layout.tsx:membershipvariable from inside theif (session)block to the outer scope, making it available for the billing check.membership?.role === 'OWNER'determines whether to show the upgrade page or the "contact your owner" message.Root Cause Analysis
The
membershipvariable was scoped inside theif (session)block, making it inaccessible to the billing check further down in the layout. The billing guard had no way to distinguish between owners and regular users, so it treated all authenticated members equally.Risk Assessment
UpgradeGuardbehavior is preserved for owners.membershipvariable uses explicit Prisma type annotation to maintain type safety.Summary by CodeRabbit