Skip to content

Conversation

@lissavxo
Copy link
Collaborator

Related to #366

Description

Fix button not triggering success when random satoshis is active

Test plan

1 - Test component(paybutton and widget) with no amount should trigger succes
2 - Test if component (paybutton and widget) with an amount and without random satoshis active is triggering sucess
3 - Test if component (paybutton and widget) with an amount and random satoshis active is triggering success as expected

@lissavxo lissavxo requested a review from chedieck April 10, 2025 19:27
@lissavxo lissavxo changed the title fix: randomSatochis trigger success fix: randomSatoshis trigger success Apr 11, 2025
Copy link
Collaborator

@chedieck chedieck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I understood it correctly, but I was guessing random-satoshis=true should make ONLY the amounts being equal necessary to call a transaction a success transaction.

i.e. ignoring the opReturn, and with him the paymentId.

contributionOffset,
...widgetProps
} = props;
const [internalCurrencyObj, _setInternalCurrencyObj] = useState<CurrencyObject>();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setter shouldn't have been prefixed with a underscore, it is used in the line below

@lissavxo lissavxo force-pushed the feat/trigger-success-random-satoshis branch from d3e2241 to c62aee4 Compare April 23, 2025 14:05
@lissavxo lissavxo requested a review from chedieck April 23, 2025 14:05
Copy link
Collaborator

@chedieck chedieck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not getting a success with the same amount if I don't send anything in the OP_RETURN.

Tried for the button:

    <div class="paybutton" to="ecash:qq3zv9s3jna6x5t0fxzfgd2zwteqyyjrscf9zmthz7" text="Mine" amount="21"
                                                                                             random-satoshis="true"
      currency="XEC" goal-amount="1000" success-text="WOW!" on-close="myClose" on-open="myOpen"
      on-success="mySuccessFunction" on-transaction="myTransactionFunction" 
                ></div>

Copy link
Collaborator

@chedieck chedieck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, didn't pulled the latest changes, it is actually working well. Just left a code question.

transaction: Transaction,
currency: string,
price: number,
randomSatoshis: number | boolean,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 'number'? AFAIK this is a boolean.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check WidgetContainerProps randomSatoshis is boolean | number there

@lissavxo lissavxo force-pushed the feat/trigger-success-random-satoshis branch from 3a231c3 to fb51c8a Compare April 30, 2025 14:35
@chedieck chedieck self-requested a review May 5, 2025 14:12
@chedieck chedieck merged commit 9d2a3e6 into master May 8, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants