-
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 26 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,74 @@ | ||
| "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, | ||
| }) | ||
| .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, | ||
| currentPeriodEnd: subscription.current_period_end, | ||
| currency: price.currency, | ||
| }; | ||
| } | ||
| 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,136 @@ 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 = Array.from( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| new Set( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| invitedEmails | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map((email) => email.trim().toLowerCase()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter((email) => emailRegex.test(email)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (validEmails.length === 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { success: true, failedEmails: [] as string[] }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const inviteRecords = await db().transaction(async (tx) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [existingInvites, existingMembers] = await Promise.all([ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tx | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .select({ invitedEmail: organizationInvites.invitedEmail }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .from(organizationInvites) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .where( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| and( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eq(organizationInvites.organizationId, organizationId), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inArray(organizationInvites.invitedEmail, validEmails), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tx | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .select({ email: users.email }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .from(organizationMembers) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .innerJoin(users, eq(organizationMembers.userId, users.id)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .where( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| and( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eq(organizationMembers.organizationId, organizationId), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inArray(users.email, validEmails), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const existingInviteEmails = new Set( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| existingInvites.map((i) => i.invitedEmail.toLowerCase()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const existingMemberEmails = new Set( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| existingMembers.map((m) => m.email.toLowerCase()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const emailsToInvite = validEmails.filter( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (email) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| !existingInviteEmails.has(email) && !existingMemberEmails.has(email), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const records = emailsToInvite.map((email) => ({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: nanoId(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| email, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (records.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await tx.insert(organizationInvites).values( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| records.map((r) => ({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: r.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| organizationId: organizationId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| invitedEmail: r.email, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| invitedByUserId: user.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| role: "member" as const, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| })), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return records; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const emailResults = await Promise.allSettled( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inviteRecords.map((record) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const inviteUrl = `${serverEnv().WEB_URL}/invite/${record.id}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return sendEmail({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| email: record.email, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subject: `Invitation to join ${organization.name} on Cap`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| react: OrganizationInvite({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| email: record.email, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url: inviteUrl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| organizationName: organization.name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const failedInvites = inviteRecords.filter( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (_, i) => emailResults[i]?.status === "rejected", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const failedEmails = failedInvites.map((r) => r.email); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const email of validEmails) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const inviteId = nanoId(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await db().insert(organizationInvites).values({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: inviteId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| organizationId: organizationId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| invitedEmail: email.trim(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| invitedByUserId: user.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| role: "member", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Send invitation email | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const inviteUrl = `${serverEnv().WEB_URL}/invite/${inviteId}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await sendEmail({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| email: email.trim(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subject: `Invitation to join ${organization[0].name} on Cap`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| react: OrganizationInvite({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| email: email.trim(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url: inviteUrl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| organizationName: organization[0].name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (failedInvites.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await db() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .delete(organizationInvites) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .where( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inArray( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| organizationInvites.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| failedInvites.map((r) => r.id), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (cleanupError) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Failed to clean up invite records after email delivery failure:", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| failedInviteIds: failedInvites.map((r) => r.id), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| failedEmails, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| error: cleanupError, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+136
to
156
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unhandled cleanup failure can orphan invite records When email delivery fails, the subsequent Wrap the cleanup in a try/catch to detect and log the failure:
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/web/actions/organization/send-invites.ts
Line: 136-145
Comment:
**Unhandled cleanup failure can orphan invite records**
When email delivery fails, the subsequent `db().delete()` cleanup (lines 136–145) runs outside the transaction with no error handling. If this delete fails due to a transient DB error, the undelivered invite records will persist indefinitely — creating a data integrity gap. A user who somehow obtains the invite ID (e.g., from a server log) could still accept the invite without having been notified.
Wrap the cleanup in a try/catch to detect and log the failure:
```suggestion
if (failedInvites.length > 0) {
try {
await db()
.delete(organizationInvites)
.where(
inArray(
organizationInvites.id,
failedInvites.map((r) => r.id),
),
);
} catch (cleanupError) {
console.error(
"CRITICAL: Failed to clean up undelivered invite records:",
failedInvites.map((r) => r.id),
cleanupError,
);
}
}
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| revalidatePath("/dashboard/settings/organization"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since settings now live under
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { success: true }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { success: true, failedEmails }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| "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 (member.hasProSeat === enable) { | ||
| return { success: true }; | ||
| } | ||
|
|
||
| if (enable) { | ||
| const allMembers = await tx | ||
| .select({ | ||
| id: organizationMembers.id, | ||
| hasProSeat: organizationMembers.hasProSeat, | ||
| }) | ||
| .from(organizationMembers) | ||
| .where(eq(organizationMembers.organizationId, organizationId)) | ||
| .for("update"); | ||
|
|
||
| const [owner] = await tx | ||
| .select({ | ||
| inviteQuota: users.inviteQuota, | ||
| stripeSubscriptionId: users.stripeSubscriptionId, | ||
| }) | ||
| .from(users) | ||
| .where(eq(users.id, organization.ownerId)) | ||
| .limit(1); | ||
|
|
||
| const { proSeatsRemaining } = calculateProSeats({ | ||
| inviteQuota: owner?.inviteQuota ?? 1, | ||
| members: allMembers, | ||
| }); | ||
|
|
||
| if (proSeatsRemaining <= 0) { | ||
| throw new Error( | ||
| "No Pro seats remaining. Purchase more seats to continue.", | ||
| ); | ||
| } | ||
|
Comment on lines
+61
to
+89
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Race condition when checking available Pro seats The To prevent this, add a row lock to the 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 AIThis 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. |
||
|
|
||
| await tx | ||
| .update(organizationMembers) | ||
| .set({ hasProSeat: true }) | ||
| .where(eq(organizationMembers.id, memberId)); | ||
|
Comment on lines
+61
to
+94
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No idempotency guard when If Add an early exit before the 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 Prompt To Fix With AIThis 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. |
||
|
|
||
| if (owner?.stripeSubscriptionId) { | ||
| await tx | ||
| .update(users) | ||
| .set({ thirdPartyStripeSubscriptionId: owner.stripeSubscriptionId }) | ||
| .where(eq(users.id, member.userId)); | ||
| } | ||
| } else { | ||
| await tx | ||
| .update(organizationMembers) | ||
| .set({ hasProSeat: false }) | ||
| .where(eq(organizationMembers.id, memberId)); | ||
|
|
||
| const otherProSeats = await tx | ||
| .select({ id: organizationMembers.id }) | ||
| .from(organizationMembers) | ||
| .where( | ||
| and( | ||
| eq(organizationMembers.userId, member.userId), | ||
| eq(organizationMembers.hasProSeat, true), | ||
| ), | ||
| ) | ||
| .limit(1); | ||
|
|
||
| if (otherProSeats.length === 0) { | ||
| await tx | ||
| .update(users) | ||
| .set({ thirdPartyStripeSubscriptionId: null }) | ||
| .where(eq(users.id, member.userId)); | ||
| } | ||
| } | ||
|
Comment on lines
+102
to
+155
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stale When revoking a Pro seat, the query on lines 108–117 checks for any other 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 AIThis 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. |
||
| }); | ||
|
|
||
| revalidatePath("/dashboard/settings/organization"); | ||
| return { success: true }; | ||
| } | ||
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.