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

Standardize on using Privacy instead of Privacy Policy #14335

Merged
merged 4 commits into from
Jan 17, 2023

Conversation

luacmartins
Copy link
Contributor

@luacmartins luacmartins commented Jan 16, 2023

Details

We have used both Privacy and Privacy Policy across the app. This PR standardizes on the use of Privacy everywhere

Fixed Issues

$ #14311

Tests

  1. Verify that the text linking to our Privacy policy reads Privacy in the places below:
  • Login page

  • Workspace > Connect bank account > Personal information step

  • Settings > About page

  • Workspace > Manage members > Invite

  • Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

Same as test

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android / native
    • Android / Chrome
    • iOS / native
    • iOS / Safari
    • MacOS / Chrome / Safari
    • MacOS / Desktop
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
      • If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

web-about

web-invite

web-login

web

Mobile Web - Chrome

chrome-about

chrome-invite

chrome-login

chrome

Mobile Web - Safari

safari

safari-invite

safari-about

safari-login

Desktop desktop

desktop-about
desktop-invite
desktop-login

iOS

ios-about

ios
ios-invite

ios-login

Android

android

android-invite

android-about

android-login

@luacmartins luacmartins self-assigned this Jan 16, 2023
@luacmartins luacmartins marked this pull request as ready for review January 16, 2023 21:26
@luacmartins luacmartins requested a review from a team as a code owner January 16, 2023 21:26
@melvin-bot melvin-bot bot requested review from AndrewGable and thesahindia and removed request for a team January 16, 2023 21:26
@melvin-bot
Copy link

melvin-bot bot commented Jan 16, 2023

@thesahindia @AndrewGable One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@AndrewGable
Copy link
Contributor

I will let @thesahindia review first!

@thesahindia
Copy link
Member

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified tests pass on all platforms & I tested again on:
    • Android / native
    • Android / Chrome
    • iOS / native
    • iOS / Safari
    • MacOS / Chrome / Safari
    • MacOS / Desktop
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web Screenshot 2023-01-17 at 8 41 45 AM Screenshot 2023-01-17 at 8 41 30 AM Screenshot 2023-01-17 at 8 41 13 AM
Mobile Web - Chrome Screenshot 2023-01-17 at 8 45 19 AM
Mobile Web - Safari Screenshot 2023-01-17 at 8 29 46 AM
Desktop Screenshot 2023-01-17 at 8 26 39 AM Screenshot 2023-01-17 at 8 25 50 AM
iOS Screenshot 2023-01-17 at 8 39 26 AM
Android Screenshot 2023-01-17 at 8 44 57 AM

Copy link
Member

@thesahindia thesahindia left a comment

Choose a reason for hiding this comment

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

Looks good! Tests are failing but it is not related to this PR.

@luacmartins
Copy link
Contributor Author

Merged main and tests are fixed!

@AndrewGable AndrewGable merged commit 6902b94 into main Jan 17, 2023
@AndrewGable AndrewGable deleted the cmartins-usePrivacy branch January 17, 2023 19:09
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@github-actions
Copy link
Contributor

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries
Name Duration
App start TTI 668.444 ms → 681.692 ms (+13.248 ms, +2.0%)
App start runJsBundle 183.645 ms → 188.063 ms (+4.417 ms, +2.4%)
Open Search Page TTI 633.284 ms → 634.678 ms (+1.393 ms, ±0.0%)
App start nativeLaunch 19.613 ms → 20.300 ms (+0.687 ms, +3.5%)
App start regularAppStart 0.014 ms → 0.015 ms (+0.001 ms, +3.7%)
Show details
Name Duration
App start TTI Baseline
Mean: 668.444 ms
Stdev: 34.950 ms (5.2%)
Runs: 609.1861619995907 627.7940300004557 630.1878500003368 631.5376030001789 634.225937999785 634.2419649995863 635.522149999626 638.503868999891 640.6324030002579 645.7936100000516 647.8718020003289 649.4879510002211 650.771983999759 651.1472270004451 653.2791259996593 659.3365350002423 661.8343200003728 672.230000000447 677.6606529997662 681.6979750003666 684.2140319999307 686.1629769997671 688.0859489999712 692.020262000151 695.9088199995458 710.8265000004321 710.8727299999446 716.0929359998554 718.0290010003373 728.3098489996046 758.3083159998059

