Disable option menu when pending action is delete for bank account #15180

daraksha-dk · 2023-02-15T18:19:10Z

Details

#14912 (comment)

Fixed Issues

$ #14912
PROPOSAL: #14912 (comment)

Tests

Go to this URL : https://staging.new.expensify.com/
Log in with any account
Navigate to Settings > Payments
Click on Add Payment Method > Bank Account ( use Chase bank with credentials: user_good, pass_good)
Disable the internet connection
Verify that you can't delete the bank account more than once
Enable the internet connection and verify that you don't have any errors

Verify that no errors appear in the JS console

Offline tests

Go to this URL : https://staging.new.expensify.com/
Log in with any account
Navigate to Settings > Payments
Click on Add Payment Method > Bank Account ( use Chase bank with credentials: user_good, pass_good)
Disable the internet connection
Verify that you can't delete the bank account more than once
Enable the internet connection and verify that you don't have any errors

QA Steps

Go to this URL : https://staging.new.expensify.com/
Log in with any account
Navigate to Settings > Payments
Click on Add Payment Method > Bank Account ( use Chase bank with credentials: user_good, pass_good)
Disable the internet connection
Verify that you can't delete the bank account more than once
Enable the internet connection and verify that you don't have any errors

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

Screen.Recording.2023-02-16.at.1.18.10.PM.mov

Mobile Web - Chrome

Screen.Recording.2023-02-16.at.2.00.05.PM.mov

Mobile Web - Safari

Screen.Recording.2023-02-16.at.1.39.04.PM.mov

Desktop

Screen.Recording.2023-02-16.at.1.14.41.PM.mov

iOS

Screen.Recording.2023-02-17.at.3.42.01.AM.mov

Android

Screen.Recording.2023-02-16.at.1.23.06.PM.mov

daraksha-dk · 2023-02-16T08:57:03Z

I am having some problems with ios simulator, I will attach the video for ios once it gets resolved.

MelvinBot · 2023-02-16T08:58:20Z

@Julesssss @sobitneupane 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]

sobitneupane

Screenshots/Videos

Web

Screen.Recording.2023-02-16.at.17.45.42.mov

Mobile Web - Chrome

Screen.Recording.2023-02-16.at.18.01.38.mov

Mobile Web - Safari

Screen.Recording.2023-02-16.at.18.04.57.mov

Desktop

Screen.Recording.2023-02-16.at.17.55.46.mov

iOS

Screen.Recording.2023-02-16.at.18.03.30.mov

Android

Screen.Recording.2023-02-16.at.17.58.43.mov

sobitneupane · 2023-02-16T12:28:27Z

@Julesssss I noticed following issue. I don't think it is directly related to this one. Want to know your thought.

Open app in two different devices (or different tab).
Go offline in one of the device (say device A).
Delete a payment method from device B.
Delete payment method from device A.
Go online in device A.
You will see following error which I think is expected. But the deleted bank account remains even after refreshing the page. User have to logout and login again to get rid of the deleted bank account.

Julesssss · 2023-02-16T13:11:40Z

@sobitneupane nice find. I agree that seems like a bug, do you want to report in in Slack as a separate issue?

Julesssss · 2023-02-16T14:30:36Z

We just need the checklist now., then we're good to go.

sobitneupane · 2023-02-16T15:18:18Z

Reported the issue in slack.

sobitneupane

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.

sobitneupane · 2023-02-16T15:24:50Z

@daraksha-dk You have not added screenshots/videos for iOS yet. Other than that, changes look good and tests wel..

daraksha-dk · 2023-02-16T22:14:24Z

Just added ios as well.

sobitneupane

Changes look good and tests well.

🎀👀🎀 C+ reviewed
cc: @Julesssss

OSBotify · 2023-02-17T11:09:30Z

✋ 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 · 2023-02-17T11:38:23Z

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	728.262 ms → 761.421 ms (+33.159 ms, +4.6%)
App start runJsBundle	201.063 ms → 212.063 ms (+11.000 ms, +5.5%)
App start nativeLaunch	19.966 ms → 20.290 ms (+0.325 ms, +1.6%)
App start regularAppStart	0.017 ms → 0.016 ms (-0.001 ms, -3.6%)
Open Search Page TTI	624.367 ms → 624.345 ms (-0.021 ms, ±0.0%)

Show details

