Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent reset offer values after switch to same currency payment account #5983

Merged
merged 1 commit into from Jan 26, 2022
Merged

Conversation

ghost
Copy link

@ghost ghost commented Jan 20, 2022

Fixes #5973

When we switch to same currency account we don't need to reset volume, minVolume, price and marketPriceMargin.

When we switch to different currency account onCurrencySelected is invoked, so these params are being reset.

Pull request applies to "create offer", "edit offer" and "duplicate offer" views.

Demo:

  1. Before:
    before

  2. After:
    after

@ripcurlx ripcurlx added this to the v1.8.2 milestone Jan 20, 2022
Copy link
Contributor

@chimp1984 chimp1984 left a comment

Choose a reason for hiding this comment

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

I think that requires good testing. The validation checks are rather complex and carry already a few bugs. Resetting the values made it easier to not carry over stale validation state after changes. So I think its good to change that, but just want to emphisis the risks.

@ghost
Copy link
Author

ghost commented Jan 21, 2022

I agree code around MutableOfferView is quite complex. When I was debugging original issue I set debugger breakpoint on updateButtonDisableState method and I saw it was invoked 5 or 6 times after changing trading account (in ideal world there should be single call).

Sure - I'll try to prepare testing scenarios that covers changes in this pull request. If you see any edge cases that I should include don't hesitate to share them here.

@ripcurlx
Copy link
Contributor

I did try some scenarios and removing this reset code didn't make anything worse, but I found a different bug.

bug

If you switch in create offer between payment methods that share the same supported currency and the new payment method has a different default currency set, the price related fields are not updated properly.

@xyzmaker123 Do you want to prepare a PR for this as well?

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

ACK - Tested it on Regtest

@ripcurlx ripcurlx merged commit 44b02b3 into bisq-network:master Jan 26, 2022
@ghost
Copy link
Author

ghost commented Jan 26, 2022

@ripcurlx Sure - I can try to fix this issue. If you don't mind please prepare appropriate issue and assign me (for better tracking).

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.

CONFIRM: EDIT OFFER button disabled after changing payment account
2 participants