Current
Mean: 681.692 ms
Stdev: 33.232 ms (4.9%)
Runs: 629.4239400001243 633.5576539998874 635.3009479995817 640.9786449996755 645.1947600003332 650.6372809996828 651.2037819996476 652.6132899997756 654.820013999939 659.0362280001864 659.0860000001267 662.486082999967 668.373960999772 670.2907250002027 672.9238369995728 675.1754809999838 676.0884220004082 689.6992419995368 696.1232610000297 698.7328040003777 700.9436010001227 701.0787840001285 701.350871999748 702.274183999747 702.9002599995583 707.3920940002427 707.7105919998139 710.6697709998116 717.4539850000292 724.5419800002128 745.5046509997919 770.5846049999818
App start runJsBundle Baseline
Mean: 183.645 ms
Stdev: 17.381 ms (9.5%)
Runs: 152 157 161 163 165 165 166 170 171 175 176 178 178 179 179 180 180 182 184 188 190 196 197 203 203 205 207 208 211 211 213

Current
Mean: 188.063 ms
Stdev: 19.434 ms (10.3%)
Runs: 155 160 168 168 169 170 170 170 170 172 173 175 177 179 183 190 190 192 193 194 195 195 199 199 200 201 208 208 216 217 228 234
Open Search Page TTI Baseline
Mean: 633.284 ms
Stdev: 28.759 ms (4.5%)
Runs: 588.1413980005309 588.5311690000817 598.9343260005116 603.7532959999517 604.10705600027 604.6187340002507 612.8244230002165 612.8762619998306 614.8892419999465 615.0623779995367 617.816487999633 618.4735520007089 619.1893720002845 619.8570159999654 622.7181400004774 622.9621580000967 625.2796229999512 631.6755370004103 636.5913089998066 638.6767179993913 639.0950119998306 643.8052169997245 645.1158849997446 646.6685800002888 652.0050860000774 655.3439949993044 659.5039069997147 672.4012860003859 681.2396249994636 683.062784999609 686.793171999976 703.0906980000436

Current
Mean: 634.678 ms
Stdev: 25.003 ms (3.9%)
Runs: 596.363404000178 596.5481779994443 601.9799800002947 605.8482670001686 607.6671959999949 613.6585690006614 613.803222999908 613.866170999594 615.8871670002118 618.7939860001206 620.2084150006995 621.353637999855 622.1624360000715 624.0178629998118 624.9342040000483 625.9115400006995 626.9086100002751 629.6650799997151 631.7617600001395 635.3816330004483 639.0163580002263 639.8117270004004 645.3308920003474 645.8372400002554 652.8804120002314 659.1304120002314 665.4632970001549 667.4246420003474 669.8173019997776 671.8101409999654 672.985107999295 679.2512619998306 688.8885500002652
App start nativeLaunch Baseline
Mean: 19.613 ms
Stdev: 1.407 ms (7.2%)
Runs: 18 18 18 18 18 18 18 19 19 19 19 19 19 19 19 19 19 19 20 20 20 20 20 20 21 21 22 22 22 22 23

Current
Mean: 20.300 ms
Stdev: 1.656 ms (8.2%)
Runs: 18 18 19 19 19 19 19 19 19 19 19 19 20 20 20 20 20 20 20 20 20 21 22 22 22 22 23 23 24 24
App start regularAppStart Baseline
Mean: 0.014 ms
Stdev: 0.001 ms (5.0%)
Runs: 0.012613999657332897 0.01318300049751997 0.013346000574529171 0.013387000188231468 0.013467999175190926 0.013468999415636063 0.013508999720215797 0.01355000026524067 0.013631999492645264 0.013712999410927296 0.013793999329209328 0.013794000260531902 0.013915999792516232 0.013956000097095966 0.014159999787807465 0.014200999401509762 0.014201000332832336 0.014241000637412071 0.014281999319791794 0.014566999860107899 0.014566999860107899 0.014566999860107899 0.01460800040513277 0.01460800040513277 0.01460800040513277 0.015015000477433205 0.015422000549733639 0.015502999536693096 0.015542999841272831

Current
Mean: 0.015 ms
Stdev: 0.001 ms (4.4%)
Runs: 0.0134680001065135 0.0138349998742342 0.013915999792516232 0.013956000097095966 0.013996999710798264 0.014038000255823135 0.014078999869525433 0.01416000071913004 0.014200999401509762 0.014282000251114368 0.014322999864816666 0.014322999864816666 0.014363999478518963 0.014485999941825867 0.01460800040513277 0.014649000018835068 0.014649000018835068 0.014689000323414803 0.0147299999371171 0.014810999855399132 0.014852000400424004 0.014852000400424004 0.015095999464392662 0.01534000039100647 0.015421999618411064 0.015421999618411064 0.015828999690711498 0.015909000299870968 0.016112999990582466

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @AndrewGable in version: 1.2.56-0 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/AndrewGable in version: 1.2.56-0 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants