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(DIA-927): conditionally display email enrollment checkbox during sign-up #11020

Merged
merged 3 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion e2e/flows/onboarding/signup.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ appId: ${MAESTRO_APP_ID}
when:
platform: Android
commands:
- tapOn: "checkbox of consent, Check this element to consent with the Terms of Service"
- tapOn: "Accept terms and privacy policy, Check this element to accept Artsy's terms and privacy policy"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@araujobarret do you know what this file is for?

Copy link
Contributor

Choose a reason for hiding this comment

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

old stuff before maestro cloud, was supposed to be used in automation, I have to clean that up

Copy link
Member

Choose a reason for hiding this comment

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

I think this file was related to our test of a new E2E tool that we've abandoned. @brainbicycle / @gkartalis - right?

Copy link
Contributor

Choose a reason for hiding this comment

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

this one is @araujobarret work on their main offering which is more standard e2e tests, we might start looking into it again at some point but feel free to delete we can always bring it back

- tapOn: "Next.*"
77 changes: 77 additions & 0 deletions src/app/Scenes/Onboarding/Auth2/hooks/useCountryCode.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { useCountryCodeQuery } from "__generated__/useCountryCodeQuery.graphql"
import { useClientQuery } from "app/utils/useClientQuery"
import { useEffect, useState } from "react"
import { getIpAddress } from "react-native-device-info"
import { graphql } from "react-relay"

const USE_COUNTRY_CODE_QUERY = graphql`
query useCountryCodeQuery($ip: String!) {
requestLocation(ip: $ip) {
countryCode
}
}
`

