Skip to content

fix: hide privacy policy toast during onboarding, and for onboarded users#25217

Merged
jonybur merged 23 commits intodevelopfrom
jb-privacy-policy-hotfix
Jun 12, 2024
Merged

fix: hide privacy policy toast during onboarding, and for onboarded users#25217
jonybur merged 23 commits intodevelopfrom
jb-privacy-policy-hotfix

Conversation

@jonybur
Copy link
Copy Markdown
Contributor

@jonybur jonybur commented Jun 11, 2024

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

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

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.

@jonybur jonybur requested a review from a team as a code owner June 11, 2024 13:17
@github-actions
Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@jonybur jonybur added the team-core-extension-ux Core Extension UX team label Jun 11, 2024
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 12, 2024

Codecov Report

Attention: Patch coverage is 52.63158% with 9 lines in your changes missing coverage. Please review.

Project coverage is 65.66%. Comparing base (32af889) to head (0268bea).
Report is 5 commits behind head on develop.

Files Patch % Lines
ui/selectors/selectors.js 50.00% 4 Missing ⚠️
app/scripts/controllers/app-state.js 0.00% 3 Missing ⚠️
ui/store/actions.ts 33.33% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #25217      +/-   ##
===========================================
+ Coverage    65.65%   65.66%   +0.01%     
===========================================
  Files         1368     1368              
  Lines        54271    54301      +30     
  Branches     14192    14200       +8     
===========================================
+ Hits         35627    35652      +25     
- Misses       18644    18649       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [0268bea]
Page Load Metrics (552 ± 421 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint71147922010
domContentLoaded8261252
load402345552877421
domInteractive8261252
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 138 Bytes (0.00%)
  • ui: 62 Bytes (0.00%)
  • common: 220 Bytes (0.00%)

@salimtb
Copy link
Copy Markdown
Contributor

salimtb commented Jun 12, 2024

Great work ! is it posible to add an e2e test for this ?

const isFromReminder = new URLSearchParams(search).get('isFromReminder');
const trackEvent = useContext(MetaMetricsContext);

useEffect(() => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we add a comment on the top of this use effect to explain why we have it here ?

};
}

export function setOnboardingDate() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is it possible to add a unit test for this function on the action.test.js ? should be sample but better for the test coverage

@jonybur
Copy link
Copy Markdown
Contributor Author

jonybur commented Jun 12, 2024

@salimtb If this needs to go in today as a hotfix, I don't think we have enough time to add the tests you are asking. We can add them on a later date if you wish. I'd like @darkwing to confirm the urgency of this PR.

@bergeron bergeron self-requested a review June 12, 2024 16:53
@bergeron
Copy link
Copy Markdown
Contributor

bergeron commented Jun 12, 2024

I haven't seen a toast appear yet when following the steps in New user onboarding before the date:

Edit: Maybe i just need to pick a better dummy date to satisfy currentDate >= newPrivacyPolicyDate. Going past June 18 wouldn't do so.

Copy link
Copy Markdown
Contributor

@bergeron bergeron left a comment

Choose a reason for hiding this comment

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

Ok with some minor code edits i think ive reproduced all the cases correctly

@jonybur jonybur merged commit 7e24e8f into develop Jun 12, 2024
@jonybur jonybur deleted the jb-privacy-policy-hotfix branch June 12, 2024 17:53
@github-actions github-actions bot locked and limited conversation to collaborators Jun 12, 2024
@metamaskbot metamaskbot added the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 12, 2024
@hesterbruikman hesterbruikman removed the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 12, 2024
@metamaskbot metamaskbot added the release-12.2.0 Issue or pull request that will be included in release 12.2.0 label Aug 19, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator

No release label on PR. Adding release label release-12.2.0 on PR, as PR was added to branch 12.2.0 when release was cut.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-12.2.0 Issue or pull request that will be included in release 12.2.0 team-core-extension-ux Core Extension UX team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants