Check attachment file type before checking size

[yuwenmemon]

Details

For some file types (like Applications in OSX) the Drag and Drop file that's given to a browser is different than what's given by a file picker (see example here) - however the common disqualifier is that the file type is not supported anyway. So, instead of checking for filesize first, make sure an attachment is of the right file type.

Fixed Issues

$ #13768

Tests

Desktop/Web only - (OSX - Mac)

1. Login with any account.
2. Choose any report, drag and drop an Application file onto that report (OSX, desktop/web only)
3. Make sure that the error message you get complains about the file format of what you uploaded rather than the size.

• Verify that no errors appear in the JS console

Offline tests

• Same steps as above, but offline.

QA Steps

• Perform the tests in the Tests/Offline tests sections above.
• 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
  • 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

Kapture.2023-01-09.at.16.47.35.mp4

Mobile Web - Chrome

• N/A

Mobile Web - Safari

• N/A

Desktop

Kapture.2023-01-09.at.16.48.56.mp4

iOS

• N/A

Android

• N/A

[melvin-bot]

@Santhosh-Sellavel @marcochavezf 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]

[Santhosh-Sellavel]

@yuwenmemon Can you update the test steps and lets us know what to test here?

3. Notice that the validation message says: "Attachment size must be greater than 240 bytes" even the size of attachment is 6MB.

This doesn't make any sense to me.

[yuwenmemon]

@Santhosh-Sellavel whoops! Copy-🍝 on my end. Updated!

[Santhosh-Sellavel]

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 & Desktop

Tests.mov

[Santhosh-Sellavel]

LGTM tests well!

@marcochavezf all you!

[marcochavezf]

LGTM 👍🏽

[OSBotify]

✋ 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]

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	656.484 ms → 677.812 ms (+21.328 ms, +3.2%)
App start runJsBundle	182.406 ms → 193.969 ms (+11.563 ms, +6.3%)
App start nativeLaunch	9.935 ms → 19.036 ms (+9.100 ms, +91.6%) 🟡
App start regularAppStart	0.013 ms → 0.022 ms (+0.009 ms, +68.6%) 🟡
Open Search Page TTI	612.545 ms → 602.640 ms (-9.906 ms, -1.6%)

Show details

Name	Duration
App start TTI	Baseline Mean: 656.484 ms Stdev: 35.878 ms (5.5%) Runs: 597.0566620007157 610.8669290002435 612.8290519993752 617.2901629991829 620.2421410009265 622.4882800001651 623.5527030006051 623.9294990003109 624.2780249994248 629.7236299999058 633.6676310002804 635.2887810003012 636.7366599999368 640.3975879997015 643.2843290008605 653.7949139997363 658.4425760004669 658.4855889994651 664.1270129997283 665.3994810003787 671.357783999294 673.4068310000002 674.7388269994408 680.3849059995264 682.1256189998239 682.3950200006366 683.3018629997969 691.00877700001 710.6315640006214 718.0398670006543 719.9181949999183 748.2871029991657 Current Mean: 677.812 ms Stdev: 25.609 ms (3.8%) Runs: 627.5030540004373 633.0186149999499 636.2682949993759 643.9123880006373 647.0645300000906 648.1763330008835 651.0811839997768 651.7344959992915 656.6000849995762 669.4978730008006 674.6688289996237 675.1823360007256 676.2427919991314 678.1691670008004 678.5807409994304 680.1638510003686 682.5307310000062 683.7238679993898 684.1555959992111 684.290748000145 690.6349209994078 691.219918999821 693.6581130009145 699.3499500006437 701.7629659995437 702.2331830002367 706.7675100006163 710.8905120007694 711.9008189998567 716.6020519994199 724.5845899991691
App start runJsBundle	Baseline Mean: 182.406 ms Stdev: 21.655 ms (11.9%) Runs: 147 150 151 153 161 164 164 165 165 167 169 170 171 176 177 177 181 182 186 187 192 196 197 197 199 201 206 206 211 219 220 230 Current Mean: 193.969 ms Stdev: 22.969 ms (11.8%) Runs: 153 163 164 165 165 168 172 172 180 182 183 183 185 185 186 187 188 195 196 203 206 210 211 211 211 215 219 223 223 229 230 244
App start nativeLaunch	Baseline Mean: 9.935 ms Stdev: 2.078 ms (20.9%) Runs: 7 8 8 8 8 8 8 8 8 8 9 9 9 9 9 9 9 10 10 10 10 11 11 12 12 12 13 13 13 14 15 Current Mean: 19.036 ms Stdev: 0.906 ms (4.8%) Runs: 18 18 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
App start regularAppStart	Baseline Mean: 0.013 ms Stdev: 0.001 ms (6.1%) Runs: 0.011841000989079475 0.011962998658418655 0.012246999889612198 0.01228799857199192 0.01228800043463707 0.01228800043463707 0.012368999421596527 0.01245100051164627 0.01245100051164627 0.012491999194025993 0.012492001056671143 0.012492001056671143 0.012532999739050865 0.012614000588655472 0.012655001133680344 0.012817000970244408 0.01285800151526928 0.012897999957203865 0.012938998639583588 0.013020999729633331 0.01314299926161766 0.013265000656247139 0.013346001505851746 0.013467999175190926 0.01371300034224987 0.013793999329209328 0.013915998861193657 0.013956999406218529 0.01407800056040287 0.014364000409841537 0.015461999922990799 Current Mean: 0.022 ms Stdev: 0.003 ms (14.7%) Runs: 0.017904000356793404 0.018838999792933464 0.01908400095999241 0.019612999632954597 0.01973399892449379 0.019735001027584076 0.019735001027584076 0.019897999241948128 0.019938001409173012 0.01993900164961815 0.019979000091552734 0.020263999700546265 0.02034500055015087 0.020508000627160072 0.02058900147676468 0.020590001717209816 0.0206300001591444 0.020914999768137932 0.020955998450517654 0.02128100022673607 0.021890999749302864 0.02225799858570099 0.023274000734090805 0.023314999416470528 0.024007000029087067 0.025919999927282333 0.027993999421596527 0.028239000588655472 0.028686000034213066 0.03080200031399727
Open Search Page TTI	Baseline Mean: 612.545 ms Stdev: 25.086 ms (4.1%) Runs: 576.0808509998024 576.3590089995414 580.261758999899 588.0757650006562 588.1728520002216 591.2860519997776 591.3176279999316 591.9260260015726 593.4900719989091 593.5796709991992 594.0309649985284 594.987549001351 597.2644859999418 600.6490480005741 606.1385909989476 607.4063310008496 608.9866540003568 609.4486900009215 609.7635090015829 610.6011560000479 617.9759529996663 624.3988450001925 625.2725419998169 628.9981280006468 630.3965250011533 631.4979649987072 632.8473719991744 634.5919189993292 634.7075200006366 639.7248130012304 660.8546549994498 667.4359540008008 675.4661050010473 Current Mean: 602.640 ms Stdev: 21.366 ms (3.5%) Runs: 562.6775709986687 576.3986409995705 579.2769779991359 579.5351969990879 579.9022619985044 580.1209319997579 584.5041099991649 586.4414060004056 588.8063960000873 588.9010830000043 589.8908289987594 589.9212650004774 593.64408400096 594.1426999997348 595.1593429986387 595.9118250012398 595.9706219993532 602.2464199997485 603.26293999888 605.387328999117 605.6510420013219 608.2019459996372 610.375284999609 611.3551439996809 612.844360999763 624.7422279994935 626.1287849992514 627.8681230004877 627.8988850004971 628.0453699994832 629.601197000593 637.0049239993095 665.2888590004295

[OSBotify]

🚀 Deployed to staging by @marcochavezf in version: 1.2.53-0 🚀

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

[OSBotify]

🚀 Deployed to production by @Julesssss in version: 1.2.53-0 🚀

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