-
Notifications
You must be signed in to change notification settings - Fork 911
fix: User onboarding and organization setup logic #1215
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 all commits
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 |
---|---|---|
|
@@ -6,7 +6,7 @@ import { | |
organizationMembers, | ||
users, | ||
} from "@cap/database/schema"; | ||
import { eq } from "drizzle-orm"; | ||
import { and, eq } from "drizzle-orm"; | ||
import { type NextRequest, NextResponse } from "next/server"; | ||
|
||
export async function POST(request: NextRequest) { | ||
|
@@ -46,18 +46,40 @@ export async function POST(request: NextRequest) { | |
); | ||
} | ||
|
||
await db().insert(organizationMembers).values({ | ||
id: nanoId(), | ||
organizationId: invite.organizationId, | ||
userId: user.id, | ||
role: invite.role, | ||
}); | ||
const [existingMembership] = await db() | ||
.select({ id: organizationMembers.id }) | ||
.from(organizationMembers) | ||
.where( | ||
and( | ||
eq(organizationMembers.organizationId, invite.organizationId), | ||
eq(organizationMembers.userId, user.id), | ||
), | ||
) | ||
.limit(1); | ||
|
||
if (!existingMembership) { | ||
await db().insert(organizationMembers).values({ | ||
id: nanoId(), | ||
organizationId: invite.organizationId, | ||
userId: user.id, | ||
role: invite.role, | ||
}); | ||
} | ||
Comment on lines
+49
to
+67
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. Eliminate TOCTOU duplicate membership risk; make insert idempotent and atomic. The check‑then‑insert can race and create duplicate rows. Wrap accept flow in a DB transaction and enforce a unique constraint on (organizationId, userId) with an upsert.
Outside this file (packages/database/schema.ts): - userIdOrganizationIdIndex: index("user_id_organization_id_idx").on(
+ userIdOrganizationIdIndex: uniqueIndex("user_id_organization_id_uidx").on(
table.userId,
table.organizationId,
),
Within this file: - const [existingMembership] = await db()
- .select({ id: organizationMembers.id })
- .from(organizationMembers)
- .where(
- and(
- eq(organizationMembers.organizationId, invite.organizationId),
- eq(organizationMembers.userId, user.id),
- ),
- )
- .limit(1);
-
- if (!existingMembership) {
- await db().insert(organizationMembers).values({
- id: nanoId(),
- organizationId: invite.organizationId,
- userId: user.id,
- role: invite.role,
- });
- }
+ // within a transaction `tx`, or use db().$primary if not transacting
+ await db()
+ .insert(organizationMembers)
+ .values({
+ id: nanoId(),
+ organizationId: invite.organizationId,
+ userId: user.id,
+ role: invite.role,
+ })
+ .onDuplicateKeyUpdate({ set: { role: invite.role } }); Also consider running the entire accept path in
🤖 Prompt for AI Agents
|
||
|
||
const onboardingSteps = { | ||
...(user.onboardingSteps ?? {}), | ||
organizationSetup: true, | ||
customDomain: true, | ||
inviteTeam: true, | ||
}; | ||
|
||
await db() | ||
.update(users) | ||
.set({ | ||
thirdPartyStripeSubscriptionId: organizationOwner.stripeSubscriptionId, | ||
activeOrganizationId: invite.organizationId, | ||
defaultOrgId: invite.organizationId, | ||
onboardingSteps, | ||
}) | ||
.where(eq(users.id, user.id)); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,9 @@ export class UsersOnboarding extends Effect.Service<UsersOnboarding>()( | |
.where(Dz.eq(Db.users.id, currentUser.id)), | ||
); | ||
|
||
const firstName = data.firstName.trim(); | ||
const lastName = data.lastName?.trim() ?? ""; | ||
|
||
yield* db.use((db) => | ||
db | ||
.update(Db.users) | ||
|
@@ -36,11 +39,31 @@ export class UsersOnboarding extends Effect.Service<UsersOnboarding>()( | |
...user.onboardingSteps, | ||
welcome: true, | ||
}, | ||
name: data.firstName, | ||
lastName: data.lastName || "", | ||
name: firstName, | ||
lastName, | ||
}) | ||
.where(Dz.eq(Db.users.id, currentUser.id)), | ||
); | ||
|
||
const activeOrgId = user.activeOrganizationId ?? user.defaultOrgId; | ||
if (activeOrgId && firstName.length > 0) { | ||
const [organization] = yield* db.use((db) => | ||
db | ||
.select({ name: Db.organizations.name }) | ||
.from(Db.organizations) | ||
.where(Dz.eq(Db.organizations.id, activeOrgId)), | ||
); | ||
|
||
if (organization?.name === "My Organization") { | ||
const personalizedName = `${firstName}'s Organization`; | ||
yield* db.use((db) => | ||
db | ||
.update(Db.organizations) | ||
.set({ name: personalizedName }) | ||
.where(Dz.eq(Db.organizations.id, activeOrgId)), | ||
); | ||
} | ||
} | ||
}), | ||
|
||
organizationSetup: Effect.fn("Onboarding.organizationSetup")( | ||
|
@@ -61,36 +84,83 @@ export class UsersOnboarding extends Effect.Service<UsersOnboarding>()( | |
.where(Dz.eq(Db.users.id, currentUser.id)), | ||
); | ||
|
||
const organizationId = Organisation.OrganisationId.make(nanoId()); | ||
const organizationName = | ||
data.organizationName.trim() || data.organizationName; | ||
let organizationId = | ||
user.activeOrganizationId ?? user.defaultOrgId ?? null; | ||
|
||
yield* db.use((db) => | ||
db.transaction(async (tx) => { | ||
await tx.insert(Db.organizations).values({ | ||
id: organizationId, | ||
ownerId: currentUser.id, | ||
name: data.organizationName, | ||
}); | ||
|
||
await tx.insert(Db.organizationMembers).values({ | ||
id: nanoId(), | ||
userId: currentUser.id, | ||
role: "owner", | ||
organizationId, | ||
}); | ||
let resolvedOrgId = organizationId; | ||
|
||
if (resolvedOrgId) { | ||
const [existingOrg] = await tx | ||
.select({ id: Db.organizations.id }) | ||
.from(Db.organizations) | ||
.where(Dz.eq(Db.organizations.id, resolvedOrgId)); | ||
|
||
if (existingOrg) { | ||
await tx | ||
.update(Db.organizations) | ||
.set({ name: organizationName }) | ||
.where(Dz.eq(Db.organizations.id, resolvedOrgId)); | ||
} else { | ||
resolvedOrgId = Organisation.OrganisationId.make(nanoId()); | ||
|
||
await tx.insert(Db.organizations).values({ | ||
id: resolvedOrgId, | ||
ownerId: currentUser.id, | ||
name: organizationName, | ||
}); | ||
|
||
await tx.insert(Db.organizationMembers).values({ | ||
id: nanoId(), | ||
organizationId: resolvedOrgId, | ||
userId: currentUser.id, | ||
role: "owner", | ||
}); | ||
} | ||
} else { | ||
resolvedOrgId = Organisation.OrganisationId.make(nanoId()); | ||
|
||
await tx.insert(Db.organizations).values({ | ||
id: resolvedOrgId, | ||
ownerId: currentUser.id, | ||
name: organizationName, | ||
}); | ||
|
||
await tx.insert(Db.organizationMembers).values({ | ||
id: nanoId(), | ||
organizationId: resolvedOrgId, | ||
userId: currentUser.id, | ||
role: "owner", | ||
}); | ||
} | ||
|
||
await tx | ||
.update(Db.users) | ||
.set({ | ||
activeOrganizationId: organizationId, | ||
activeOrganizationId: resolvedOrgId, | ||
defaultOrgId: resolvedOrgId, | ||
onboardingSteps: { | ||
...user.onboardingSteps, | ||
organizationSetup: true, | ||
}, | ||
}) | ||
.where(Dz.eq(Db.users.id, currentUser.id)); | ||
|
||
organizationId = resolvedOrgId; | ||
}), | ||
); | ||
|
||
if (!organizationId) { | ||
throw new Error( | ||
"Failed to resolve organization during onboarding", | ||
); | ||
} | ||
|
||
const finalOrganizationId = organizationId; | ||
|
||
if (data.organizationIcon) { | ||
const organizationIcon = data.organizationIcon; | ||
const uploadEffect = Effect.gen(function* () { | ||
|
@@ -104,7 +174,7 @@ export class UsersOnboarding extends Effect.Service<UsersOnboarding>()( | |
const fileExtension = allowedExt.get(contentType); | ||
if (!fileExtension) | ||
throw new Error("Unsupported icon content type"); | ||
const fileKey = `organizations/${organizationId}/icon-${Date.now()}.${fileExtension}`; | ||
const fileKey = `organizations/${finalOrganizationId}/icon-${Date.now()}.${fileExtension}`; | ||
|
||
const [bucket] = yield* s3Buckets.getBucketAccess( | ||
Option.none(), | ||
|
@@ -117,7 +187,7 @@ export class UsersOnboarding extends Effect.Service<UsersOnboarding>()( | |
db | ||
.update(Db.organizations) | ||
.set({ iconUrl }) | ||
.where(Dz.eq(Db.organizations.id, organizationId)), | ||
.where(Dz.eq(Db.organizations.id, finalOrganizationId)), | ||
); | ||
Comment on lines
+190
to
191
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. Don’t persist pre‑signed URLs; they expire. Store the object key and derive URLs on demand. Saving a signed URL will break later. Persist the key (e.g., Example:
🤖 Prompt for AI Agents
|
||
}).pipe( | ||
Effect.catchAll((error) => | ||
|
@@ -128,7 +198,7 @@ export class UsersOnboarding extends Effect.Service<UsersOnboarding>()( | |
yield* uploadEffect; | ||
} | ||
|
||
return { organizationId }; | ||
return { organizationId: finalOrganizationId }; | ||
}, | ||
), | ||
|
||
|
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.
🛠️ Refactor suggestion | 🟠 Major
Fix params typing and step extraction (string[] vs Promise/union).
Next.js passes params synchronously; catch‑all
[...steps]
yieldsstring[]
. Current type (Promise<{ steps: union }>
), then indexingsteps[0]
, is wrong and loses type safety.Apply:
Also applies to: 20-20
🤖 Prompt for AI Agents