Name	Duration
App start TTI	Baseline Mean: 728.262 ms Stdev: 29.484 ms (4.0%) Runs: 667.1337269991636 684.5169159993529 685.1182239986956 696.2307390011847 696.3290789984167 696.9632290005684 698.0338179990649 699.8803590014577 703.8837490007281 712.537893999368 714.1322759985924 714.4106540009379 716.2260960005224 718.1227109991014 718.4107029996812 729.8036530017853 730.2145330011845 736.9452250003815 737.4947829991579 739.6240059994161 739.952213998884 742.8530539982021 746.4869339987636 751.5173400007188 752.0046010017395 756.5894120000303 758.7414389997721 760.2812479995191 760.9594799987972 770.6441529989243 780.368407998234 787.9679980017245 Current Mean: 761.421 ms Stdev: 29.802 ms (3.9%) Runs: 698.7835440002382 707.7165010012686 724.5196780003607 726.6827279999852 730.7678500004113 731.8600489981472 732.7457639984787 739.2282020002604 741.4151629991829 747.5731259994209 750.8967089988291 755.9777269996703 756.1645010001957 756.2581349983811 757.1412050016224 762.3636550009251 764.6551930010319 765.0117040015757 765.4525289982557 769.8873689994216 774.5996050015092 775.8530620001256 778.4929930008948 781.044971998781 783.6299510002136 785.4046600013971 786.3353450000286 793.1605420000851 818.7181029990315 819.0215190015733 822.6970929987729
App start runJsBundle	Baseline Mean: 201.063 ms Stdev: 25.912 ms (12.9%) Runs: 157 172 174 174 176 177 180 181 181 182 184 185 191 192 192 194 196 197 198 198 206 207 209 222 223 225 229 229 238 244 256 265 Current Mean: 212.063 ms Stdev: 21.789 ms (10.3%) Runs: 179 180 181 181 185 192 193 197 198 198 201 204 204 205 206 209 210 211 215 221 221 221 222 223 225 228 232 237 240 247 251 269
App start nativeLaunch	Baseline Mean: 19.966 ms Stdev: 1.671 ms (8.4%) Runs: 17 17 18 18 18 18 19 19 19 19 19 19 20 20 20 20 20 20 21 21 21 21 21 22 22 22 22 22 24 Current Mean: 20.290 ms Stdev: 1.800 ms (8.9%) Runs: 18 18 18 18 18 18 18 19 19 19 19 19 20 20 20 20 20 21 21 21 21 22 22 22 22 22 22 22 22 23 25
App start regularAppStart	Baseline Mean: 0.017 ms Stdev: 0.001 ms (7.1%) Runs: 0.014364000409841537 0.01534000039100647 0.015462998300790787 0.015666000545024872 0.015869002789258957 0.015950996428728104 0.015951000154018402 0.015991002321243286 0.016112998127937317 0.016113001853227615 0.01615399867296219 0.01615399867296219 0.01635799929499626 0.016397997736930847 0.01648000255227089 0.016560997813940048 0.016600999981164932 0.016764000058174133 0.016927000135183334 0.01700800284743309 0.017048999667167664 0.01749599725008011 0.017618998885154724 0.017740998417139053 0.017741002142429352 0.017862997949123383 0.018350999802350998 0.01908300071954727 0.019491001963615417 0.01953200250864029 Current Mean: 0.016 ms Stdev: 0.001 ms (6.2%) Runs: 0.014485999941825867 0.014811001718044281 0.01493300125002861 0.014974001795053482 0.014974001795053482 0.015137001872062683 0.01521800085902214 0.015258997678756714 0.01550300046801567 0.015705998986959457 0.01574699953198433 0.015828002244234085 0.01603199914097786 0.01607300341129303 0.016113001853227615 0.016316000372171402 0.01631699874997139 0.016357000917196274 0.016398001462221146 0.01647999882698059 0.016520999372005463 0.01668199896812439 0.016683001071214676 0.01672299951314926 0.016805000603199005 0.017090000212192535 0.017131000757217407 0.01753700152039528 0.017903998494148254 0.01798500120639801 0.018716998398303986
Open Search Page TTI	Baseline Mean: 624.367 ms Stdev: 28.150 ms (4.5%) Runs: 579.881714001298 582.6230879984796 588.312704000622 591.1079920008779 597.3571369983256 597.7236329987645 599.8328050002456 600.2933750003576 601.7359619997442 601.9355480000377 606.6597089990973 607.2083739973605 611.5531009994447 617.084595002234 617.2502039968967 618.46883200109 622.2622069977224 622.296509001404 623.0145669989288 626.6940919980407 626.9868580028415 628.8617760017514 631.818563003093 635.3207200020552 643.672973997891 645.5105389989913 649.6916100010276 660.5 661.4114180020988 666.9143470004201 677.4084069989622 681.1790769994259 681.5223799981177 Current Mean: 624.345 ms Stdev: 20.789 ms (3.3%) Runs: 589.3675129972398 597.9652110002935 598.4465330019593 599.2604990005493 599.5340170003474 603.6067709997296 604.391764998436 606.2674559988081 609.3888350017369 609.8552660010755 611.459310002625 616.2642010003328 616.3280849978328 621.2930499985814 623.735270999372 623.8039549998939 624.3376879990101 624.9862480014563 627.5859379991889 630.5847579985857 631.2995200008154 631.6027020029724 633.4110920019448 634.5531419999897 635.7012539990246 635.7100429981947 641.6668710000813 646.2480470016599 647.8410640023649 648.000652000308 674.8472899980843 679.6965340003371

