Skip to content

feat: Organization onboarding improvement - Payment before creation of org but onboarding doesn't require payment#18990

Merged
hariombalhara merged 60 commits intomainfrom
feat/org-payment-before-creation
Mar 13, 2025
Merged

feat: Organization onboarding improvement - Payment before creation of org but onboarding doesn't require payment#18990
hariombalhara merged 60 commits intomainfrom
feat/org-payment-before-creation

Conversation

@sean-brydon
Copy link
Copy Markdown
Member

@sean-brydon sean-brydon commented Jan 29, 2025

What does this PR do?

Self Serve Flow - Loom

  • Org is now created only through stripe webhook and thus code has been adapted to be idempotent in case of retries from Stripe when failure happens.
  • We store the complete progress of the onboarding in OrganizationOnboarding table including any error that last happened and the end of lifecycle with isComplete set to true.
  • Admin can configure a custom price for a customer(identified by email) and handover the onboarding to the customer(through a handover onboarding link). In this way, the customer still setups the onboarding himself but pays a custom price.
  • Admin Doing organization onboarding on behalf of an email that doesn't exist in our system, is temporarily disabled. [Can be enabled in the future if needed]
  • Earlier Organization onboarding updated DB step by step but now in one go after the payment, we create everything - Domain Creation, Org Setup, Teams Creation, Teams Migration, Migrating Teams' members migration, member invitations. So, we have been extra careful with the logic to ensure errors don;t occur and if occur they are retried by webhook and also recorded in OrganizationOnboarding table.

Deprecations/Removals:

  • NEXT_PUBLIC_ORGANIZATIONS_SELF_SERVE_PRICE env variable is removed and user must set NEXT_PUBLIC_ORGANIZATIONS_SELF_SERVE_PRICE_NEW with the difference that it doesn't have 00 in the end (37 instead of 3700 now). Reason was that 00 is a stripe specific thing and also because we store the price in DB with OrganizationOnboarding record and it doesn't make sense for it to be 3700 when infact it is 37.

Deployment Plan

  • Need Stripe Product ID in Env variable
  • Set NEXT_PUBLIC_ORGANIZATIONS_SELF_SERVE_PRICE_NEW to 37
  • Add invoice.paid event to webhook in Stripe dashboard if not alreadythere

Detailed TODO:

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.
  • If there is still conflict in creating the organization due to slug(because team with conflicting slug wasn't migrated, rename the team)
  • Show the pricing to user set by admin
  • Make sure that a slug that is taken by another organization isn't able to be checkout
  • PLATFORM: We need to create the organization without requiring payment first.
  • BUG: logo and bio not bieng saved to Onboarding/Organization
  • Verify if we have the logic to allow creating an organization with owner as the email for which the user doesn't have an account already.
  • Make sure that we pass creationSource to createTeamsMutation.mutate
  • Refreshing on onboarding once it is complete should redirect to Organization profile page in dashboard.
  • Test admin creation flow.
  • Even though the payment succeeds, the organization setup might fail. Possible failure reasons are below. We should record the failure in organizationOnboarding table.
    • Organization slug conflict. Taken by some other organization. As soon as the onboarding is complete, the organization slug is reserved.
    • Domain setup - Vercel/Cloudflare domain creation might fail.
    • There could be soft errors like - certain users that couldn't be invited. Possibly some teams that couldn't be migrated
  • Failed payment handling
  • auto-accept invite doesn't seem to be working
  • createTeams.handler No subscriptionId found in team metadata when moving team members via onboarding
  • We need to reuse some of trpc/server/routers/viewer/organizations/create.handler.ts logic to start creating subdomains which isn't happening in _invoice.paid.org.ts at the moment.
  • Organization slug conflict in onboarding
    • There could be many different users that are checking out organization with same slug. The first one to checkout should win.
  • We should flush onboarding store(useOnboardingStore) after the organizationOnboarding DB entry is created. Because persisted data in localStorage could accidentally get reused by admin for creating other organizations.
  • checkout flow
    • checkout button should be disabled till createWithPaymentIntent request completes.
    • checkout button should show a spinner when loading.
    • Errors from createWithPaymentIntent aren't being shown in UI.
    • "You already have a pending organization" error must not be shown if it is the same organization that is being created.
    • Handle payment failure. Maybe create a new page for failure like success page.
  • Also we need to move the core logic from _invoice.paid.org.ts to somewhere else and just have import in there.
  • Webhook could be retried by Stripe and thus we need to handle duplicates/existing records and be able to fix the data in DB.
  • Schema
    • OrganizationOnboarding schema should be unaware of the Payment app being used. We might need to move to another payment service as well.
  • Cleanup
    • We don't need team.metadata.requestedSlug for organizations as organization is created only after confirmation.

Followup Improvements:

  • If due to the number of people being invited during the onboarding if the seats increase, we should show that as a toast in the last onboarding step
  • Onboarding handover URL should take the user to the first step(intead of second step) where he could review the org and price details first
  • Allow admin to change the price of an existing onboarding. This is important because the admin might need to change the price of the onboarding after it is created and there can only be one onboarding for an orgOwner email.
  • Logo upload isn't working form onboarding(Existing bug)
  • Send telemetry event for org creation,
  • What if renewal of plan fails? Need to handle that.
  • OrganizationOnboarding schema should be created through intentToCreateOrgHandler and rest of the flow should just update it. This could be important because we do a lot of validation in intentToCreateOrgHandler and without that we shouldn't allow organization to be created and thus no-onboarding should happen.
  • Consider showing what each step in onboarding does, like some indication in front of all steps viewable at once.
  • Allow going back to previous steps.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 29, 2025

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "Feat/org payment before creation". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@github-actions github-actions bot added ❗️ migrations contains migration files ❗️ .env changes contains changes to env variables labels Jan 29, 2025
@keithwillcode keithwillcode added consumer core area: core, team members only labels Jan 29, 2025
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Feb 10, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ zomars
❌ Hariom Balhara


Hariom Balhara seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@vercel
Copy link
Copy Markdown

vercel bot commented Feb 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
cal-com-ui-playground ❌ Failed (Inspect) Mar 13, 2025 3:52am
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Mar 13, 2025 3:52am
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Mar 13, 2025 3:52am

@hariombalhara hariombalhara force-pushed the feat/org-payment-before-creation branch from e0d1024 to 211aab9 Compare February 20, 2025 05:01
@zomars
Copy link
Copy Markdown
Contributor

zomars commented Feb 27, 2025

Taking a look

Copy link
Copy Markdown
Contributor

@zomars zomars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still reviewing but looking good so far. Will continue tomorrow

Comment thread apps/web/modules/settings/organizations/new/flow.mermaid
});

test("Admin should be able to create an org where the owner doesn't exist yet", async ({
test.skip("Admin should be able to create an org where the owner doesn't exist yet", async ({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this test skip meant to go to main?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this flow isn't supported now. If and when it really makes sense to support it, we need to fix the core and enable the test

zomars
zomars previously requested changes Mar 6, 2025
Copy link
Copy Markdown
Contributor

@zomars zomars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix conflicts please 🙏🏽

@hariombalhara
Copy link
Copy Markdown
Member

@zomars Done

Comment thread PR_TODO.md Outdated
@socket-security
Copy link
Copy Markdown

socket-security bot commented Mar 7, 2025

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@types/luxon@3.4.2 None 0 120 kB types
npm/qs-stringify@1.2.1 None 0 5.65 kB goto-bus-stop

View full report↗︎

zomars
zomars previously approved these changes Mar 13, 2025
Copy link
Copy Markdown
Contributor

@zomars zomars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work again. I have no objections.

@zomars
Copy link
Copy Markdown
Contributor

zomars commented Mar 13, 2025

Seems like we have some new conflcits tho.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

consumer core area: core, team members only ❗️ .env changes contains changes to env variables ✨ feature New feature or request ❗️ migrations contains migration files organizations area: organizations, orgs ready-for-e2e

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants