Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: speed up team upgrade #9902

Merged
merged 2 commits into from
Jul 1, 2024
Merged

fix: speed up team upgrade #9902

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50057a7
fix: speed up team upgrade
mattkrick Jun 28, 2024
56ec729
fix: don't update Org with subscription until payment succeeds
mattkrick Jul 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/client/modules/userDashboard/components/OrgBilling/BillingForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,18 @@ import {
} from '@stripe/react-stripe-js'
import {StripeElementChangeEvent} from '@stripe/stripe-js'
import React, {useState} from 'react'
import {commitLocalUpdate} from 'relay-runtime'
import {CreateStripeSubscriptionMutation$data} from '../../../../__generated__/CreateStripeSubscriptionMutation.graphql'
import Ellipsis from '../../../../components/Ellipsis/Ellipsis'
import PrimaryButton from '../../../../components/PrimaryButton'
import StyledError from '../../../../components/StyledError'
import useAtmosphere from '../../../../hooks/useAtmosphere'
import useMutationProps from '../../../../hooks/useMutationProps'
import CreateStripeSubscriptionMutation from '../../../../mutations/CreateStripeSubscriptionMutation'
import upgradeToTeamTierSuccessUpdater from '../../../../mutations/handlers/upgradeToTeamTierSuccessUpdater'
import {PALETTE} from '../../../../styles/paletteV3'
import SendClientSideEvent from '../../../../utils/SendClientSideEvent'
import createProxyRecord from '../../../../utils/relay/createProxyRecord'

const ButtonBlock = styled('div')({
display: 'flex',
Expand Down Expand Up @@ -131,6 +134,11 @@ const BillingForm = (props: Props) => {
setIsLoading(false)
return
}
commitLocalUpdate(atmosphere, (store) => {
const payload = createProxyRecord(store, 'payload', {})
payload.setLinkedRecord(store.get(orgId)!, 'organization')
upgradeToTeamTierSuccessUpdater(payload)
})
onCompleted()
}

Expand Down
13 changes: 4 additions & 9 deletions packages/client/mutations/fragments/UpgradeToTeamTierFrag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,13 @@ graphql`
periodStart
updatedAt
lockedAt
teams {
isPaid
tier
}
}
meetings {
showConversionModal
}
}
`

graphql`
fragment UpgradeToTeamTierFrag_team on UpgradeToTeamTierSuccess {
teams {
isPaid
tier
}
}
`
3 changes: 0 additions & 3 deletions packages/client/subscriptions/TeamSubscription.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,6 @@ const subscription = graphql`
OldUpgradeToTeamTierPayload {
...OldUpgradeToTeamTierMutation_team @relay(mask: false)
}
UpgradeToTeamTierSuccess {
...UpgradeToTeamTierFrag_team @relay(mask: false)
}
UpdateIntegrationProviderSuccess {
...UpdateIntegrationProviderMutation_team @relay(mask: false)
}
Expand Down
2 changes: 1 addition & 1 deletion packages/server/graphql/mutations/helpers/oldUpgradeToTeamTier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const oldUpgradeToTeamTier = async (
const manager = getStripeManager()
const customer = stripeId
? await manager.updatePayment(stripeId, source)
: await manager.createCustomer(orgId, email, source)
: await manager.createCustomer(orgId, email, undefined, source)

let subscriptionFields = {}
if (!stripeSubscriptionId) {
Expand Down
4 changes: 0 additions & 4 deletions packages/server/graphql/private/mutations/upgradeToTeamTier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,6 @@ const upgradeToTeamTier: MutationResolvers['upgradeToTeamTier'] = async (
const data = {orgId, teamIds, meetingIds}
publish(SubscriptionChannel.ORGANIZATION, orgId, 'UpgradeToTeamTierSuccess', data, subOptions)

teamIds.forEach((teamId) => {
const teamData = {orgId, teamIds: [teamId]}
publish(SubscriptionChannel.TEAM, teamId, 'UpgradeToTeamTierSuccess', teamData, subOptions)
})
return data
}

Expand Down
20 changes: 12 additions & 8 deletions packages/server/graphql/public/mutations/createStripeSubscription.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,18 @@ const createStripeSubscription: MutationResolvers['createStripeSubscription'] =
return standardError(new Error('Organization already has a subscription'), {userId: viewerId})
}
const {email} = viewer
const customer = stripeId
? await manager.retrieveCustomer(stripeId)
: await manager.createCustomer(orgId, email)
const {id: customerId} = customer
const res = await manager.attachPaymentToCustomer(customerId, paymentMethodId)
if (res instanceof Error) return standardError(res, {userId: viewerId})
// wait until the payment is attached to the customer before updating the default payment method
await manager.updateDefaultPaymentMethod(customerId, paymentMethodId)
let customer: Stripe.Response<Stripe.Customer | Stripe.DeletedCustomer>
if (stripeId) {
customer = await manager.retrieveCustomer(stripeId)
const {id: customerId} = customer
const res = await manager.attachPaymentToCustomer(customerId, paymentMethodId)
if (res instanceof Error) return standardError(res, {userId: viewerId})
// wait until the payment is attached to the customer before updating the default payment method
await manager.updateDefaultPaymentMethod(customerId, paymentMethodId)
Copy link
Contributor

Choose a reason for hiding this comment

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

For the edge case where a user is upgrading for a second time, we could use a Promise.all to speed things up a bit here? Something like:

  const [attachPaymentResult, updateDefaultPaymentResult] = await Promise.all([
    manager.attachPaymentToCustomer(customerId, paymentMethodId),
    manager.updateDefaultPaymentMethod(customerId, paymentMethodId)
  ])
  if (attachPaymentResult instanceof Error) return standardError(attachPaymentResult, {userId: viewerId})

Copy link
Member Author

Choose a reason for hiding this comment

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

i tried this...
nope, gotta attach first. ah well, not too bad since re-upgrade is such an edge case!

} else {
customer = await manager.createCustomer(orgId, email, paymentMethodId)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

And we could remove the await on line 65 when updating rethinkdb?

Copy link
Member Author

Choose a reason for hiding this comment

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

i was thinking about that! but since our DB is colocated it's usually only a couple ms so I figured probably better to be safe than sorry in case the write failed for some reason.

Copy link
Member Author

Choose a reason for hiding this comment

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

On second look, we should completely get rid of the write to the DB here!
We should only update our DB when the payment succeeds, otherwise if we update the stripeSubscriptionId & then the 3D payment fails, the org gets stuck in a weird state where it can't upgrade because it has a subscription, but it can't downgrade because it's still on the starter tier 😬

const subscription = await manager.createTeamSubscription(customer.id, orgId, orgUsersCount)

const latestInvoice = subscription.latest_invoice as Stripe.Invoice
Expand Down
2 changes: 1 addition & 1 deletion packages/server/utils/defaultTier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export const defaultTier = process.env.IS_ENTERPRISE ? 'enterprise' : 'starter'
export const defaultTier = process.env.IS_ENTERPRISE === 'true' ? 'enterprise' : 'starter'
13 changes: 12 additions & 1 deletion packages/server/utils/stripe/StripeManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,21 @@ export default class StripeManager {
}
}

async createCustomer(orgId: string, email: string, source?: string) {
async createCustomer(
orgId: string,
email: string,
paymentMethodId?: string | undefined,
source?: string
) {
return this.stripe.customers.create({
email,
source,
payment_method: paymentMethodId,
invoice_settings: paymentMethodId
? {
default_payment_method: paymentMethodId
}
: undefined,
metadata: {
orgId
}
Expand Down
Loading