-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Pro seat management and organization settings restructure #1640
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
Changes from 15 commits
51e7277
7673b00
179ead4
6f5ad5c
2d1354e
272923c
7e7520f
a6c7cb4
9faadfd
a6143bc
da98ca6
3e55f5f
8611afd
20b5e46
c081e04
5519f91
bf8db1c
f858a56
b3a63b8
c6fd14a
2e5442c
1fa3fd8
f3ab753
c82516f
024bf20
2af60c4
65547e6
968703b
89b0d4f
aa9d327
7caa889
1a1b41f
516031a
c1c743c
15d94d3
df8a6b0
3eeb701
4610f9c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,75 @@ | ||||||
| "use server"; | ||||||
|
|
||||||
| import { db } from "@cap/database"; | ||||||
| import { getCurrentUser } from "@cap/database/auth/session"; | ||||||
| import { organizations, users } from "@cap/database/schema"; | ||||||
| import { stripe } from "@cap/utils"; | ||||||
| import type { Organisation } from "@cap/web-domain"; | ||||||
| import { eq } from "drizzle-orm"; | ||||||
|
|
||||||
| export type SubscriptionDetails = { | ||||||
| planName: string; | ||||||
| status: string; | ||||||
| billingInterval: "month" | "year"; | ||||||
| pricePerSeat: number; | ||||||
| currentQuantity: number; | ||||||
| currentPeriodEnd: number; | ||||||
| currency: string; | ||||||
| }; | ||||||
|
|
||||||
| export async function getSubscriptionDetails( | ||||||
| organizationId: Organisation.OrganisationId, | ||||||
| ): Promise<SubscriptionDetails | null> { | ||||||
| const user = await getCurrentUser(); | ||||||
| if (!user) throw new Error("Unauthorized"); | ||||||
|
|
||||||
| const [organization] = await db() | ||||||
| .select() | ||||||
| .from(organizations) | ||||||
| .where(eq(organizations.id, organizationId)) | ||||||
| .limit(1); | ||||||
|
|
||||||
| if (!organization) throw new Error("Organization not found"); | ||||||
| if (organization.ownerId !== user.id) | ||||||
| throw new Error("Only the owner can view subscription details"); | ||||||
|
|
||||||
| const [owner] = await db() | ||||||
| .select({ | ||||||
| stripeSubscriptionId: users.stripeSubscriptionId, | ||||||
| stripeSubscriptionStatus: users.stripeSubscriptionStatus, | ||||||
| }) | ||||||
| .from(users) | ||||||
| .where(eq(users.id, user.id)) | ||||||
| .limit(1); | ||||||
|
|
||||||
| if (!owner?.stripeSubscriptionId) { | ||||||
| return null; | ||||||
| } | ||||||
|
|
||||||
| const subscription = await stripe().subscriptions.retrieve( | ||||||
| owner.stripeSubscriptionId, | ||||||
| ); | ||||||
|
|
||||||
| if (subscription.status !== "active" && subscription.status !== "trialing") { | ||||||
| return null; | ||||||
| } | ||||||
|
|
||||||
| const item = subscription.items.data[0]; | ||||||
| if (!item) return null; | ||||||
|
|
||||||
| const price = item.price; | ||||||
| const unitAmount = price.unit_amount ?? 0; | ||||||
| const interval = price.recurring?.interval === "year" ? "year" : "month"; | ||||||
| const pricePerSeat = | ||||||
| interval === "year" ? unitAmount / 100 / 12 : unitAmount / 100; | ||||||
|
|
||||||
| return { | ||||||
| planName: "Cap Pro", | ||||||
| status: subscription.status, | ||||||
| billingInterval: interval, | ||||||
| pricePerSeat, | ||||||
| currentQuantity: item.quantity || 1, | ||||||
|
||||||
| currentQuantity: item.quantity || 1, | |
| currentQuantity: item.quantity ?? 1, |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,10 +5,15 @@ import { getCurrentUser } from "@cap/database/auth/session"; | |||||||||||||||||||||||
| import { sendEmail } from "@cap/database/emails/config"; | ||||||||||||||||||||||||
| import { OrganizationInvite } from "@cap/database/emails/organization-invite"; | ||||||||||||||||||||||||
| import { nanoId } from "@cap/database/helpers"; | ||||||||||||||||||||||||
| import { organizationInvites, organizations } from "@cap/database/schema"; | ||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||
| organizationInvites, | ||||||||||||||||||||||||
| organizationMembers, | ||||||||||||||||||||||||
| organizations, | ||||||||||||||||||||||||
| users, | ||||||||||||||||||||||||
| } from "@cap/database/schema"; | ||||||||||||||||||||||||
| import { serverEnv } from "@cap/env"; | ||||||||||||||||||||||||
| import type { Organisation } from "@cap/web-domain"; | ||||||||||||||||||||||||
| import { eq } from "drizzle-orm"; | ||||||||||||||||||||||||
| import { and, eq, inArray } from "drizzle-orm"; | ||||||||||||||||||||||||
| import { revalidatePath } from "next/cache"; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export async function sendOrganizationInvites( | ||||||||||||||||||||||||
|
|
@@ -21,48 +26,109 @@ export async function sendOrganizationInvites( | |||||||||||||||||||||||
| throw new Error("Unauthorized"); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const organization = await db() | ||||||||||||||||||||||||
| const [organization] = await db() | ||||||||||||||||||||||||
| .select() | ||||||||||||||||||||||||
| .from(organizations) | ||||||||||||||||||||||||
| .where(eq(organizations.id, organizationId)); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (!organization || organization.length === 0) { | ||||||||||||||||||||||||
| if (!organization) { | ||||||||||||||||||||||||
| throw new Error("Organization not found"); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (organization[0]?.ownerId !== user.id) { | ||||||||||||||||||||||||
| throw new Error("Only the owner can send organization invites"); | ||||||||||||||||||||||||
| if (organization.ownerId !== user.id) { | ||||||||||||||||||||||||
| throw new Error("Only the organization owner can send invites"); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const MAX_INVITES = 50; | ||||||||||||||||||||||||
| if (invitedEmails.length > MAX_INVITES) { | ||||||||||||||||||||||||
| throw new Error(`Cannot send more than ${MAX_INVITES} invites at once`); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | ||||||||||||||||||||||||
| const validEmails = invitedEmails.filter((email) => | ||||||||||||||||||||||||
| emailRegex.test(email.trim()), | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
| const validEmails = invitedEmails | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| const validEmails = invitedEmails | |
| const validEmails = Array.from( | |
| new Set( | |
| invitedEmails | |
| .map((email) => email.trim().toLowerCase()) | |
| .filter((email) => emailRegex.test(email)), | |
| ), | |
| ); |
Outdated
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.
Right now if sendEmail rejects, the invite row still exists, so retrying will be deduped away (unless there’s a separate resend flow). One option is to delete failed invite records so retries work.
| const failedEmails = inviteRecords | |
| const failedInvites = inviteRecords.filter( | |
| (_, i) => emailResults[i]?.status === "rejected", | |
| ); | |
| const failedEmails = failedInvites.map((r) => r.email); | |
| if (failedInvites.length > 0) { | |
| await db() | |
| .delete(organizationInvites) | |
| .where(inArray(organizationInvites.id, failedInvites.map((r) => r.id))); | |
| } |
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.
Since settings now live under /dashboard/settings/organization/*, this won’t invalidate the tab routes. Revalidating the layout should cover /billing, /preferences, etc.
| revalidatePath("/dashboard/settings/organization"); | |
| revalidatePath("/dashboard/settings/organization", "layout"); |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,125 @@ | ||||||||
| "use server"; | ||||||||
|
|
||||||||
| import { db } from "@cap/database"; | ||||||||
| import { getCurrentUser } from "@cap/database/auth/session"; | ||||||||
| import { | ||||||||
| organizationMembers, | ||||||||
| organizations, | ||||||||
| users, | ||||||||
| } from "@cap/database/schema"; | ||||||||
| import type { Organisation } from "@cap/web-domain"; | ||||||||
| import { and, eq } from "drizzle-orm"; | ||||||||
| import { revalidatePath } from "next/cache"; | ||||||||
| import { calculateProSeats } from "@/utils/organization"; | ||||||||
|
|
||||||||
| export async function toggleProSeat( | ||||||||
| memberId: string, | ||||||||
| organizationId: Organisation.OrganisationId, | ||||||||
| enable: boolean, | ||||||||
| ) { | ||||||||
| const user = await getCurrentUser(); | ||||||||
| if (!user) throw new Error("Unauthorized"); | ||||||||
|
|
||||||||
| const [organization] = await db() | ||||||||
| .select() | ||||||||
| .from(organizations) | ||||||||
| .where(eq(organizations.id, organizationId)) | ||||||||
| .limit(1); | ||||||||
|
|
||||||||
| if (!organization) { | ||||||||
| throw new Error("Organization not found"); | ||||||||
| } | ||||||||
|
|
||||||||
| if (organization.ownerId !== user.id) { | ||||||||
| throw new Error("Only the owner can manage Pro seats"); | ||||||||
| } | ||||||||
|
|
||||||||
| await db().transaction(async (tx) => { | ||||||||
| const [member] = await tx | ||||||||
| .select() | ||||||||
| .from(organizationMembers) | ||||||||
| .where( | ||||||||
| and( | ||||||||
| eq(organizationMembers.id, memberId), | ||||||||
| eq(organizationMembers.organizationId, organizationId), | ||||||||
| ), | ||||||||
| ) | ||||||||
| .for("update"); | ||||||||
|
|
||||||||
| if (!member) { | ||||||||
| throw new Error("Member not found"); | ||||||||
| } | ||||||||
|
|
||||||||
| if (member.userId === organization.ownerId) { | ||||||||
| throw new Error("Cannot toggle Pro seat for the organization owner"); | ||||||||
| } | ||||||||
|
|
||||||||
| if (enable) { | ||||||||
| const allMembers = await tx | ||||||||
| .select({ | ||||||||
| id: organizationMembers.id, | ||||||||
| hasProSeat: organizationMembers.hasProSeat, | ||||||||
| }) | ||||||||
| .from(organizationMembers) | ||||||||
| .where(eq(organizationMembers.organizationId, organizationId)); | ||||||||
|
||||||||
| .where(eq(organizationMembers.organizationId, organizationId)); | |
| .where(eq(organizationMembers.organizationId, organizationId)) | |
| .for("update"); |
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.
Race condition when checking available Pro seats
The allMembers query inside the transaction (line 58) is executed without a row lock, while the single member row is locked with .for("update") on line 47. Two concurrent requests can both read the same proSeatsRemaining > 0 value, pass the check, and each proceed to assign a seat — resulting in more seats consumed than the quota allows.
To prevent this, add a row lock to the allMembers query:
const allMembers = await tx
.select({
id: organizationMembers.id,
hasProSeat: organizationMembers.hasProSeat,
})
.from(organizationMembers)
.where(eq(organizationMembers.organizationId, organizationId))
.for("update");This serializes the seat availability check across concurrent requests.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/actions/organization/toggle-pro-seat.ts
Line: 57-84
Comment:
**Race condition when checking available Pro seats**
The `allMembers` query inside the transaction (line 58) is executed without a row lock, while the single `member` row is locked with `.for("update")` on line 47. Two concurrent requests can both read the same `proSeatsRemaining > 0` value, pass the check, and each proceed to assign a seat — resulting in more seats consumed than the quota allows.
To prevent this, add a row lock to the `allMembers` query:
```ts
const allMembers = await tx
.select({
id: organizationMembers.id,
hasProSeat: organizationMembers.hasProSeat,
})
.from(organizationMembers)
.where(eq(organizationMembers.organizationId, organizationId))
.for("update");
```
This serializes the seat availability check across concurrent requests.
How can I resolve this? If you propose a fix, please make it concise.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.
No idempotency guard when enable = true and seat is already assigned
If member.hasProSeat is already true and toggleProSeat is called with enable = true (possible via direct API call or duplicate UI events), the code still runs the full seat-count check. If the organization is at exactly its quota limit, proSeatsRemaining will be 0 and the call throws "No Pro seats remaining" — even though no new seat is actually being consumed.
Add an early exit before the if (enable) block to short-circuit when state is already desired:
if (member.hasProSeat === enable) {
// Already in the desired state, nothing to do
return { success: true };
}Insert this after the owner check (line 53) and before the if (enable) branch (line 57).
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/actions/organization/toggle-pro-seat.ts
Line: 57-89
Comment:
**No idempotency guard when `enable = true` and seat is already assigned**
If `member.hasProSeat` is already `true` and `toggleProSeat` is called with `enable = true` (possible via direct API call or duplicate UI events), the code still runs the full seat-count check. If the organization is at exactly its quota limit, `proSeatsRemaining` will be `0` and the call throws "No Pro seats remaining" — even though no new seat is actually being consumed.
Add an early exit before the `if (enable)` block to short-circuit when state is already desired:
```ts
if (member.hasProSeat === enable) {
// Already in the desired state, nothing to do
return { success: true };
}
```
Insert this after the owner check (line 53) and before the `if (enable)` branch (line 57).
How can I resolve this? If you propose a fix, please make it concise.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.
Stale thirdPartyStripeSubscriptionId when user holds Pro seats across multiple orgs
When revoking a Pro seat, the query on lines 108–117 checks for any other hasProSeat = true record for the same userId without filtering by organization. If the user belongs to Org A (subscription X) and Org B (subscription Y), and you revoke their seat in Org A, the query finds their Org B seat and skips the subscription ID update — leaving thirdPartyStripeSubscriptionId unchanged, still pointing to Org A's subscription X. The user retains a subscription ID that no longer applies in the current organization context.
Update the code to set the correct subscription when other Pro seats exist:
if (otherProSeats.length === 0) {
await tx
.update(users)
.set({ thirdPartyStripeSubscriptionId: null })
.where(eq(users.id, member.userId));
} else {
// Update to the subscription of a remaining Pro-seat org
const [remainingOrg] = await tx
.select({ stripeSubscriptionId: users.stripeSubscriptionId })
.from(organizationMembers)
.innerJoin(organizations, eq(organizationMembers.organizationId, organizations.id))
.innerJoin(users, eq(organizations.ownerId, users.id))
.where(
and(
eq(organizationMembers.userId, member.userId),
eq(organizationMembers.hasProSeat, true),
),
)
.limit(1);
if (remainingOrg?.stripeSubscriptionId) {
await tx
.update(users)
.set({ thirdPartyStripeSubscriptionId: remainingOrg.stripeSubscriptionId })
.where(eq(users.id, member.userId));
}
}Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/actions/organization/toggle-pro-seat.ts
Line: 102-125
Comment:
**Stale `thirdPartyStripeSubscriptionId` when user holds Pro seats across multiple orgs**
When revoking a Pro seat, the query on lines 108–117 checks for any other `hasProSeat = true` record for the same `userId` **without filtering by organization**. If the user belongs to Org A (subscription X) and Org B (subscription Y), and you revoke their seat in Org A, the query finds their Org B seat and skips the subscription ID update — leaving `thirdPartyStripeSubscriptionId` unchanged, still pointing to Org A's subscription X. The user retains a subscription ID that no longer applies in the current organization context.
Update the code to set the correct subscription when other Pro seats exist:
```ts
if (otherProSeats.length === 0) {
await tx
.update(users)
.set({ thirdPartyStripeSubscriptionId: null })
.where(eq(users.id, member.userId));
} else {
// Update to the subscription of a remaining Pro-seat org
const [remainingOrg] = await tx
.select({ stripeSubscriptionId: users.stripeSubscriptionId })
.from(organizationMembers)
.innerJoin(organizations, eq(organizationMembers.organizationId, organizations.id))
.innerJoin(users, eq(organizations.ownerId, users.id))
.where(
and(
eq(organizationMembers.userId, member.userId),
eq(organizationMembers.hasProSeat, true),
),
)
.limit(1);
if (remainingOrg?.stripeSubscriptionId) {
await tx
.update(users)
.set({ thirdPartyStripeSubscriptionId: remainingOrg.stripeSubscriptionId })
.where(eq(users.id, member.userId));
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
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.
stripeSubscriptionStatusis selected but unused here; removing it avoids unused-data drift.