prevent send/request money multiple times #13733

0xmiros · 2022-12-20T17:33:17Z

Details

Use local flag (stateless) to check whether send/request event should be triggered or not

Fixed Issues

$ #12714 (comment)
PROPOSAL: #12714

Tests

Go to any chat
Click on the plus icon
Click Send money or Request Money
Enter any amount
Click Next
Press enter key multiple times on web/desktop (click Request/I'll settle up elsewhere button multiple times on mobile)

Verify that no errors appear in the JS console

Offline tests

Go to any chat
Disable network
Click on the plus icon
Click Send money or Request Money
Enter any amount
Click Next
Press enter key multiple times on web/desktop (click Request/I'll settle up elsewhere button multiple times on mobile)

QA Steps

Go to any chat
Click on the plus icon
Click Send money or Request Money
Enter any amount
Click Next
Press enter key multiple times on web/desktop (click Request/I'll settle up elsewhere button multiple times on mobile)

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

web.mov

Mobile Web - Chrome

mchrome.mp4

Mobile Web - Safari

msafari.mp4

Desktop

desktop.mov

iOS

ios.mp4

Android

android.mp4

melvin-bot · 2022-12-20T17:33:24Z

@youssef-lr @aimane-chnaif 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]

youssef-lr · 2022-12-20T20:25:54Z

src/pages/iou/IOUModal.js

-                                                onConfirm={this.createTransaction}
-                                                onSendMoney={this.sendMoney}
+                                                onConfirm={(selectedParticipants) => {
+                                                    if (this.creatingIOUTransaction) { return; }


Suggested change

if (this.creatingIOUTransaction) { return; }

// Prevent creating multiple transactions if the button is pressed repeatedly

if (this.creatingIOUTransaction) { return; }

Let's add a comment here to clarify the change.

youssef-lr · 2022-12-20T20:27:16Z

LGTM! Left a comment.

src/pages/iou/IOUModal.js

youssef-lr

LGTM and tested well! All yours @aimane-chnaif :)

aimane-chnaif · 2022-12-20T20:47:27Z

@youssef-lr is there any clear step of failure case? I tried to fail but always success. I believe it will work but just to confirm :)

youssef-lr · 2022-12-20T20:49:52Z

I think the failures that we used to handle in the IOU modal date back to when the API wasn't offline first, so we get an error right away if there's one. Now I believe this is handled in our OfflineWithFeedback component and in the ReportActionItem component.

But just to double check, let me try and force an error in the backend locally and see how it's handled.

youssef-lr · 2022-12-20T20:54:43Z

We're good to go I think, the modal does not seem to handle IOU errors anymore as it is dismissed immediately regardless of the outcome of the request. Instead this is delegated to the report action component.

aimane-chnaif · 2022-12-21T06:39:30Z

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

web.mov

Mobile Web - Chrome

mchrome.mp4

Mobile Web - Safari

msafari.mp4

Desktop

desktop.mov

iOS

ios.mp4

Android

android.mp4

aimane-chnaif

LGTM too and tests well!

OSBotify · 2022-12-21T06:47:06Z

✋ 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 · 2022-12-21T07:06:25Z

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
TTI	788.467 ms → 821.795 ms (+33.328 ms, +4.2%)
runJsBundle	184.484 ms → 197.781 ms (+13.297 ms, +7.2%)
nativeLaunch	9.533 ms → 10.355 ms (+0.822 ms, +8.6%)
regularAppStart	0.013 ms → 0.014 ms (+0.001 ms, +5.8%)

Show details

Name	Duration
TTI	Baseline Mean: 788.467 ms Stdev: 37.991 ms (4.8%) Runs: 729.180788999889 739.2257369998842 741.9324489999563 742.4191689998843 746.4694079998881 750.8819299999159 760.3486560001038 761.8422000000719 763.5151150000747 767.532113000052 769.1984910001047 769.4820159999654 776.003839999903 777.8112860000692 781.607861999888 782.2720790000167 783.9676049998961 785.6078389999457 787.6710580000654 792.4963650000282 794.8401190000586 796.7577480000909 800.7425259999 803.1152900001034 825.4187400001101 834.2232649999205 840.6024279999547 842.5916369999759 846.2432909999043 871.4382410000544 877.0468760000076 Current Mean: 821.795 ms Stdev: 48.327 ms (5.9%) Runs: 756.7225520000793 766.3813720000908 766.7440349999815 767.3538609999232 768.0298180000391 772.5206559998915 773.2062140000053 784.0502939999569 784.600424000062 785.152962999884 798.1391549999826 800.7600449998863 801.7219680000562 805.036945000058 805.1712529999204 806.0163690000772 813.8202859999146 823.8263340000995 827.0645409999415 829.7396060000174 832.7196909999475 844.0732040000148 844.5968069999944 844.9288669999223 852.1679150001146 867.8985500000417 897.46240999992 899.2494669998996 907.2083429999184 922.8255499999505 926.4556390000507
runJsBundle	Baseline Mean: 184.484 ms Stdev: 18.698 ms (10.1%) Runs: 153 158 162 164 168 169 171 171 172 173 175 176 177 179 179 180 180 184 185 188 188 191 191 194 196 198 212 217 218 224 226 Current Mean: 197.781 ms Stdev: 26.226 ms (13.3%) Runs: 162 164 169 172 174 174 176 177 178 178 179 181 182 184 186 191 191 193 198 203 205 205 214 215 217 217 217 233 233 243 251 267
nativeLaunch	Baseline Mean: 9.533 ms Stdev: 1.784 ms (18.7%) Runs: 8 8 8 8 8 8 8 8 8 8 8 8 9 9 9 9 9 10 10 10 10 10 10 11 11 11 11 12 14 15 Current Mean: 10.355 ms Stdev: 2.730 ms (26.4%) Runs: 7 7 8 8 8 8 8 8 8 9 9 9 9 9 9 10 10 10 10 10 11 11 11 12 13 13 13 14 14 16 19
regularAppStart	Baseline Mean: 0.013 ms Stdev: 0.001 ms (6.6%) Runs: 0.011760000139474869 0.012329000048339367 0.012370000127702951 0.012491999892517924 0.012654999969527125 0.012857999885454774 0.012899000197649002 0.013019999954849482 0.013020999962463975 0.013061000034213066 0.013142999960109591 0.013183000031858683 0.013184000039473176 0.013224000111222267 0.013264999957755208 0.013387999963015318 0.013589999871328473 0.01359100011177361 0.013630999950692058 0.013631999958306551 0.01387499994598329 0.013996999943628907 0.014078999869525433 0.014078999869525433 0.014079000102356076 0.014200999867171049 0.014649000018835068 0.015746999997645617 0.015787999844178557 Current Mean: 0.014 ms Stdev: 0.001 ms (6.6%) Runs: 0.012980999890714884 0.013142999960109591 0.013142999960109591 0.013264999957755208 0.013306000037118793 0.013346000108867884 0.013467999873682857 0.013630999950692058 0.013631000183522701 0.013671000022441149 0.01371199986897409 0.013753999955952168 0.013793000020086765 0.013916000025346875 0.014038000022992492 0.014038000022992492 0.014078999869525433 0.014118999941274524 0.014119999948889017 0.014608000172302127 0.014810999855399132 0.015177000081166625 0.015379999997094274 0.015381000004708767 0.015420999843627214 0.015583999920636415 0.01570700015872717 0.015909999841824174 0.0163569999858737

OSBotify · 2022-12-21T23:39:50Z

🚀 Deployed to staging by @youssef-lr in version: 1.2.43-0 🚀

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

OSBotify · 2022-12-27T18:26:12Z

🚀 Deployed to production by @chiragsalian in version: 1.2.43-1 🚀

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

prevent send/request money multiple times

69f7281

0xmiros requested a review from a team as a code owner December 20, 2022 17:33

melvin-bot bot requested review from aimane-chnaif and youssef-lr and removed request for a team December 20, 2022 17:33

0xmiros mentioned this pull request Dec 20, 2022

Money is sent/requested multiple times if the enter key is pressed multiple times reported by @adeel0202 #12714

Closed

youssef-lr reviewed Dec 20, 2022

View reviewed changes

add comment for preventing repeated transactions

4834d8e

youssef-lr reviewed Dec 20, 2022

View reviewed changes

src/pages/iou/IOUModal.js Show resolved Hide resolved

merge comment to prevent multiple transactions into one

4db637b

youssef-lr approved these changes Dec 20, 2022

View reviewed changes

aimane-chnaif approved these changes Dec 21, 2022

View reviewed changes

youssef-lr merged commit 6c74c2c into Expensify:main Dec 21, 2022

OSBotify mentioned this pull request Dec 21, 2022

Deploy Checklist: New Expensify 2022-12-21 #13777

Closed

20 tasks

situchan mentioned this pull request Mar 26, 2023

[HOLD for payment 2023-06-01] [HOLD for payment 2023-05-23] [$2000] Fast clicking any button 'n' times execute 'n' times #14572

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

prevent send/request money multiple times #13733

prevent send/request money multiple times #13733

0xmiros commented Dec 20, 2022

melvin-bot bot commented Dec 20, 2022

youssef-lr Dec 20, 2022

0xmiros Dec 20, 2022

youssef-lr commented Dec 20, 2022

youssef-lr left a comment

aimane-chnaif commented Dec 20, 2022

youssef-lr commented Dec 20, 2022

youssef-lr commented Dec 20, 2022

aimane-chnaif commented Dec 21, 2022

aimane-chnaif left a comment

OSBotify commented Dec 21, 2022

github-actions bot commented Dec 21, 2022

OSBotify commented Dec 21, 2022

OSBotify commented Dec 27, 2022

	if (this.creatingIOUTransaction) { return; }
	// Prevent creating multiple transactions if the button is pressed repeatedly
	if (this.creatingIOUTransaction) { return; }

prevent send/request money multiple times #13733

prevent send/request money multiple times #13733

Conversation

0xmiros commented Dec 20, 2022

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Dec 20, 2022

youssef-lr Dec 20, 2022

Choose a reason for hiding this comment

0xmiros Dec 20, 2022

Choose a reason for hiding this comment

youssef-lr commented Dec 20, 2022

youssef-lr left a comment

Choose a reason for hiding this comment

aimane-chnaif commented Dec 20, 2022

youssef-lr commented Dec 20, 2022

youssef-lr commented Dec 20, 2022

aimane-chnaif commented Dec 21, 2022

Reviewer Checklist

Screenshots/Videos

aimane-chnaif left a comment

Choose a reason for hiding this comment

OSBotify commented Dec 21, 2022

github-actions bot commented Dec 21, 2022

Performance Comparison Report 📊

Significant Changes To Duration

Meaningless Changes To Duration

OSBotify commented Dec 21, 2022

OSBotify commented Dec 27, 2022