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.

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

feat: Add ACH microdeposits handling #3693

Merged
merged 7 commits into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import config from 'config'

import { ThemeContextProvider } from 'shared/ThemeContext'

import { Location } from 'history'
import { UnverifiedPaymentMethodSchema } from 'services/account'
import { z } from 'zod'
import PlanPage from './PlanPage'

vi.mock('config')
Expand Down Expand Up @@ -40,10 +43,10 @@ const queryClientV5 = new QueryClientV5({
defaultOptions: { queries: { retry: false } },
})

let testLocation
let testLocation: Location<unknown>
const wrapper =
(initialEntries = '') =>
({ children }) => (
({ children }: { children: React.ReactNode }) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This should be React.FC

<QueryClientProviderV5 client={queryClientV5}>
<QueryClientProvider client={queryClient}>
<ThemeContextProvider>
Expand Down Expand Up @@ -79,7 +82,13 @@ afterAll(() => {

describe('PlanPage', () => {
function setup(
{ owner, isSelfHosted = false } = {
{
owner,
isSelfHosted = false,
unverifiedPaymentMethods = [] as z.infer<
typeof UnverifiedPaymentMethodSchema
>[],
} = {
owner: {
username: 'codecov',
isCurrentUserPartOfOrg: true,
Expand All @@ -92,6 +101,17 @@ describe('PlanPage', () => {
server.use(
graphql.query('PlanPageData', () => {
return HttpResponse.json({ data: { owner } })
}),
graphql.query('UnverifiedPaymentMethods', () => {
return HttpResponse.json({
data: {
owner: {
billing: {
unverifiedPaymentMethods,
},
},
},
})
})
)
}
Expand All @@ -102,7 +122,7 @@ describe('PlanPage', () => {
owner: {
username: 'codecov',
isCurrentUserPartOfOrg: false,
numberOfUploads: null,
numberOfUploads: 0,
},
})
})
Expand All @@ -120,7 +140,7 @@ describe('PlanPage', () => {
owner: {
username: 'codecov',
isCurrentUserPartOfOrg: false,
numberOfUploads: null,
numberOfUploads: 0,
},
})
})
Expand Down Expand Up @@ -149,6 +169,34 @@ describe('PlanPage', () => {
const tabs = await screen.findByText(/Tabs/)
expect(tabs).toBeInTheDocument()
})

describe('when there are unverified payment methods', () => {
beforeEach(() => {
setup({
owner: {
username: 'codecov',
isCurrentUserPartOfOrg: true,
numberOfUploads: 30,
},
unverifiedPaymentMethods: [
{
paymentMethodId: 'pm_123',
hostedVerificationUrl: 'https://verify.stripe.com',
},
],
})
})

it('renders unverified payment method alert', async () => {
render(<PlanPage />, { wrapper: wrapper('/plan/gh/codecov') })

const alert = await screen.findByText(/Verify Your New Payment Method/)
expect(alert).toBeInTheDocument()

const link = screen.getByText('Click here')
expect(link).toHaveAttribute('href', 'https://verify.stripe.com')
})
})
})

describe('testing routes', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import config from 'config'

import { SentryRoute } from 'sentry'

import { useUnverifiedPaymentMethods } from 'services/account/useUnverifiedPaymentMethods'
import { Provider } from 'shared/api/helpers'
import { Theme, useThemeContext } from 'shared/ThemeContext'
import A from 'ui/A'
import { Alert } from 'ui/Alert'
import LoadingLogo from 'ui/LoadingLogo'

import { PlanProvider } from './context'
Expand All @@ -35,11 +39,21 @@ const Loader = () => (
</div>
)

interface URLParams {
owner: string
provider: Provider
}

function PlanPage() {
const { owner, provider } = useParams()
const { owner, provider } = useParams<URLParams>()
const { data: ownerData } = useSuspenseQueryV5(
PlanPageDataQueryOpts({ owner, provider })
)
const { data: unverifiedPaymentMethods } = useUnverifiedPaymentMethods({
provider,
owner,
})

const { theme } = useThemeContext()
const isDarkMode = theme !== Theme.LIGHT

Expand All @@ -61,6 +75,11 @@ function PlanPage() {
>
<PlanProvider>
<PlanBreadcrumb />
{unverifiedPaymentMethods?.length ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe .length > 0 rather than the property existing, but this is also falsey for the 0 case I believe.... maybe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think as is works, but I'll make it more explicit with the length check so it we don't have to double think

<UnverifiedPaymentMethodAlert
url={unverifiedPaymentMethods?.[0]?.hostedVerificationUrl}
/>
) : null}
<Suspense fallback={<Loader />}>
<Switch>
<SentryRoute path={path} exact>
Expand Down Expand Up @@ -90,4 +109,28 @@ function PlanPage() {
)
}

const UnverifiedPaymentMethodAlert = ({ url }: { url?: string | null }) => {
return (
<>
<Alert variant="warning">
<Alert.Title>Verify Your New Payment Method</Alert.Title>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should double check caps on this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the title and what @Adal3n3 suggested. I could go either way on that 🤷‍♀️

<Alert.Description>
Your new payment method needs to be verified.{' '}
<A
href={url}
isExternal
hook="stripe-payment-method-verification"
to={undefined}
>
Click here
</A>{' '}
to complete the process. The verification code may take around 2 days
to appear on your bank statement.
</Alert.Description>
</Alert>
<br />
</>
)
}

export default PlanPage
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import { MemoryRouter, Route } from 'react-router-dom'
import { z } from 'zod'

import { PlanUpdatedPlanNotificationContext } from 'pages/PlanPage/context'
import { AccountDetailsSchema, TrialStatuses } from 'services/account'
import { BillingRate, Plans } from 'shared/utils/billing'
import { AccountDetailsSchema } from 'services/account'
import { Plans } from 'shared/utils/billing'
import { AlertOptions, type AlertOptionsType } from 'ui/Alert'

import CurrentOrgPlan from './CurrentOrgPlan'
Expand All @@ -36,28 +36,6 @@ const mockNoEnterpriseAccount = {
},
}

const mockPlanDataResponse = {
baseUnitPrice: 10,
benefits: [],
billingRate: BillingRate.MONTHLY,
marketingName: 'some-name',
monthlyUploadLimit: 123,
value: Plans.USERS_PR_INAPPM,
trialStatus: TrialStatuses.NOT_STARTED,
trialStartDate: '',
trialEndDate: '',
trialTotalDays: 0,
pretrialUsersCount: 0,
planUserCount: 1,
hasSeatsLeft: true,
isEnterprisePlan: false,
isFreePlan: false,
isProPlan: false,
isSentryPlan: false,
isTeamPlan: false,
isTrialPlan: false,
}

const mockEnterpriseAccountDetailsNinetyPercent = {
owner: {
account: {
Expand Down Expand Up @@ -170,10 +148,15 @@ describe('CurrentOrgPlan', () => {
graphql.query('EnterpriseAccountDetails', () => {
return HttpResponse.json({ data: enterpriseAccountDetails })
}),
graphql.query('GetPlanData', () => {
graphql.query('CurrentOrgPlanPageData', () => {
return HttpResponse.json({
data: {
owner: { hasPrivateRepos: true, plan: { ...mockPlanDataResponse } },
owner: {
plan: { value: Plans.USERS_PR_INAPPM },
billing: {
unverifiedPaymentMethods: [],
},
},
},
})
}),
Expand Down
27 changes: 20 additions & 7 deletions src/pages/PlanPage/subRoutes/CurrentOrgPlan/CurrentOrgPlan.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useSuspenseQuery as useSuspenseQueryV5 } from '@tanstack/react-queryV5'
import { useParams } from 'react-router-dom'

import { usePlanUpdatedNotification } from 'pages/PlanPage/context'
import { useAccountDetails, usePlanData } from 'services/account'
import { useAccountDetails, useCurrentOrgPlanPageData } from 'services/account'
import { Provider } from 'shared/api/helpers'
import { getScheduleStart } from 'shared/plan/ScheduledPlanDetails/ScheduledPlanDetails'
import A from 'ui/A'
Expand All @@ -28,7 +28,7 @@ function CurrentOrgPlan() {
owner,
})

const { data: planData } = usePlanData({
const { data: pageData } = useCurrentOrgPlanPageData({
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can maybe just use data vs. pageData. Meaning is around the same but can avoid the cast

provider,
owner,
})
Expand All @@ -40,16 +40,25 @@ function CurrentOrgPlan() {
})
)

// awaitingInitialPaymentMethodVerification is true if the
// customer needs to verify a delayed notification payment method
// like ACH for their first subscription
const awaitingInitialPaymentMethodVerification =
!accountDetails?.subscriptionDetail?.defaultPaymentMethod &&
pageData?.billing?.unverifiedPaymentMethods?.length

const scheduledPhase = accountDetails?.scheduleDetail?.scheduledPhase
const isDelinquent = accountDetails?.delinquent
const isDelinquent =
accountDetails?.delinquent && !awaitingInitialPaymentMethodVerification
const scheduleStart = scheduledPhase
? getScheduleStart(scheduledPhase)
: undefined

const shouldRenderBillingDetails =
(accountDetails?.planProvider !== 'github' &&
!awaitingInitialPaymentMethodVerification &&
((accountDetails?.planProvider !== 'github' &&
!accountDetails?.rootOrganization) ||
accountDetails?.usesInvoice
accountDetails?.usesInvoice)

const planUpdatedNotification = usePlanUpdatedNotification()

Expand All @@ -62,9 +71,13 @@ function CurrentOrgPlan() {
subscriptionDetail={accountDetails?.subscriptionDetail}
/>
) : null}
<InfoMessageStripeCallback />
<InfoMessageStripeCallback
hasUnverifiedPaymentMethods={
!!pageData?.billing?.unverifiedPaymentMethods?.length
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can maybe set this to a const at the top as hasUnverifiedPaymentMethods and use that for the awaitingInitialPaymentMethodVerification const

}
/>
{isDelinquent ? <DelinquentAlert /> : null}
{planData?.plan ? (
{pageData?.plan ? (
<div className="flex flex-col gap-4 sm:mr-4 sm:flex-initial md:w-2/3 lg:w-3/4">
{planUpdatedNotification.alertOption &&
!planUpdatedNotification.isCancellation ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@ const wrapper =

describe('InfoMessageStripeCallback', () => {
describe('when rendering without success or cancel in the url', () => {
const { container } = render(<InfoMessageStripeCallback />, {
wrapper: wrapper('/account/gh/codecov'),
})
const { container } = render(
<InfoMessageStripeCallback hasUnverifiedPaymentMethods={false} />,
{
wrapper: wrapper('/account/gh/codecov'),
}
)

it('doesnt render anything', () => {
expect(container).toBeEmptyDOMElement()
Expand All @@ -23,13 +26,29 @@ describe('InfoMessageStripeCallback', () => {

describe('when rendering with success in the url', () => {
it('renders a success message', async () => {
render(<InfoMessageStripeCallback />, {
wrapper: wrapper('/account/gh/codecov?success'),
})
render(
<InfoMessageStripeCallback hasUnverifiedPaymentMethods={false} />,
{
wrapper: wrapper('/account/gh/codecov?success'),
}
)

await expect(
screen.getByText(/Subscription Update Successful/)
).toBeInTheDocument()
})
})

describe('when hasUnverifiedPaymentMethods is true', () => {
it('does not enders a success message even at ?success', async () => {
const { container } = render(
<InfoMessageStripeCallback hasUnverifiedPaymentMethods={true} />,
{
wrapper: wrapper('/account/gh/codecov?success'),
}
)

expect(container).toBeEmptyDOMElement()
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@ import Message from 'old_ui/Message'

// Stripe redirects to this page with ?success or ?cancel in the URL
// this component takes care of rendering a message if it is successful
function InfoMessageStripeCallback() {
function InfoMessageStripeCallback({
hasUnverifiedPaymentMethods,
}: {
hasUnverifiedPaymentMethods: boolean
}) {
const urlParams = qs.parse(useLocation().search, {
ignoreQueryPrefix: true,
})

if ('success' in urlParams)
if ('success' in urlParams && !hasUnverifiedPaymentMethods)
return (
<div className="col-start-1 col-end-13 mb-4">
<Message variant="success">
Expand Down
Loading