-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
refactor: Team Creation Flow [CAL-2751] #12501
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
5 Ignored Deployments
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link to collect XP and win prizes! |
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 Fifty-one Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
Current Playwright Test Results Summary✅ 347 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 11/29/2023 04:25:13pm UTC) Run DetailsRunning Workflow PR Update on Github Actions Commit: 792d6a0 Started: 11/29/2023 04:16:49pm UTC
|
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Booking With Checkbox Group Question and Each Other Question Booking With Checkbox Group Question and Address Question Booking With Checkbox Group Question and Multi email Question Checkbox Group and Multi email not required
Retry 1 • Initial Attempt |
0% (0)0 / 292 runsfailed over last 7 days |
1.03% (3)3 / 292 runsflaked over last 7 days |
📄 apps/web/playwright/event-types.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Event Types tests user -- future can add multiple organizer address
Retry 1 • Initial Attempt |
0% (0)0 / 249 runsfailed over last 7 days |
14.86% (37)37 / 249 runsflaked over last 7 days |
📄 packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 2 Flakes
Top 1 Common Error Messages
|
2 Test Cases Affected |
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Popup Tests should be able to reschedule
Retry 1 • Initial Attempt |
6.03% (19)19 / 315 runsfailed over last 7 days |
90.79% (286)286 / 315 runsflaked over last 7 days |
Popup Tests Floating Button Popup Pro User - Configured in App with default setting of system theme should open embed iframe according to system theme when no theme is configured through Embed API
Retry 1 • Initial Attempt |
0.95% (3)3 / 317 runsfailed over last 7 days |
74.13% (235)235 / 317 runsflaked over last 7 days |
📄 apps/web/playwright/organization/organization-creation.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Organization should be able to create an organization and complete onboarding
Retry 2 • Retry 1 • Initial Attempt |
2.68% (8)8 / 299 runsfailed over last 7 days |
94.65% (283)283 / 299 runsflaked over last 7 days |
📄 apps/web/playwright/teams.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Teams - NonOrg Can create a private team
Retry 1 • Initial Attempt |
0% (0)0 / 317 runsfailed over last 7 days |
23.97% (76)76 / 317 runsflaked over last 7 days |
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 we're now creating a team after the checkout session. I think this should belong in it's own endpoint.
apps/web/pages/api/teams/create.ts
Outdated
// Let's query to ensure that the team metadata carried over from the checkout session. | ||
const parseCheckoutSessionMetadata = checkoutSessionMetadataSchema.safeParse(checkoutSession.metadata); | ||
|
||
if (!parseCheckoutSessionMetadata.success) { | ||
console.error( | ||
"Team metadata not found in checkout session", | ||
parseCheckoutSessionMetadata.error, | ||
checkoutSession.id | ||
); | ||
|
||
if (!checkoutSession.metadata.userId) { | ||
throw new HttpError({ | ||
statusCode: 400, | ||
message: "Can't publish team/org without userId", | ||
}); | ||
} | ||
} | ||
|
||
const checkoutSessionMetadata = parseCheckoutSessionMetadata.data ?? { | ||
teamName: checkoutSession?.metadata?.teamName ?? generateRandomString(), | ||
teamSlug: checkoutSession?.metadata?.teamSlug ?? generateRandomString(), | ||
userId: checkoutSession.metadata.userId, | ||
}; |
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.
Very unlikely that the metadata from creating the checkout session won't be carried over but I wanted to think about this edge case. Do we think this is worth covering?
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.
I'd rather let it fail than generating a random string.
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.
I was thinking about this as well. At this point the team is already paid for. The user is redirected back to the profile page, where they can see if they need to change the team name.
If we fail, then we would have to make another call to Stripe to cancel the subscription. I think that would be more friction than making the user change the team name.
apps/web/pages/api/teams/create.ts
Outdated
if (!checkoutSession.metadata.userId) { | ||
throw new HttpError({ | ||
statusCode: 400, | ||
message: "Can't publish team/org without userId", | ||
}); | ||
} |
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.
Absolutely need the userId of the owner to create the first member.
} else { | ||
publishTeamMutation.mutate({ teamId }); | ||
} | ||
router.push(`/settings/teams/${teamId}/profile`); |
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.
When finishing on this page, just navigate to the new team's profile page.
<div className="mb-8"> | ||
<Controller | ||
control={newTeamFormMethods.control} | ||
name="logo" | ||
render={({ field: { value } }) => ( | ||
<> | ||
<Label>{t("team_logo")}</Label> | ||
<div className="flex items-center"> | ||
<Avatar | ||
alt="" | ||
imageSrc={value} | ||
fallback={<Plus className="text-subtle h-6 w-6" />} | ||
size="lg" | ||
/> | ||
<div className="ms-4"> | ||
<ImageUploader | ||
target="avatar" | ||
id="avatar-upload" | ||
buttonMsg={t("update")} | ||
handleAvatarChange={(newAvatar: string) => { | ||
newTeamFormMethods.setValue("logo", newAvatar); | ||
createTeamMutation.reset(); | ||
}} | ||
imageSrc={value} | ||
/> | ||
</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.
Since users can leave this part of the form without the data being saved, uploading an avatar and losing it might be too much friction. Let's leave this part out until the user has paid for the team and is directed to the profile page.
/** | ||
* Used to generate a checkout session when trying to create a team | ||
*/ | ||
export const generateTeamCheckoutSession = async ({ |
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.
Created a new function to generate a checkout session for this scenario. I want to keep the previous function that was used purchaseTeamSubscription
for backwards compatibility for unpublished teams and orgs.
// If the user is not a part of an org, then make them pay before creating the team | ||
if (!isOrgChildTeam) { | ||
const checkoutSession = await generateCheckoutSession({ | ||
teamSlug: slug, | ||
teamName: name, | ||
userId: user.id, | ||
}); | ||
|
||
return checkoutSession; | ||
} |
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.
If this team is not created under an org, then direct user to checkout session.
CAL-2751 Team's pay-first invite-later flow
Basically we switch out Step 2 & 3. By step 3 we must already have an active subscription and team will be already published. As the user start adding/removing members we run the Step 1 Step 2 Step 3 |
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.
New team creation flow broke many e2e tests. Made some fixes to prevent flakyness.
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.
@zomars some tests are still failing
One more to go. If no one beats me to it I'll take another stab later today |
* Create new endpoint for creating a team * Generate a team checkout session * Create team navigate to checkout * Clean up * UI changes * Add comments * Fix * Type fix * Type fix * Type fix * Type fixes * Set telemetry * Import fix * Type fix * Update tests * Type fix * fix: e2e * fix: e2e * fix: e2e * fix: e2e * Update teams.e2e.ts * fix: e2e --------- Co-authored-by: Omar López <zomars@me.com>
* Create new endpoint for creating a team * Generate a team checkout session * Create team navigate to checkout * Clean up * UI changes * Add comments * Fix * Type fix * Type fix * Type fix * Type fixes * Set telemetry * Import fix * Type fix * Update tests * Type fix * fix: e2e * fix: e2e * fix: e2e * fix: e2e * Update teams.e2e.ts * fix: e2e --------- Co-authored-by: Omar López <zomars@me.com>
What does this PR do?
This PR changes the team creation flow. Currently you would have the option to create a team and then go through the Stripe checkout flow to publish the team.
This PR changes the flow so now, to create a team you need an active subscription before continuing.
Fixes # (issue)
https://www.loom.com/share/47e1a588a85d4e6fa47a989dee55282b
Requirement/Documentation
Type of change
How should this be tested?
Mandatory Tasks
Checklist