From 3464aea966b76557a6f9b6fabf2221c2d1b68c31 Mon Sep 17 00:00:00 2001 From: Jony Bursztyn Date: Thu, 13 Jun 2024 18:56:22 +0100 Subject: [PATCH] fix (cherry-pick) : hide privacy policy toast during onboarding, and for onboarded users (#25303) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Fixes two bugs: - Privacy policy toast should not be seen during onboarding - Users who onboarded after the privacy policy date should not see the new privacy policy toast. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25217?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** New user onbaording after the date: 1. Set the privacy policy date to a date in the future from today 2. Onboard (create a new user) 3. User should not see the privacy policy toast at any moment New user onboarding before the date: 1. Set the privacy policy date to a date in the past from today. 2. Onboard (create a new user) 3. User should not see the privacy policy toast at any moment 4. Set the privacy policy date to a date in the future from today 5. User should not see the privacy policy toast on the home screen Old user after the privacy policy date: 1. Have a user already onboarded 2. Make sure the `onboardingDate` is `null` in the redux state (this simulates a user who has registered before this PR has been in production) 3. Set the privacy policy date to a date in the future from today 4. User should not see the new privacy policy toast Old user before the privacy policy date: 1. Have a user onboarded 2. Make sure the `onboardingDate` is `null` in the redux state (this simulates a user who has registered before this PR has been in production) 3. Set the privacy policy date to a date in the past before today 4. User should see the new privacy policy toast TL;DR: If the privacy policy date is in the future, the user should never see it -BUT- if the privacy policy is in the past, the user should see it, as long as the user onboarded before the privacy policy date. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. Co-authored-by: David Walsh Co-authored-by: Brian Bergeron Co-authored-by: NidhiKJha Co-authored-by: Nidhi Kumari --- app/scripts/controllers/app-state.js | 7 +++ app/scripts/lib/setupSentry.js | 1 + app/scripts/metamask-controller.js | 2 + ...rs-after-init-opt-in-background-state.json | 1 + .../errors-after-init-opt-in-ui-state.json | 1 + ui/pages/onboarding-flow/onboarding-flow.js | 5 ++ .../onboarding-flow/onboarding-flow.test.js | 1 + ui/pages/routes/routes.component.js | 8 ++- ui/pages/routes/routes.component.test.js | 35 +++++++++++- ui/selectors/selectors.js | 14 ++++- ui/selectors/selectors.test.js | 55 +++++++++++++++++-- ui/store/actions.ts | 6 ++ 12 files changed, 124 insertions(+), 12 deletions(-) diff --git a/app/scripts/controllers/app-state.js b/app/scripts/controllers/app-state.js index 731a9d0d25e4..256dfba38438 100644 --- a/app/scripts/controllers/app-state.js +++ b/app/scripts/controllers/app-state.js @@ -52,6 +52,7 @@ export default class AppStateController extends EventEmitter { showAccountBanner: true, trezorModel: null, currentPopupId: undefined, + onboardingDate: null, newPrivacyPolicyToastClickedOrClosed: null, newPrivacyPolicyToastShownDate: null, // This key is only used for checking if the user had set advancedGasFee @@ -186,6 +187,12 @@ export default class AppStateController extends EventEmitter { }); } + setOnboardingDate() { + this.store.updateState({ + onboardingDate: Date.now(), + }); + } + setNewPrivacyPolicyToastClickedOrClosed() { this.store.updateState({ newPrivacyPolicyToastClickedOrClosed: true, diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index 68289d0fffaf..acefee2d2825 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -86,6 +86,7 @@ export const SENTRY_BACKGROUND_STATE = { browserEnvironment: true, connectedStatusPopoverHasBeenShown: true, currentPopupId: false, + onboardingDate: false, currentExtensionPopupId: false, defaultHomeActiveTabName: true, fullScreenGasPollTokens: true, diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index d25c0e15e589..b283e96dca30 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -3184,6 +3184,8 @@ export default class MetamaskController extends EventEmitter { appStateController.setSurveyLinkLastClickedOrClosed.bind( appStateController, ), + setOnboardingDate: + appStateController.setOnboardingDate.bind(appStateController), setNewPrivacyPolicyToastClickedOrClosed: appStateController.setNewPrivacyPolicyToastClickedOrClosed.bind( appStateController, diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json index f1f60ae568ed..cd57263d9184 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json @@ -37,6 +37,7 @@ "showNetworkBanner": true, "showAccountBanner": true, "trezorModel": null, + "onboardingDate": "object", "hadAdvancedGasFeesSetPriorToMigration92_3": false, "nftsDropdownState": {}, "termsOfUseLastAgreed": "number", diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json index be4d78846a6b..d64cfbaad9bb 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json @@ -72,6 +72,7 @@ "showNetworkBanner": true, "showAccountBanner": true, "trezorModel": null, + "onboardingDate": "object", "hadAdvancedGasFeesSetPriorToMigration92_3": false, "nftsDropdownState": {}, "termsOfUseLastAgreed": "number", diff --git a/ui/pages/onboarding-flow/onboarding-flow.js b/ui/pages/onboarding-flow/onboarding-flow.js index 180d6cd68c70..01ea177eb851 100644 --- a/ui/pages/onboarding-flow/onboarding-flow.js +++ b/ui/pages/onboarding-flow/onboarding-flow.js @@ -28,6 +28,7 @@ import { createNewVaultAndGetSeedPhrase, unlockAndGetSeedPhrase, createNewVaultAndRestore, + setOnboardingDate, } from '../../store/actions'; import { getFirstTimeFlowTypeRouteAfterUnlock } from '../../selectors'; import { MetaMetricsContext } from '../../contexts/metametrics'; @@ -70,6 +71,10 @@ export default function OnboardingFlow() { const isFromReminder = new URLSearchParams(search).get('isFromReminder'); const trackEvent = useContext(MetaMetricsContext); + useEffect(() => { + dispatch(setOnboardingDate()); + }, [dispatch]); + useEffect(() => { if (completedOnboarding && !isFromReminder) { history.push(DEFAULT_ROUTE); diff --git a/ui/pages/onboarding-flow/onboarding-flow.test.js b/ui/pages/onboarding-flow/onboarding-flow.test.js index 460d4bf4c9ce..cc471f5febd5 100644 --- a/ui/pages/onboarding-flow/onboarding-flow.test.js +++ b/ui/pages/onboarding-flow/onboarding-flow.test.js @@ -30,6 +30,7 @@ jest.mock('../../store/actions', () => ({ createNewVaultAndGetSeedPhrase: jest.fn().mockResolvedValue(null), unlockAndGetSeedPhrase: jest.fn().mockResolvedValue(null), createNewVaultAndRestore: jest.fn(), + setOnboardingDate: jest.fn(() => ({ type: 'TEST_DISPATCH' })), })); describe('Onboarding Flow', () => { diff --git a/ui/pages/routes/routes.component.js b/ui/pages/routes/routes.component.js index 71364c46c92e..ad1e99946c15 100644 --- a/ui/pages/routes/routes.component.js +++ b/ui/pages/routes/routes.component.js @@ -658,11 +658,13 @@ export default class Routes extends Component { const isPrivacyToastRecent = this.getIsPrivacyToastRecent(); const isPrivacyToastNotShown = !newPrivacyPolicyToastShownDate; + if (!this.onHomeScreen()) { + return null; + } + return ( - {showConnectAccountToast && - this.onHomeScreen() && - !this.state.hideConnectAccountToast ? ( + {showConnectAccountToast && !this.state.hideConnectAccountToast ? ( { }); }); }); + +describe('toast display', () => { + const testState = { + ...mockState, + metamask: { + ...mockState.metamask, + announcements: {}, + approvalFlows: [], + completedOnboarding: true, + usedNetworks: [], + swapsState: { swapsFeatureIsLive: true }, + }, + }; + + it('renders toastContainer on default route', async () => { + await render([DEFAULT_ROUTE], testState); + const toastContainer = document.querySelector('.toasts-container'); + expect(toastContainer).toBeInTheDocument(); + }); + + it('does not render toastContainer on confirmation route', async () => { + await render([CONFIRMATION_V_NEXT_ROUTE], testState); + const toastContainer = document.querySelector('.toasts-container'); + expect(toastContainer).not.toBeInTheDocument(); + }); +}); diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 2d2ebdc1d6c3..ec17e426757c 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -1878,11 +1878,17 @@ export function getShowSurveyToast(state) { * @returns {boolean} True if the current date is on or after the new privacy policy date and the privacy policy toast was not clicked or closed. False otherwise. */ export function getShowPrivacyPolicyToast(state) { - const { newPrivacyPolicyToastClickedOrClosed } = state.metamask; + const { newPrivacyPolicyToastClickedOrClosed, onboardingDate } = + state.metamask; const newPrivacyPolicyDate = new Date(PRIVACY_POLICY_DATE); const currentDate = new Date(Date.now()); return ( - !newPrivacyPolicyToastClickedOrClosed && currentDate >= newPrivacyPolicyDate + !newPrivacyPolicyToastClickedOrClosed && + currentDate >= newPrivacyPolicyDate && + // users who onboarded before the privacy policy date should see the notice + // and + // old users who don't have onboardingDate set should see the notice + (onboardingDate < newPrivacyPolicyDate || !onboardingDate) ); } @@ -1899,6 +1905,10 @@ export function getNewPrivacyPolicyToastShownDate(state) { return state.metamask.newPrivacyPolicyToastShownDate; } +export function getOnboardingDate(state) { + return state.metamask.onboardingDate; +} + export function getShowBetaHeader(state) { return state.metamask.showBetaHeader; } diff --git a/ui/selectors/selectors.test.js b/ui/selectors/selectors.test.js index 2cb63385a27f..f272c064f538 100644 --- a/ui/selectors/selectors.test.js +++ b/ui/selectors/selectors.test.js @@ -1259,30 +1259,46 @@ describe('Selectors', () => { dateNowSpy = jest .spyOn(Date, 'now') - .mockReturnValue(dayAfterPolicyDate); + .mockReturnValue(dayAfterPolicyDate.getTime()); }); afterEach(() => { dateNowSpy.mockRestore(); }); - it('shows the privacy policy toast when not yet seen and on or after the policy date', () => { + it('shows the privacy policy toast when not yet seen, on or after the policy date, and onboardingDate is before the policy date', () => { const result = selectors.getShowPrivacyPolicyToast({ metamask: { newPrivacyPolicyToastClickedOrClosed: null, + onboardingDate: new Date(PRIVACY_POLICY_DATE).setDate( + new Date(PRIVACY_POLICY_DATE).getDate() - 2, + ), }, }); expect(result).toBe(true); }); - it('does not show the privacy policy toast when seen and on or after the policy date', () => { + it('does not show the privacy policy toast when seen, even if on or after the policy date and onboardingDate is before the policy date', () => { const result = selectors.getShowPrivacyPolicyToast({ metamask: { newPrivacyPolicyToastClickedOrClosed: true, + onboardingDate: new Date(PRIVACY_POLICY_DATE).setDate( + new Date(PRIVACY_POLICY_DATE).getDate() - 2, + ), }, }); expect(result).toBe(false); }); + + it('shows the privacy policy toast when not yet seen, on or after the policy date, and onboardingDate is not set', () => { + const result = selectors.getShowPrivacyPolicyToast({ + metamask: { + newPrivacyPolicyToastClickedOrClosed: null, + onboardingDate: null, + }, + }); + expect(result).toBe(true); + }); }); describe('mock same day', () => { @@ -1296,23 +1312,39 @@ describe('Selectors', () => { dateNowSpy.mockRestore(); }); - it('shows the privacy policy toast when not yet seen and on or after the policy date', () => { + it('shows the privacy policy toast when not yet seen, on or after the policy date, and onboardingDate is before the policy date', () => { const result = selectors.getShowPrivacyPolicyToast({ metamask: { newPrivacyPolicyToastClickedOrClosed: null, + onboardingDate: new Date(PRIVACY_POLICY_DATE).setDate( + new Date(PRIVACY_POLICY_DATE).getDate() - 2, + ), }, }); expect(result).toBe(true); }); - it('does not show the privacy policy toast when seen and on or after the policy date', () => { + it('does not show the privacy policy toast when seen, even if on or after the policy date and onboardingDate is before the policy date', () => { const result = selectors.getShowPrivacyPolicyToast({ metamask: { newPrivacyPolicyToastClickedOrClosed: true, + onboardingDate: new Date(PRIVACY_POLICY_DATE).setDate( + new Date(PRIVACY_POLICY_DATE).getDate() - 2, + ), }, }); expect(result).toBe(false); }); + + it('shows the privacy policy toast when not yet seen, on or after the policy date, and onboardingDate is not set', () => { + const result = selectors.getShowPrivacyPolicyToast({ + metamask: { + newPrivacyPolicyToastClickedOrClosed: null, + onboardingDate: null, + }, + }); + expect(result).toBe(true); + }); }); describe('mock day before', () => { @@ -1333,6 +1365,19 @@ describe('Selectors', () => { const result = selectors.getShowPrivacyPolicyToast({ metamask: { newPrivacyPolicyToastClickedOrClosed: null, + onboardingDate: new Date(PRIVACY_POLICY_DATE).setDate( + new Date(PRIVACY_POLICY_DATE).getDate() - 2, + ), + }, + }); + expect(result).toBe(false); + }); + + it('does not show the privacy policy toast before the policy date even if onboardingDate is not set', () => { + const result = selectors.getShowPrivacyPolicyToast({ + metamask: { + newPrivacyPolicyToastClickedOrClosed: null, + onboardingDate: null, }, }); expect(result).toBe(false); diff --git a/ui/store/actions.ts b/ui/store/actions.ts index 90ad6bcddce6..08ad588b8b30 100644 --- a/ui/store/actions.ts +++ b/ui/store/actions.ts @@ -4231,6 +4231,12 @@ export function setNewPrivacyPolicyToastClickedOrClosed() { }; } +export function setOnboardingDate() { + return async () => { + await submitRequestToBackground('setOnboardingDate'); + }; +} + export function setNewPrivacyPolicyToastShownDate(time: number) { return async () => { await submitRequestToBackground('setNewPrivacyPolicyToastShownDate', [