Feat/improve payment detected behavior

[lissavxo]

Related to #378

Description

• Fixed logic to trigger success and transaction callback
• Fixed sound to play only if the tx was successful
• Isolate logic to trigger success
• Added tests to shouldTriggerSuccess function
• Removed unnecessary comments

Test plan

Open the project, run yarn clean:build make a payment and check if we trigger success correctly as described here #378

[Klakurka]

when we have [sound:true hide-toasts: true] will we display sound ?

Yes

[lissavxo]

when we have [sound:true hide-toasts: true] will we display sound ?

Yes

Fixed

[Klakurka]

After changing the amount within an editable dialog, paying to it doesn't trigger on-success.

[Klakurka]

Let's add some tests for this branch.

We can talk about it together with @chedieck to see what we can + should do. It's tedious and unreliable to be testing things manually. The bug with the editable dialog is an example of something that is easy to miss during manual testing.

[chedieck]

Left some remarks about test naming, also pushed a commit improving a little on a test

I found that in the button generator:

• Editable toggle set to false still allows it to be editable
• Sending a TX to the address with the same amount, same payment id (in the opReturn) but different data (so different opReturn but same paymentId) triggers onSuccess, not onTransaction. For what I gathered from Ensure proper thing happens when payment is received #378 this is not desired behavior.
• If we have paymentId disabled, then sending the tx with the correct amount and no paymentId still doesn't trigger onSuccess. My suspicion is that the "shouldTriggerOnSuccess" function is comparing undefined with '' or smth like that.

When solving the bugs above, make sure you first write tests that catch them, then solve them.

Finally, would be good to also:

• add the yarn test:coverage command to the root folder
• put min=0 in the amount & goal amount fields of the button generator

[chedieck]

onSuccess triggered twice.

To reproduce the screenshot, go to chedieck.com/paybutton, and send 30XEC to the button with name "ONOPEN".

[lissavxo]

onSuccess triggered twice.

To reproduce the screenshot, go to chedieck.com/paybutton, and send 30XEC to the button with name "ONOPEN".

this happens on master too, I believe It is due to having a widget and a button with the same address in the same page, they are not isolated. For example, when you pay in the widget and you have two widgets to the same address both will change the qrcode to the check mark, this is why the callback is called twice.

[chedieck]

Okay, so let's start by breaking this into multiple PRs. I see some changes are really isolated from the rest, so separating those should be easy. By just reading the PR description, I would suggest:

Fixed logic to call OnSuccess and OnTransaction

1

Added a Vue.js paybutton generator to the dev page

2

Added css improvements to the html dev page

2

Cleaned the component widget container

1

added a new command to help testing yarn dev:refresh it will rebuild the react project and run dev

2

fix docs can-edit -> editable

1

fixed sound not playing even if sound:true

1

refactored utils

3

added new utils and hooks to remove extra responsibilities of components

3

added test:coverage command to generate coverage files from tests

4

... where 1-4 are branches which each of these changes should belong to. Bear in mind this is just a suggestion by reading what are the changes, I may well be putting together things that having nothing to do with each other (diffwise) and separating things that do.

[lissavxo]

Okay, so let's start by breaking this into multiple PRs. I see some changes are really isolated from the rest, so separating those should be easy. By just reading the PR description, I would suggest:

Fixed logic to call OnSuccess and OnTransaction

1

Added a Vue.js paybutton generator to the dev page

2

Added css improvements to the html dev page

2

Cleaned the component widget container

1

added a new command to help testing yarn dev:refresh it will rebuild the react project and run dev

2

fix docs can-edit -> editable

1

fixed sound not playing even if sound:true

1

refactored utils

3

added new utils and hooks to remove extra responsibilities of components

3

added test:coverage command to generate coverage files from tests

4

... where 1-4 are branches which each of these changes should belong to. Bear in mind this is just a suggestion by reading what are the changes, I may well be putting together things that having nothing to do with each other (diffwise) and separating things that do.

thanks for the suggestion, breaking up these changes in smaller branches sounds like a good plan, will work on that, thanks

[chedieck]

Tested on my server chedieck.com/paybutton

By trying to pay the first widget visible on the page, both get triggered to success, even though their OPRETURN and amount differs.

Now what I get is that success is not triggered on neither.

[lissavxo]

Tested on my server chedieck.com/paybutton
By trying to pay the first widget visible on the page, both get triggered to success, even though their OPRETURN and amount differs.

Now what I get is that success is not triggered on neither.

This was due to rawMessage not being returned from server, when I tested on local server on master it works because local server returned rawMessage. Even though I did a change to consider the message if there is no rawMessage in transaction.
So the logic is , if there is a rawMessage we compare with the rawMessage if not we compare with message if exists.

[Klakurka]

Conflicts.

[chedieck]

Same issue as before when I tested. This was tested in my localhost, having paybutton-server running locally.

Receiving a 20XEC tx activates success in a paybutton with 20USD of amount. Even if there is only the 20USD widget there.

This is wrong not only because the amount is not being taken into account to activate it, but neither is the paymentId.

[lissavxo]

Can you provide more info on this ? like the code you used to test it like the html code ?

[chedieck]

LGTM!