Skip to content

Commit

Permalink
Merge branch 'main' into gh-eng-2963-migrate-useUpdateOktaConfig-to-t…
Browse files Browse the repository at this point in the history
…sq-v5
  • Loading branch information
nicholas-codecov authored Feb 4, 2025
2 parents e222290 + 71c0a5b commit 738962e
Show file tree
Hide file tree
Showing 14 changed files with 127 additions and 44 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@
"@radix-ui/react-radio-group": "^1.1.3",
"@radix-ui/react-tooltip": "^1.1.2",
"@sentry/react": "^8.35.0",
"@stripe/react-stripe-js": "^2.7.1",
"@stripe/stripe-js": "^3.4.0",
"@stripe/react-stripe-js": "^3.1.1",
"@stripe/stripe-js": "^5.6.0",
"@tanstack/react-query": "^4.29.5",
"@tanstack/react-queryV5": "npm:@tanstack/react-query@^5.59.15",
"@tanstack/react-table": "^8.9.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function PaymentCard({ accountDetails, provider, owner }) {
variant="primary"
onClick={() => setIsFormOpen(true)}
>
Set card
Set payment method
</Button>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { render, screen } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { MemoryRouter } from 'react-router-dom'

import { ThemeContextProvider } from 'shared/ThemeContext'
import { Plans } from 'shared/utils/billing'
Expand Down Expand Up @@ -57,6 +58,7 @@ const subscriptionDetail = {

const accountDetails = {
subscriptionDetail,
activatedUserCount: 1,
}

const usBankSubscriptionDetail = {
Expand All @@ -75,7 +77,9 @@ const usBankSubscriptionDetail = {

const wrapper = ({ children }) => (
<QueryClientProvider client={queryClient}>
<ThemeContextProvider>{children}</ThemeContextProvider>
<MemoryRouter>
<ThemeContextProvider>{children}</ThemeContextProvider>
</MemoryRouter>
</QueryClientProvider>
)

Expand Down Expand Up @@ -383,7 +387,13 @@ describe('PaymentCard', () => {

await user.click(screen.getByTestId('edit-payment-method'))

expect(screen.getByText(randomError)).toBeInTheDocument()
expect(
screen.getByText((content) =>
content.includes(
"There's been an error. Please try refreshing your browser"
)
)
).toBeInTheDocument()
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,15 @@ const mocks = {
useUpdatePaymentMethod: vi.fn(),
}

vi.mock('services/account/useUpdatePaymentMethod', () => ({
useUpdatePaymentMethod: () => mocks.useUpdatePaymentMethod(),
}))
vi.mock('services/account/useUpdatePaymentMethod', async () => {
const original = await vi.importActual(
'services/account/useUpdatePaymentMethod'
)
return {
...original,
useUpdatePaymentMethod: () => mocks.useUpdatePaymentMethod(),
}
})

afterEach(() => {
vi.clearAllMocks()
Expand Down Expand Up @@ -214,7 +220,13 @@ describe('PaymentMethodForm', () => {

await user.click(screen.getByTestId('save-payment-method'))

expect(screen.getByText(randomError)).toBeInTheDocument()
expect(
screen.getByText((content) =>
content.includes(
"There's been an error. Please try refreshing your browser"
)
)
).toBeInTheDocument()
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,14 @@ import cs from 'classnames'
import { z } from 'zod'

import { AccountDetailsSchema, BillingDetailsSchema } from 'services/account'
import { useUpdatePaymentMethod } from 'services/account/useUpdatePaymentMethod'
import {
MissingAddressError,
MissingEmailError,
MissingNameError,
useUpdatePaymentMethod,
} from 'services/account/useUpdatePaymentMethod'
import { Provider } from 'shared/api/helpers'
import A from 'ui/A'
import Button from 'ui/Button'

interface PaymentMethodFormProps {
Expand Down Expand Up @@ -81,7 +87,7 @@ const PaymentMethodForm = ({
}}
/>
<p className="mt-1 text-ds-primary-red">
{showError && error?.message}
{showError ? getErrorMessage(error) : null}
</p>
<div className="mb-8 mt-4 flex gap-1">
<Button
Expand Down Expand Up @@ -150,4 +156,27 @@ export const getName = (
)
}

export const getErrorMessage = (error: Error): JSX.Element => {
switch (error.message) {
case MissingNameError:
return <span>Missing name, please edit Full Name</span>
case MissingEmailError:
return <span>Missing email, please edit Email</span>
case MissingAddressError:
return <span>Missing address, please edit Address</span>
default:
return (
<span>
There&apos;s been an error. Please try refreshing your browser, if
this error persists please{' '}
{/* @ts-expect-error ignore until we can convert A component to ts */}
<A to={{ pageName: 'support' }} variant="link">
contact support
</A>
.
</span>
)
}
}

export default PaymentMethodForm
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ describe('CurrentOrgPlan', () => {
)
expect(paymentFailed).toBeInTheDocument()
const contactSupport = await screen.findByText(
'Please try a different card or contact support at [email protected].'
'Please try a different payment method or contact support at [email protected].'
)
expect(contactSupport).toBeInTheDocument()
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ function CurrentOrgPlan() {
/>
) : null}
<InfoMessageStripeCallback
hasUnverifiedPaymentMethods={hasUnverifiedPaymentMethods}
awaitingInitialPaymentMethodVerification={
awaitingInitialPaymentMethodVerification
}
/>
{isDelinquent ? <DelinquentAlert /> : null}
{data?.plan ? (
Expand Down Expand Up @@ -161,7 +163,8 @@ const DelinquentAlert = () => {
<Alert variant={'error'}>
<Alert.Title>Your most recent payment failed</Alert.Title>
<Alert.Description>
Please try a different card or contact support at [email protected].
Please try a different payment method or contact support at
[email protected].
</Alert.Description>
</Alert>
<br />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ const wrapper =
describe('InfoMessageStripeCallback', () => {
describe('when rendering without success or cancel in the url', () => {
const { container } = render(
<InfoMessageStripeCallback hasUnverifiedPaymentMethods={false} />,
<InfoMessageStripeCallback
awaitingInitialPaymentMethodVerification={false}
/>,
{
wrapper: wrapper('/account/gh/codecov'),
}
Expand All @@ -27,7 +29,9 @@ describe('InfoMessageStripeCallback', () => {
describe('when rendering with success in the url', () => {
it('renders a success message', async () => {
render(
<InfoMessageStripeCallback hasUnverifiedPaymentMethods={false} />,
<InfoMessageStripeCallback
awaitingInitialPaymentMethodVerification={false}
/>,
{
wrapper: wrapper('/account/gh/codecov?success'),
}
Expand All @@ -39,10 +43,12 @@ describe('InfoMessageStripeCallback', () => {
})
})

describe('when hasUnverifiedPaymentMethods is true', () => {
describe('when awaitingInitialPaymentMethodVerification is true', () => {
it('does not enders a success message even at ?success', async () => {
const { container } = render(
<InfoMessageStripeCallback hasUnverifiedPaymentMethods={true} />,
<InfoMessageStripeCallback
awaitingInitialPaymentMethodVerification={true}
/>,
{
wrapper: wrapper('/account/gh/codecov?success'),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ 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({
hasUnverifiedPaymentMethods,
awaitingInitialPaymentMethodVerification,
}: {
hasUnverifiedPaymentMethods: boolean
awaitingInitialPaymentMethodVerification: boolean
}) {
const urlParams = qs.parse(useLocation().search, {
ignoreQueryPrefix: true,
})

if ('success' in urlParams && !hasUnverifiedPaymentMethods)
if ('success' in urlParams && !awaitingInitialPaymentMethodVerification)
return (
<div className="col-start-1 col-end-13 mb-4">
<Message variant="success">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ type SetupArgs = {
hasSentryPlans?: boolean
monthlyPlan?: boolean
planUserCount?: number
hasUnverifiedPaymentMethod?: boolean
hasUnverifiedPaymentMethods?: boolean
subscriptionHasDefaultPaymentMethod?: boolean
}

describe('UpgradeForm', () => {
Expand All @@ -310,7 +311,8 @@ describe('UpgradeForm', () => {
hasSentryPlans = false,
monthlyPlan = true,
planUserCount = 1,
hasUnverifiedPaymentMethod = false,
hasUnverifiedPaymentMethods = false,
subscriptionHasDefaultPaymentMethod = true,
}: SetupArgs) {
const addNotification = vi.fn()
const user = userEvent.setup()
Expand All @@ -319,6 +321,16 @@ describe('UpgradeForm', () => {

server.use(
http.get(`/internal/:provider/:owner/account-details/`, () => {
if (!subscriptionHasDefaultPaymentMethod) {
return HttpResponse.json({
...mockAccountDetailsBasic,
subscriptionDetail: {
...mockAccountDetailsBasic.subscriptionDetail,
defaultPaymentMethod: null,
},
})
}

if (planValue === Plans.USERS_BASIC) {
return HttpResponse.json(mockAccountDetailsBasic)
} else if (planValue === Plans.USERS_PR_INAPPM) {
Expand Down Expand Up @@ -408,7 +420,7 @@ describe('UpgradeForm', () => {
data: {
owner: {
billing: {
unverifiedPaymentMethods: hasUnverifiedPaymentMethod
unverifiedPaymentMethods: hasUnverifiedPaymentMethods
? [
{
paymentMethodId: 'asdf',
Expand Down Expand Up @@ -436,7 +448,8 @@ describe('UpgradeForm', () => {
it('shows modal when form is submitted', async () => {
const { user } = setup({
planValue: Plans.USERS_BASIC,
hasUnverifiedPaymentMethod: true,
hasUnverifiedPaymentMethods: true,
subscriptionHasDefaultPaymentMethod: false,
})
render(<UpgradeForm {...props} />, { wrapper: wrapper() })

Expand All @@ -457,7 +470,7 @@ describe('UpgradeForm', () => {
it('does not show modal when no unverified payment methods', async () => {
const { user } = setup({
planValue: Plans.USERS_BASIC,
hasUnverifiedPaymentMethod: false,
hasUnverifiedPaymentMethods: false,
})
render(<UpgradeForm {...props} />, { wrapper: wrapper() })

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,17 @@ function UpgradeForm({ selectedPlan, setSelectedPlan }: UpgradeFormProps) {
const newPlan = watch('newPlan')
const seats = watch('seats')

const awaitingInitialPaymentMethodVerification =
!!unverifiedPaymentMethods?.length &&
!accountDetails?.subscriptionDetail?.defaultPaymentMethod

useEffect(() => {
// This is necessary because the validity of seats depends on the value of newPlan
trigger('seats')
}, [newPlan, trigger])

const onSubmit = handleSubmit((data) => {
if (unverifiedPaymentMethods?.length) {
if (awaitingInitialPaymentMethodVerification) {
setFormData(data)
setShowModal(true)
} else {
Expand Down
9 changes: 5 additions & 4 deletions src/pages/TermsOfService/TermsOfService.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,12 @@ export default function TermsOfService() {
if (data?.saveTermsAgreement?.error) {
setError('apiError', data?.saveTermsAgreement?.error)
console.error('validation error')
return
}

const url = new URL(window.location.href)
url.searchParams.set('source', ONBOARDING_SOURCE)
window.location.href = url.toString()
},
onError: (error) => setError('apiError', error),
})
Expand All @@ -159,10 +164,6 @@ export default function TermsOfService() {
name: data.marketingName,
termsAgreement: true,
})

const url = new URL(window.location.href)
url.searchParams.set('source', ONBOARDING_SOURCE)
window.location.href = url.toString()
}

useEffect(() => {
Expand Down
5 changes: 5 additions & 0 deletions src/services/account/useUpdatePaymentMethod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,8 @@ export function useUpdatePaymentMethod({
},
})
}

// Errors from Stripe api confirmSetup() - unfortunately seems to just be by text, no error codes
export const MissingNameError = `You specified "never" for fields.billing_details.name when creating the payment Element, but did not pass confirmParams.payment_method_data.billing_details.name when calling stripe.confirmSetup(). If you opt out of collecting data via the payment Element using the fields option, the data must be passed in when calling stripe.confirmSetup().`
export const MissingEmailError = `You specified "never" for fields.billing_details.email when creating the payment Element, but did not pass confirmParams.payment_method_data.billing_details.email when calling stripe.confirmSetup(). If you opt out of collecting data via the payment Element using the fields option, the data must be passed in when calling stripe.confirmSetup().`
export const MissingAddressError = `You specified "never" for fields.billing_details.address when creating the payment Element, but did not pass confirmParams.payment_method_data.billing_details.address when calling stripe.confirmSetup(). If you opt out of collecting data via the payment Element using the fields option, the data must be passed in when calling stripe.confirmSetup().`
26 changes: 13 additions & 13 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4847,23 +4847,23 @@ __metadata:
languageName: node
linkType: hard

"@stripe/react-stripe-js@npm:^2.7.1":
version: 2.7.3
resolution: "@stripe/react-stripe-js@npm:2.7.3"
"@stripe/react-stripe-js@npm:^3.1.1":
version: 3.1.1
resolution: "@stripe/react-stripe-js@npm:3.1.1"
dependencies:
prop-types: "npm:^15.7.2"
peerDependencies:
"@stripe/stripe-js": ^1.44.1 || ^2.0.0 || ^3.0.0 || ^4.0.0
react: ^16.8.0 || ^17.0.0 || ^18.0.0
react-dom: ^16.8.0 || ^17.0.0 || ^18.0.0
checksum: 10c0/820923a3abfae68fbe8145efa0f35317634d44f7ada4b8d57f928f10dfa32e768252939ea75f2e166c55aa9035ecaa2d686423216d0bbfb06ba22c3c70fc7771
"@stripe/stripe-js": ^1.44.1 || ^2.0.0 || ^3.0.0 || ^4.0.0 || ^5.0.0
react: ">=16.8.0 <20.0.0"
react-dom: ">=16.8.0 <20.0.0"
checksum: 10c0/d8f877e956f711acb96248ab6b183d330c6b194884aeeb82e96bc05dab08020a2c53f0c13359e893ac22bc7e15facc720877cfb04d999b1f3050bb58aca5124f
languageName: node
linkType: hard

"@stripe/stripe-js@npm:^3.4.0":
version: 3.5.0
resolution: "@stripe/stripe-js@npm:3.5.0"
checksum: 10c0/df874448bd0c879c624f38ea286f9b662ead71fb08a95eb787b7fb6d46a89f414dc987094f5fe22001f65cf3063c14913c36b8d8c88b16847dd137f8298648c0
"@stripe/stripe-js@npm:^5.6.0":
version: 5.6.0
resolution: "@stripe/stripe-js@npm:5.6.0"
checksum: 10c0/56aa804ed2946aae4622b56c1736b7fa69e640d293d1ba6d465732851f313234c00d29006e2270bbdceee93222b5c62d8aa92791373ec56bfc4bd214a0b950c3
languageName: node
linkType: hard

Expand Down Expand Up @@ -9090,8 +9090,8 @@ __metadata:
"@storybook/react": "npm:^8.3.7"
"@storybook/react-vite": "npm:^8.3.7"
"@storybook/theming": "npm:^8.3.7"
"@stripe/react-stripe-js": "npm:^2.7.1"
"@stripe/stripe-js": "npm:^3.4.0"
"@stripe/react-stripe-js": "npm:^3.1.1"
"@stripe/stripe-js": "npm:^5.6.0"
"@tailwindcss/container-queries": "npm:^0.1.1"
"@tanstack/eslint-plugin-query": "npm:^4.38.0"
"@tanstack/react-query": "npm:^4.29.5"
Expand Down

0 comments on commit 738962e

Please sign in to comment.