(Bug) #1052 Prevent to run multiple transaction #1053

yaroslav-fedyshyn-nordwhale · 2019-12-11T13:31:37Z

Description

Prevent users to click on the confirm button multiple times as then multiple transactions will be sent. The finally block was executed right after the try block as the result wasn't awaited. SO the fix is to wait for generateLink fn to be performed.

About #1052

How Has This Been Tested?

Create a receive link.
Use it with another account.
Go to the last/confirm step.
Try to press the confirm button quickly many times.
It should be disabled with a loader after the first press.

Checklist:

Video: https://www.screencast.com/t/VBuFHCgSI

src/lib/gundb/UserStorageClass.js

sirpy

send should be immediate, we don't await for it. if there's a button double click issue, you need to disable the button on click. I think we already implemented this somewhere. maybe add it as a feature to all buttons
yaroslav already did a pr that checks if feeditem is undefined, we should not have undefined items, double check his previous fix for undefined. it could also be solved by the latest gun fix I've pushed.

AlexeyKosinski · 2019-12-11T19:24:07Z

send should be immediate, we don't await for it. if there's a button double click issue, you need to disable the button.

we already have a disable button here. it is setLoading to state. but it happens at an incorrect time. so we need add await. or remove setLoading(false) from finally section and add setLoading(false)in onTxHash and onError. for user it will be the same as just add await.

AlexeyKosinski · 2019-12-11T19:29:32Z

yaroslav already did a pr that checks if feeditem is undefined, we should not have undefined items, double check his previous fix for undefined. it could also be solved by the latest gun fix I've pushed.

I've added just filter not show undefined items I think it will be good to have them. but if u want we can delete this fix

sirpy · 2019-12-12T20:33:20Z

it's not the same. the promise is resolved on receipt.
we need to understand why there's undefined, why the fix yaroslav did doesn't work

… handler

yaroslav-fedyshyn-nordwhale · 2019-12-13T10:37:08Z

it's not the same. the promise is resolved on receipt.

we need to understand why there's undefined, why the fix yaroslav did doesn't work

@sirpy

made the setLoading fn to be called from callback handler

AlexeyKosinski · 2019-12-13T13:15:16Z

2. we need to understand why there's undefined, why the fix yaroslav did doesn't work

I think the problem was with duplicate transactions when I press confirm button. because once I clicked 5 times. and it ran 5 transactions. but the I saw popup error not enough GD. this case is not real if we will merge this PR

sirpy

@yaroslav-fedyshyn-nordwhale @AlexeyKosinski we have to ways 2 send GD

directly
via payment link

sirpy · 2019-12-15T10:11:44Z

@yaroslav-fedyshyn-nordwhale @AlexeyKosinski also please check all buttons functionality and how we can prevent double clicking issues

yaroslav-fedyshyn-nordwhale · 2019-12-16T09:01:30Z

@yaroslav-fedyshyn-nordwhale @AlexeyKosinski also please check all buttons functionality and how we can prevent double clicking issues

@sirpy we already have the functionality to prevent double-clicking. We can block the button to be clicked multiple times by passing the 'loading' (boolean) property to the CustomButton component. It was already implemented in the place we had an issue, but the setLoading(false); was called in 'finally' clause of 'try' block, so the button was unlocked without waiting for any transaction response. That is why we could confirm multiple times. After we place the setLoading(false) to the correct place (after the response received) - confirm button will be unlocked correctly.

I checked the button across the app and didn't find the same behavior.

sirpy

in SendLinkSummary.js i dont see any usage of loading.
scenario seems the same to me, user clicks confirms and a tx is generated. dont see how loading is implemented there

yaroslav-fedyshyn-nordwhale · 2019-12-16T15:43:00Z

in SendLinkSummary.js i dont see any usage of loading.
scenario seems the same to me, user clicks confirms and a tx is generated. dont see how loading is implemented there

We don't need loading there as after the user press confirm button he will be redirected to next step or share dialog will be shown in case of mobile. The hash we waiting from onTxHash callback needed just to create a feed card (async).

yaroslav-fedyshyn-nordwhale · 2019-12-17T12:12:08Z

@sirpy I pushed new commit

# Conflicts: # src/components/profile/__tests__/OptionsRow.js # src/components/profile/__tests__/__snapshots__/OptionsRow.js.snap

sirpy · 2019-12-18T09:51:02Z

src/components/dashboard/SendLinkSummary.js

-        ({ paymentLink, code }) => (hash: string) => {
-          log.debug({ hash })
+      const generateLinkResponse = goodWallet.generateLink(amount, reason, {
+        onTransactionHash: ({ hash, paymentLink, code }) => {


no need for special ontxhash
take paymetlink and code from generatelinkresponse

sirpy · 2019-12-18T09:51:42Z

src/lib/wallet/GoodWalletClass.js

@@ -560,7 +559,7 @@ export class GoodWallet {
    })

    //pass extra data
-    const onTransactionHash = getOnTxHash({ paymentLink, code })
+    const onTransactionHash = hash => events.onTransactionHash({ hash, paymentLink, code })


fix: Prevent the confirm button to be clicked multiple times

6d0a072

AlexeyKosinski approved these changes Dec 11, 2019

View reviewed changes

src/lib/gundb/UserStorageClass.js Outdated Show resolved Hide resolved

yaroslav-fedyshyn-nordwhale requested review from sirpy, AlexeyKosinski and Nordwhale December 11, 2019 13:35

AlexeyKosinski approved these changes Dec 11, 2019

View reviewed changes

sirpy requested changes Dec 11, 2019

View reviewed changes

yaroslav-fedyshyn-nordwhale requested a review from sirpy December 12, 2019 07:29

fix: sendQRSummary make setLoading(false) on transactionHash, onError…

bac60a4

… handler

sirpy requested changes Dec 15, 2019

View reviewed changes

yaroslav-fedyshyn-nordwhale requested a review from sirpy December 16, 2019 09:01

fix: add new test to pass coverage

d8bf3a1

sirpy requested changes Dec 16, 2019

View reviewed changes

yaroslav-fedyshyn-nordwhale requested a review from sirpy December 16, 2019 15:43

refactor: refactor SendLinkSummary generateLink method

4fb5966

yaroslav-fedyshyn-nordwhale added 3 commits December 17, 2019 16:58

add: add test to pass coverage

90f4356

Merge branch 'master' into 1052-Prevent_to_run_multiple_transaction

970e061

# Conflicts: # src/components/profile/__tests__/OptionsRow.js # src/components/profile/__tests__/__snapshots__/OptionsRow.js.snap

fix: delete duplicated test, add new to pass coverage

0f5670e

sirpy requested changes Dec 18, 2019

View reviewed changes

fix: remove additional args from onTxHash callback

2072c98

yaroslav-fedyshyn-nordwhale requested a review from sirpy December 18, 2019 10:11

sirpy merged commit 908acf2 into master Dec 18, 2019

serdiukov-o-nordwhale deleted the 1052-Prevent_to_run_multiple_transaction branch July 8, 2021 21:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(Bug) #1052 Prevent to run multiple transaction #1053

(Bug) #1052 Prevent to run multiple transaction #1053

yaroslav-fedyshyn-nordwhale commented Dec 11, 2019 •

edited

Loading

sirpy left a comment •

edited

Loading

AlexeyKosinski commented Dec 11, 2019 •

edited

Loading

AlexeyKosinski commented Dec 11, 2019

sirpy commented Dec 12, 2019

yaroslav-fedyshyn-nordwhale commented Dec 13, 2019 •

edited

Loading

AlexeyKosinski commented Dec 13, 2019 •

edited

Loading

sirpy left a comment

sirpy commented Dec 15, 2019

yaroslav-fedyshyn-nordwhale commented Dec 16, 2019

sirpy left a comment

yaroslav-fedyshyn-nordwhale commented Dec 16, 2019

yaroslav-fedyshyn-nordwhale commented Dec 17, 2019

sirpy Dec 18, 2019

yaroslav-fedyshyn-nordwhale Dec 18, 2019

sirpy Dec 18, 2019

yaroslav-fedyshyn-nordwhale Dec 18, 2019

(Bug) #1052 Prevent to run multiple transaction #1053

(Bug) #1052 Prevent to run multiple transaction #1053

Conversation

yaroslav-fedyshyn-nordwhale commented Dec 11, 2019 • edited Loading

Description

How Has This Been Tested?

Checklist:

sirpy left a comment • edited Loading

Choose a reason for hiding this comment

AlexeyKosinski commented Dec 11, 2019 • edited Loading

AlexeyKosinski commented Dec 11, 2019

sirpy commented Dec 12, 2019

yaroslav-fedyshyn-nordwhale commented Dec 13, 2019 • edited Loading

AlexeyKosinski commented Dec 13, 2019 • edited Loading

sirpy left a comment

Choose a reason for hiding this comment

sirpy commented Dec 15, 2019

yaroslav-fedyshyn-nordwhale commented Dec 16, 2019

sirpy left a comment

Choose a reason for hiding this comment

yaroslav-fedyshyn-nordwhale commented Dec 16, 2019

yaroslav-fedyshyn-nordwhale commented Dec 17, 2019

sirpy Dec 18, 2019

Choose a reason for hiding this comment

yaroslav-fedyshyn-nordwhale Dec 18, 2019

Choose a reason for hiding this comment

sirpy Dec 18, 2019

Choose a reason for hiding this comment

yaroslav-fedyshyn-nordwhale Dec 18, 2019

Choose a reason for hiding this comment

yaroslav-fedyshyn-nordwhale commented Dec 11, 2019 •

edited

Loading

sirpy left a comment •

edited

Loading

AlexeyKosinski commented Dec 11, 2019 •

edited

Loading

yaroslav-fedyshyn-nordwhale commented Dec 13, 2019 •

edited

Loading

AlexeyKosinski commented Dec 13, 2019 •

edited

Loading