export const useCountryCode = () => {
const [ip, setIp] = useState("0.0.0.0")

useEffect(() => {
getIpAddress().then((ip) => {
setIp(ip)
})
}, [])

const { data, loading, error } = useClientQuery<useCountryCodeQuery>({
query: USE_COUNTRY_CODE_QUERY,
variables: {
ip,
},
cacheConfig: {
networkCacheConfig: {
force: false,
},
},
})

const countryCode = data?.requestLocation?.countryCode

const isAutomaticallySubscribed = !!(countryCode && !GDPR_COUNTRY_CODES.includes(countryCode))

return {
countryCode,
error,
isAutomaticallySubscribed,
loading,
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Nice and simple 👍


export const GDPR_COUNTRY_CODES = [
"AT",
"BE",
"BG",
"CY",
"CZ",
"DE",
"DK",
"EE",
"ES",
"FI",
"FR",
"GB",
"GR",
"HR",
"HU",
"IE",
"IT",
"LT",
"LU",
"LV",
"MT",
"NL",
"PL",
"PT",
"RO",
"SE",
"SI",
"SK",
]
22 changes: 17 additions & 5 deletions src/app/Scenes/Onboarding/Auth2/scenes/SignUpNameStep.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
useAuthNavigation,
useAuthScreen,
} from "app/Scenes/Onboarding/Auth2/hooks/useAuthNavigation"
import { useCountryCode } from "app/Scenes/Onboarding/Auth2/hooks/useCountryCode"
import { useInputAutofocus } from "app/Scenes/Onboarding/Auth2/hooks/useInputAutofocus"
import { OnboardingNavigationStack } from "app/Scenes/Onboarding/Onboarding"
import { EmailSubscriptionCheckbox } from "app/Scenes/Onboarding/OnboardingCreateAccount/EmailSubscriptionCheckbox"
Expand All @@ -23,13 +24,18 @@ interface SignUpNameStepFormValues {

export const SignUpNameStep: React.FC = () => {
const screen = useAuthScreen()
const { loading, isAutomaticallySubscribed } = useCountryCode()

if (loading) {
return null
Copy link
Member

Choose a reason for hiding this comment

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

This is the sort of thing where we should avoid using a skeleton because its so conditional. For most users, this just wont appear and we can probably just have it pop in. But def play with it! might be able to make it nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Thanks!

}

return (
<Formik<SignUpNameStepFormValues>
initialValues={{
name: "",
acceptedTerms: false,
agreedToReceiveEmails: false,
agreedToReceiveEmails: isAutomaticallySubscribed,
}}
validationSchema={Yup.object().shape({
name: Yup.string().trim().required("Full name field is required"),
Expand Down Expand Up @@ -84,6 +90,7 @@ const SignUpNameStepForm: React.FC = () => {

const navigation = useAuthNavigation()
const parentNavigation = useNavigation<NavigationProp<OnboardingNavigationStack>>()
const { isAutomaticallySubscribed } = useCountryCode()
const { color } = useTheme()
const nameRef = useRef<Input>(null)

Expand All @@ -106,6 +113,7 @@ const SignUpNameStepForm: React.FC = () => {
<Text variant="sm-display">Welcome to Artsy</Text>

<Input
accessibilityHint="Enter your full name"
autoCapitalize="words"
autoComplete="name"
autoCorrect={false}
Expand Down Expand Up @@ -146,10 +154,14 @@ const SignUpNameStepForm: React.FC = () => {
error={highlightTerms}
navigation={parentNavigation}
/>
<EmailSubscriptionCheckbox
setChecked={() => setFieldValue("agreedToReceiveEmails", !values.agreedToReceiveEmails)}
checked={values.agreedToReceiveEmails}
/>
{!isAutomaticallySubscribed ? (
<EmailSubscriptionCheckbox
setChecked={() => setFieldValue("agreedToReceiveEmails", !values.agreedToReceiveEmails)}
checked={values.agreedToReceiveEmails}
/>
) : (
<Spacer y={2} />
)}
</Flex>

<Button
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { fireEvent, screen, waitFor } from "@testing-library/react-native"
import { useCountryCode } from "app/Scenes/Onboarding/Auth2/hooks/useCountryCode"
import { SignUpNameStep } from "app/Scenes/Onboarding/Auth2/scenes/SignUpNameStep"
import { GlobalStore } from "app/store/GlobalStore"
import { renderWithWrappers } from "app/utils/tests/renderWithWrappers"

jest.mock("@artsy/palette-mobile", () => ({
...jest.requireActual("@artsy/palette-mobile"),
useTheme: jest.fn().mockReturnValue({
color: jest.fn(),
}),
}))

const mockUseCountryCode = useCountryCode as jest.Mock

jest.mock("app/Scenes/Onboarding/Auth2/hooks/useCountryCode", () => ({
useCountryCode: jest.fn().mockReturnValue({
loading: false,
isAutomaticallySubscribed: false,
}),
}))

jest.mock("app/Scenes/Onboarding/Auth2/hooks/useAuthNavigation", () => ({
useAuthNavigation: jest.fn(),
useAuthScreen: jest.fn().mockReturnValue({
currentScreen: "SignUpNameStep",
}),
}))

jest.mock("app/store/GlobalStore", () => ({
...jest.requireActual("app/store/GlobalStore"),
GlobalStore: {
...jest.requireActual("app/store/GlobalStore").GlobalStore,
actions: {
auth: {
signUp: jest.fn(),
},
},
},
}))

describe("SignUpNameStep", () => {
it("renders the full name input", () => {
renderWithWrappers(<SignUpNameStep />)

expect(screen.getByA11yHint("Enter your full name")).toBeTruthy()
})

it("renders the terms and privacy policy checkbox", () => {
renderWithWrappers(<SignUpNameStep />)

expect(
screen.getByA11yHint("Check this element to accept Artsy's terms and privacy policy")
).toBeTruthy()
})

it("renders the email subscription checkbox", () => {
renderWithWrappers(<SignUpNameStep />)

expect(screen.getByA11yHint("Check this element to receive Artsy's emails")).toBeTruthy()
})

describe("user is automatically subscribed", () => {
beforeEach(() => {
mockUseCountryCode.mockReturnValue({
isAutomaticallySubscribed: true,
})
})

afterEach(() => {
jest.clearAllMocks()
})

it("does not render the email opt-in checkbox if the user is automatically subscribed", () => {
renderWithWrappers(<SignUpNameStep />)

expect(screen.queryByA11yHint("Check this element to receive Artsy's emails")).toBeNull()
})

it("agrees to receive emails if the user is automatically ", async () => {
renderWithWrappers(<SignUpNameStep />)

fireEvent.changeText(screen.getByA11yHint("Enter your full name"), "Percy Cat")
fireEvent.press(
screen.getByA11yHint("Check this element to accept Artsy's terms and privacy policy")
)

fireEvent.press(screen.getByText("Continue"))

await waitFor(() => {
expect(GlobalStore.actions.auth.signUp).toHaveBeenCalledWith(
expect.objectContaining({
agreedToReceiveEmails: true,
})
)
})
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,21 @@ export const EmailSubscriptionCheckbox: React.FC<EmailSubscriptionCheckboxProps>
return (
<Touchable haptic onPress={() => setChecked(!checked)}>
<Flex my={2} flexDirection="row" alignItems="flex-start">
<Checkbox error={error} checked={checked} onPress={() => setChecked(!checked)} mt={0.5}>
<Checkbox
error={error}
checked={checked}
onPress={() => setChecked(!checked)}
mt={0.5}
checkboxAccessibilityProps={{
accessible: true,
accessibilityRole: "checkbox",
accessibilityLabel: "Agree to receive Artsy's emails",
accessibilityHint: "Check this element to receive Artsy's emails",
accessibilityState: {
checked,
},
}}
>
{signupLoginFusionEnabled ? (
<Text variant="xs">
Get Artsy's emails on the art market, products, services, editorial, and promotional
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,12 @@ export const TermsOfServiceCheckbox: React.FC<TermsOfServiceCheckboxProps> = ({
checkboxAccessibilityProps={{
accessible: true,
accessibilityRole: "checkbox",
accessibilityLabel: "checkbox of consent",
accessibilityHint: "Check this element to consent with the Terms of Service",
accessibilityLabel: "Accept terms and privacy policy",
accessibilityHint: "Check this element to accept Artsy's terms and privacy policy",
accessibilityState: {
checked,
},
}}
testID="termsCheckbox"
>
{showNewDisclaimer ? (
<Text variant="xs" testID="disclaimer">
Expand Down