From 2e320172961a3bfce335999363f7521b8e547042 Mon Sep 17 00:00:00 2001 From: calvin-codecov Date: Wed, 15 Jan 2025 15:51:45 -0800 Subject: [PATCH] Cy/new tos (#3642) --- .../TermsOfService/TermsOfService.test.tsx | 176 ++++++------------ src/pages/TermsOfService/TermsOfService.tsx | 166 ++++++++--------- .../hooks/useTermsOfService.test.tsx | 41 +--- .../TermsOfService/hooks/useTermsOfService.ts | 14 +- 4 files changed, 152 insertions(+), 245 deletions(-) diff --git a/src/pages/TermsOfService/TermsOfService.test.tsx b/src/pages/TermsOfService/TermsOfService.test.tsx index 048ab16f6e..fd68f26516 100644 --- a/src/pages/TermsOfService/TermsOfService.test.tsx +++ b/src/pages/TermsOfService/TermsOfService.test.tsx @@ -248,9 +248,6 @@ describe('TermsOfService', () => { /I agree to the TOS and privacy policy/i ) - const customerIntent = screen.getByRole('radio', { name: /Personal use/ }) - await user.click(customerIntent) - await user.click(selectedTos) const submit = await screen.findByRole('button', { name: /Continue/ }) @@ -263,7 +260,7 @@ describe('TermsOfService', () => { businessEmail: 'personal@cr.com', termsAgreement: true, marketingConsent: false, - customerIntent: 'PERSONAL', + name: 'Chetney', }, }) ) @@ -298,7 +295,7 @@ describe('TermsOfService', () => { 'case #1', { validationDescription: - 'user has email, signs TOS, submit is now enabled', + 'user has email and name, signs TOS, submit is now enabled', internalUserData: { email: 'personal@cr.com', termsAgreement: false, @@ -309,15 +306,15 @@ describe('TermsOfService', () => { }, [expectPageIsReady], [expectSubmitIsDisabled], + [expectPrepopulatedFields, { email: 'personal@cr.com', name: 'Chetney' }], [expectUserSignsTOS], - [expectUserToChooseCustomerIntent], [expectSubmitIsEnabled], ], [ 'case #2', { validationDescription: - 'user wants to receive emails, signs TOS, submit is now enabled', + 'user has email and name, user wants to receive emails, signs TOS, submit is now enabled', internalUserData: { email: 'chetney@cr.com', termsAgreement: false, @@ -327,38 +324,18 @@ describe('TermsOfService', () => { }, }, [expectPageIsReady], - [expectUserSelectsMarketingWithFoundEmail, { email: 'chetney@cr.com' }], + [expectPrepopulatedFields, { email: 'chetney@cr.com', name: 'Chetney' }], [expectSubmitIsDisabled], - [expectUserToChooseCustomerIntent], - [expectUserSignsTOS], - [expectSubmitIsEnabled], - ], - [ - 'case #3', - { - validationDescription: - 'user has email, user wants to receive emails, signs TOS, submit is now enabled', - internalUserData: { - email: 'chetney@cr.com', - termsAgreement: false, - name: 'Chetney', - externalId: '1234', - owners: null, - }, - }, - [expectPageIsReady], - [expectSubmitIsDisabled], - [expectUserSelectsMarketingWithFoundEmail, { email: 'chetney@cr.com' }], + [expectUserSelectsMarketing], [expectSubmitIsDisabled], [expectUserSignsTOS], - [expectUserToChooseCustomerIntent], [expectSubmitIsEnabled], ], [ - 'case #4', + 'case #3', { validationDescription: - 'signs TOS, decides not to, is warned they must sign and cannot submit', + 'has prefilled email and name, signs TOS, decides not to, is warned they must sign and cannot submit', internalUserData: { email: 'chetney@cr.com', termsAgreement: false, @@ -369,7 +346,7 @@ describe('TermsOfService', () => { }, [expectPageIsReady], [expectSubmitIsDisabled], - [expectUserToChooseCustomerIntent], + [expectPrepopulatedFields, { email: 'chetney@cr.com', name: 'Chetney' }], [expectUserSignsTOS], [expectSubmitIsEnabled], [expectUserSignsTOS], @@ -377,10 +354,10 @@ describe('TermsOfService', () => { [expectUserIsWarnedTOS], ], [ - 'case #5', + 'case #4', { validationDescription: - 'user checks marketing consent and is required to provide an email, sign TOS (check email validation messages)', + 'user checks marketing consent and is required to provide an email, provide a name, sign TOS (check email validation messages)', internalUserData: { termsAgreement: false, name: 'Chetney', @@ -391,21 +368,20 @@ describe('TermsOfService', () => { }, [expectPageIsReady], [expectSubmitIsDisabled], - [expectEmailRequired], [expectUserTextEntryEmailField, { email: 'chetney' }], [expectUserIsWarnedForValidEmail], [expectSubmitIsDisabled], - [expectUserTextEntryEmailField, { email: '@cr.com' }], + [expectUserTextEntryEmailField, { email: '@hello.com' }], [expectUserIsNotWarnedForValidEmail], [expectSubmitIsDisabled], + [expectUserTextEntryNameField], [expectUserSelectsMarketing], [expectSubmitIsDisabled], - [expectUserToChooseCustomerIntent], [expectUserSignsTOS], [expectSubmitIsEnabled], ], [ - 'case #6', + 'case #5', { validationDescription: 'user checks marketing consent and does not provide an email, sign TOS (check email validation messages)', @@ -418,29 +394,28 @@ describe('TermsOfService', () => { }, }, [expectPageIsReady], - [expectEmailRequired], [expectSubmitIsDisabled], [expectUserSignsTOS], [expectSubmitIsDisabled], - [expectUserToChooseCustomerIntent], ], [ - 'case #7', + 'case #6', { validationDescription: 'server unknown error notification', isUnknownError: true, internalUserData: { termsAgreement: false, - email: 'personal@cr.com', - name: 'Chetney', + email: '', + name: '', externalId: '1234', owners: null, }, }, [expectPageIsReady], + [expectUserTextEntryEmailField, { email: 'personal@cr.com' }], + [expectUserTextEntryNameField], [expectUserSignsTOS], [expectClickSubmit], - [expectUserToChooseCustomerIntent], [ expectRendersServerFailureResult, { @@ -456,22 +431,23 @@ describe('TermsOfService', () => { ], ], [ - 'case #8', + 'case #7', { validationDescription: 'server failure error notification', isUnAuthError: true, internalUserData: { termsAgreement: false, - email: 'personal@cr.com', - name: 'Chetney', + email: '', + name: '', externalId: '1234', owners: null, }, }, [expectPageIsReady], + [expectUserTextEntryEmailField, { email: 'personal@cr.com' }], + [expectUserTextEntryNameField], [expectUserSignsTOS], [expectClickSubmit], - [expectUserToChooseCustomerIntent], [ expectRendersServerFailureResult, { @@ -481,27 +457,28 @@ describe('TermsOfService', () => { ], ], [ - 'case #9', + 'case #8', { validationDescription: 'server validation error notification (saveTerms)', isValidationError: true, internalUserData: { termsAgreement: false, - email: 'personal@cr.com', - name: 'Chetney', + email: '', + name: '', externalId: '1234', owners: null, }, }, [expectPageIsReady], + [expectUserTextEntryEmailField, { email: 'personal@cr.com' }], + [expectUserTextEntryNameField], [expectUserSignsTOS], [expectClickSubmit], - [expectUserToChooseCustomerIntent], [expectRendersServerFailureResult, 'validation error'], ], [ - 'case #10', + 'case #9', { validationDescription: 'redirects to main root if user has already synced a provider', @@ -526,33 +503,6 @@ describe('TermsOfService', () => { }, [expectRedirectTo, '/gh/codecov/cool-repo'], ], - [ - 'case #11', - { - validationDescription: 'provide no customer intent, does not submit', - internalUserData: { - termsAgreement: true, - name: 'Chetney', - externalId: '1234', - email: '', - owners: [ - { - avatarUrl: 'http://roland.com/avatar-url', - integrationId: null, - name: null, - ownerid: 2, - stats: null, - service: 'github', - username: 'roland', - }, - ], - }, - }, - [expectPageIsReady], - [expectSubmitIsDisabled], - [expectUserSignsTOS], - [expectSubmitIsDisabled], - ], ])( 'form validation, %s', ( @@ -693,33 +643,43 @@ async function expectPageIsReady() { expect(welcome).toBeInTheDocument() } -async function expectUserToChooseCustomerIntent(user: UserEvent) { - const customerIntent = screen.getByRole('radio', { name: /Personal use/ }) - - await user.click(customerIntent) +async function expectPrepopulatedFields( + user: UserEvent, + args: { email: string; name: string } +) { + await waitFor(() => { + const emailInput = screen.getByLabelText( + /Enter your email/i + ) as HTMLInputElement + expect(emailInput).toHaveValue(args.email) + }) + await waitFor(() => { + const nameInput = screen.getByLabelText( + /Enter your name/i + ) as HTMLInputElement + expect(nameInput).toHaveValue(args.name) + }) } -async function expectUserSignsTOS(user: UserEvent) { - const selectedTos = screen.getByLabelText( - /I agree to the TOS and privacy policy/i - ) - - await user.click(selectedTos) +async function expectUserTextEntryNameField(user: UserEvent) { + const nameInput = screen.getByLabelText(/Enter your name/i) + await user.type(nameInput, 'My name') } -async function expectUserSelectsMarketingWithFoundEmail( +async function expectUserTextEntryEmailField( user: UserEvent, args: { email: string } ) { - const selectedMarketing = screen.getByLabelText( - /I would like to receive updates via email/i - ) - const emailIsInTheLabelOfSelectedMarketing = screen.getByText( - new RegExp(args.email, 'i') + const emailInput = screen.getByLabelText(/Enter your email/i) + await user.type(emailInput, args.email) +} + +async function expectUserSignsTOS(user: UserEvent) { + const selectedTos = screen.getByLabelText( + /I agree to the TOS and privacy policy/i ) - expect(emailIsInTheLabelOfSelectedMarketing).toBeInTheDocument() - await user.click(selectedMarketing) + await user.click(selectedTos) } async function expectUserSelectsMarketing(user: UserEvent) { @@ -730,15 +690,6 @@ async function expectUserSelectsMarketing(user: UserEvent) { await user.click(selectedMarketing) } -async function expectUserTextEntryEmailField( - user: UserEvent, - args: { email: string } -) { - const emailInput = screen.getByLabelText(/Contact email/i) - - await user.type(emailInput, args.email) -} - async function expectSubmitIsDisabled() { const submit = screen.getByRole('button', { name: /Continue/ }) expect(submit).toBeDisabled() @@ -770,17 +721,6 @@ async function expectClickSubmit(user: UserEvent) { await user.click(submit) } -async function expectEmailRequired(user: UserEvent) { - const selectedMarketing = screen.getByLabelText( - /I would like to receive updates via email/i - ) - - await user.click(selectedMarketing) - - const emailRequired = screen.getByText(/Contact email/i) - expect(emailRequired).toBeInTheDocument() -} - async function expectRendersServerFailureResult( user: UserEvent, expectedError = {} diff --git a/src/pages/TermsOfService/TermsOfService.tsx b/src/pages/TermsOfService/TermsOfService.tsx index 1e1e1a290b..54fb433934 100644 --- a/src/pages/TermsOfService/TermsOfService.tsx +++ b/src/pages/TermsOfService/TermsOfService.tsx @@ -8,21 +8,29 @@ import config from 'config' import { SentryBugReporter } from 'sentry' import umbrellaSvg from 'assets/svg/umbrella.svg' -import { CustomerIntent, useInternalUser } from 'services/user' +import { useInternalUser } from 'services/user' import A from 'ui/A' import Button from 'ui/Button' -import RadioInput from 'ui/RadioInput/RadioInput' import TextInput from 'ui/TextInput' import { useSaveTermsAgreement } from './hooks/useTermsOfService' const FormSchema = z.object({ - marketingEmail: z.string().email().nullish(), + marketingName: z.string().min(1, 'Name is required'), + marketingEmail: z.string().email('Invalid email'), marketingConsent: z.boolean().nullish(), tos: z.literal(true), - customerIntent: z.string(), + apiError: z.string().nullish(), }) +type FormData = { + marketingName?: string + marketingEmail?: string + marketingConsent?: boolean + tos?: boolean + apiError?: string +} + interface IsDisabled { isValid: boolean isDirty: boolean @@ -33,33 +41,50 @@ function isDisabled({ isValid, isDirty, isMutationLoading }: IsDisabled) { return (!isValid && isDirty) || !isDirty || isMutationLoading } +interface NameInputProps { + register: ReturnType['register'] + marketingNameMessage?: string +} + +function NameInput({ register, marketingNameMessage }: NameInputProps) { + return ( +
+ +
+ + {marketingNameMessage && ( +

{marketingNameMessage}

+ )} +
+
+ ) +} + interface EmailInputProps { register: ReturnType['register'] marketingEmailMessage?: string - showEmailRequired: boolean } -function EmailInput({ - register, - marketingEmailMessage, - showEmailRequired, -}: EmailInputProps) { - if (!showEmailRequired) return null - +function EmailInput({ register, marketingEmailMessage }: EmailInputProps) { return (
{marketingEmailMessage && ( @@ -71,17 +96,40 @@ function EmailInput({ } export default function TermsOfService() { + const { data: currentUser, isLoading: userIsLoading } = useInternalUser({}) const { register, + reset, handleSubmit, formState: { isDirty, isValid, errors: formErrors }, setError, watch, unregister, - } = useForm({ + } = useForm({ resolver: zodResolver(FormSchema), mode: 'onChange', + defaultValues: { + marketingName: currentUser?.name || '', + marketingEmail: currentUser?.email || '', + marketingConsent: undefined, + tos: false, + // this field just used for custom form error + apiError: undefined, + }, }) + + useEffect(() => { + if (currentUser && !isDirty) { + reset({ + marketingName: currentUser.name || '', + marketingEmail: currentUser.email || '', + marketingConsent: undefined, + tos: false, + apiError: undefined, + }) + } + }, [currentUser, isDirty, reset]) + const { mutate, isLoading: isMutationLoading } = useSaveTermsAgreement({ onSuccess: ({ data }) => { if (data?.saveTermsAgreement?.error) { @@ -91,7 +139,6 @@ export default function TermsOfService() { }, onError: (error) => setError('apiError', error), }) - const { data: currentUser, isLoading: userIsLoading } = useInternalUser({}) useLayoutEffect(() => { if (!config.SENTRY_DSN) { @@ -101,18 +148,14 @@ export default function TermsOfService() { return widget.removeFromDom }, []) - interface FormValues { - marketingEmail?: string - marketingConsent?: boolean - customerIntent?: string - } + const onSubmit: SubmitHandler = (data: FormData) => { + if (!data.marketingName || !data.marketingEmail) return - const onSubmit: SubmitHandler = (data: FormValues) => { mutate({ - businessEmail: data?.marketingEmail || currentUser?.email, - termsAgreement: true, + businessEmail: data.marketingEmail, marketingConsent: data?.marketingConsent, - customerIntent: data?.customerIntent || CustomerIntent.PERSONAL, + name: data.marketingName, + termsAgreement: true, }) } @@ -139,41 +182,14 @@ export default function TermsOfService() {
- -
- - -
-
- - -
+ +
-
- + I agree to{' '} {' '} - required
{formErrors?.tos && ( diff --git a/src/pages/TermsOfService/hooks/useTermsOfService.test.tsx b/src/pages/TermsOfService/hooks/useTermsOfService.test.tsx index f4a2609f54..9557fc6b1b 100644 --- a/src/pages/TermsOfService/hooks/useTermsOfService.test.tsx +++ b/src/pages/TermsOfService/hooks/useTermsOfService.test.tsx @@ -94,7 +94,7 @@ describe('useSaveTermsAgreement', () => { result.current.mutate({ businessEmail: 'test@test.com', termsAgreement: true, - customerIntent: 'PERSONAL', + name: 'Test Name', }) await waitFor(() => expect(successFn).toHaveBeenCalledWith('completed')) @@ -113,7 +113,7 @@ describe('useSaveTermsAgreement', () => { result.current.mutate({ businessEmail: 'test@test.com', termsAgreement: true, - customerIntent: 'PERSONAL', + name: 'Test Name', }) await waitFor(() => expect(testLocation.pathname).toEqual('/')) @@ -140,7 +140,7 @@ describe('useSaveTermsAgreement', () => { result.current.mutate({ businessEmail: 'test@test.com', termsAgreement: true, - customerIntent: 'PERSONAL', + name: 'Test Name', }) await waitFor(() => expect(successFn).toHaveBeenCalledWith('completed')) @@ -159,42 +159,13 @@ describe('useSaveTermsAgreement', () => { result.current.mutate({ businessEmail: 'test@test.com', termsAgreement: true, - customerIntent: 'PERSONAL', + name: 'Test Name', }) await waitFor(() => expect(testLocation.pathname).toEqual('/')) }) }) - it('proceeds with mutation on missing email', async () => { - setup() - const invalidateQueries = vi.spyOn(queryClient, 'invalidateQueries') - const successFn = vi.fn() - const { result } = renderHook( - () => - useSaveTermsAgreement({ - onSuccess: () => { - successFn('completed') - }, - }), - { - wrapper: wrapper(), - } - ) - - result.current.mutate({ - businessEmail: null, - termsAgreement: true, - customerIntent: 'PERSONAL', - }) - - await waitFor(() => expect(successFn).toHaveBeenCalledWith('completed')) - - expect(invalidateQueries).toHaveBeenCalledWith({ - queryKey: ['InternalUser'], - }) - }) - describe('there is was an api error', () => { it('throws an error', async () => { setup({ apiError: true }) @@ -215,7 +186,7 @@ describe('useSaveTermsAgreement', () => { result.current.mutate({ businessEmail: 'test@test.com', termsAgreement: true, - customerIntent: 'PERSONAL', + name: 'Test Name', }) await waitFor(() => @@ -249,7 +220,7 @@ describe('useSaveTermsAgreement', () => { result.current.mutate({ businessEmail: 'test@test.com', termsAgreement: true, - customerIntent: 'PERSONAL', + name: 'Test Name', }) await waitFor(() => expect(testLocation.pathname).toEqual('/gh')) diff --git a/src/pages/TermsOfService/hooks/useTermsOfService.ts b/src/pages/TermsOfService/hooks/useTermsOfService.ts index 9eef5d57d7..858dbcd515 100644 --- a/src/pages/TermsOfService/hooks/useTermsOfService.ts +++ b/src/pages/TermsOfService/hooks/useTermsOfService.ts @@ -5,10 +5,10 @@ import { z } from 'zod' import Api from 'shared/api' const SaveTermsAgreementInputConfig = z.object({ - businessEmail: z.string().nullable().optional(), + businessEmail: z.string(), + name: z.string(), termsAgreement: z.boolean(), marketingConsent: z.boolean().optional(), - customerIntent: z.string(), }) export type SaveTermsAgreementInput = z.infer< @@ -47,12 +47,8 @@ export function useSaveTermsAgreement(options: SaveTermsAgreementOptions = {}) { mutationFn: (input: SaveTermsAgreementInput) => { const parsedInput = SaveTermsAgreementInputConfig.parse(input) - const { - businessEmail, - termsAgreement, - marketingConsent, - customerIntent, - } = parsedInput + const { businessEmail, termsAgreement, marketingConsent, name } = + parsedInput const querySignAgreement = ` mutation SigningTermsAgreement($tosInput: SaveTermsAgreementInput!) { @@ -72,7 +68,7 @@ export function useSaveTermsAgreement(options: SaveTermsAgreementOptions = {}) { businessEmail, termsAgreement, marketingConsent, - customerIntent, + name, }, } return Api.graphqlMutation({