OSBotify · 2023-02-17T21:24:00Z

🚀 Deployed to staging by https://github.com/Julesssss in version: 1.2.74-0 🚀

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

OSBotify · 2023-02-21T20:34:43Z

🚀 Deployed to production by https://github.com/chiragsalian in version: 1.2.74-0 🚀

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

daraksha-dk added 2 commits February 15, 2023 21:46

add userSelectNone style for children

95d9f3b

disable payment method when pending action is delete

8c94c3f

daraksha-dk marked this pull request as ready for review February 16, 2023 08:58

daraksha-dk requested a review from a team as a code owner February 16, 2023 08:58

melvin-bot bot requested review from Julesssss and sobitneupane and removed request for a team February 16, 2023 08:58

Julesssss assigned daraksha-dk Feb 16, 2023

sobitneupane reviewed Feb 16, 2023

View reviewed changes

sobitneupane approved these changes Feb 16, 2023

View reviewed changes

sobitneupane approved these changes Feb 17, 2023

View reviewed changes

Julesssss approved these changes Feb 17, 2023

View reviewed changes

Julesssss merged commit 1180571 into Expensify:main Feb 17, 2023

OSBotify mentioned this pull request Feb 17, 2023

Deploy Checklist: New Expensify 2023-02-17 #15260

Closed

17 tasks

MelvinBot mentioned this pull request Feb 21, 2023

[HOLD for payment 2023-02-28] [$1000] Payment - After deleting the bank account, the payment is displayed with an empty icon #14912

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable option menu when pending action is delete for bank account #15180

Disable option menu when pending action is delete for bank account #15180

daraksha-dk commented Feb 15, 2023 •

edited

Loading

daraksha-dk commented Feb 16, 2023

MelvinBot commented Feb 16, 2023

sobitneupane left a comment

sobitneupane commented Feb 16, 2023

Julesssss commented Feb 16, 2023

Julesssss commented Feb 16, 2023

sobitneupane commented Feb 16, 2023

sobitneupane left a comment •

edited

Loading

sobitneupane commented Feb 16, 2023 •

edited

Loading

daraksha-dk commented Feb 16, 2023

sobitneupane left a comment

OSBotify commented Feb 17, 2023

github-actions bot commented Feb 17, 2023

OSBotify commented Feb 17, 2023

OSBotify commented Feb 21, 2023

Disable option menu when pending action is delete for bank account #15180

Disable option menu when pending action is delete for bank account #15180

Conversation

daraksha-dk commented Feb 15, 2023 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

daraksha-dk commented Feb 16, 2023

MelvinBot commented Feb 16, 2023

sobitneupane left a comment

Choose a reason for hiding this comment

Screenshots/Videos

sobitneupane commented Feb 16, 2023

Julesssss commented Feb 16, 2023

Julesssss commented Feb 16, 2023

sobitneupane commented Feb 16, 2023

sobitneupane left a comment • edited Loading

Choose a reason for hiding this comment

Reviewer Checklist

sobitneupane commented Feb 16, 2023 • edited Loading

daraksha-dk commented Feb 16, 2023

sobitneupane left a comment

Choose a reason for hiding this comment

OSBotify commented Feb 17, 2023

github-actions bot commented Feb 17, 2023

Performance Comparison Report 📊

Significant Changes To Duration

Meaningless Changes To Duration

OSBotify commented Feb 17, 2023

OSBotify commented Feb 21, 2023

daraksha-dk commented Feb 15, 2023 •

edited

Loading

sobitneupane left a comment •

edited

Loading

sobitneupane commented Feb 16, 2023 •

edited

Loading