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

Let API user set currencies in some payment account json forms #5685

Merged
merged 12 commits into from
Sep 17, 2021

Conversation

ghubstan
Copy link
Contributor

@ghubstan ghubstan commented Sep 6, 2021

Several payment methods support multiple trade currencies and a selected trade currency, but the API has not let CLI users specify them in the json form passed to the createpaymentacct command.

This change adds tradeCurrencies and selectedTradeCurrency fields to the appropriate json forms.

Summary of changes:

  • Added support for reading/writing trade currency info from/to json forms to PaymentAccountTypeAdapter.

  • Added new boolean convenience methods to PaymentAccount to aid in correctly defining and validating pay account json from currency lists and selections.

  • Adjusted CorePaymentAccountsService, CreateOfferService MutableOfferDataModel, MutableOfferView, MutableOfferViewModel, TakeOfferDataModel to PaymentAccount refactoring.

  • Added Res # List<String> getWrappedAsList(String key, int wrapLength) and Utilities # List<String> toListOfWrappedStrings(String s, int wrapLength) to convert long, multi-line i18n strings to wrapped strings the API can use.

  • Added constants to apitest's AbstractPaymentAccountTest for new SwiftAccount test case to be added after PR 5672 is merged.

  • Added asserts to existing CreatePaymentAccountTest cases: check new account instances' tradeCurrencies and selectedTradeCurrency field values written to payment account creation json forms.

  • Added new CreatePaymentAccountTest cases, including commented out case testCreateSwiftAccount -- in anticipation of the new Swift payment method merge.

  • Added apitest/src/test/resources/logback.xml for configuring apitest case logging.

  • Removed dead/commented code in some of the touched classes.

Based on branch detault-api-testharness-callrate-config, which was merged earlier today in PR 5683

Some i18n property values can be used by the API if long strings are
wrapped before written as commments to json payment account forms, or
written to the CLI console.

This change anticipates the addition of the more complex Swift payment
method (PR 5672). PR 5672's i18n property value for key "payment.swift.info"
will be wrapped and appended to the comments of the Swift payment account's
json form.
Several payment methods support multiple trade currencies and a selected
trade currency, but the api's payment account creation has not let CLI
users specify them in the json form passed to the `createpaymentacct`
command.

This change adds `tradeCurrencies` and `selectedTradeCurrency` fields
to the appropriate json forms.
Define and verify trade currencies and selected trade currency values
in appropriate api test cases, and add test cases for new payment methods.
Method was added on the false assumption `PaymentAccount#hasMultipleCurrencies()`
would not always return a correct value when a `PaymentAccount` instance is created
via reflection.  But `hasMultipleCurrencies()` will work as long as appropriate
PaymentAccount subclasses continue setting their `tradeCurrencies` fields within
their default constructors.
Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

Some comments, looks generally good

ghubstan added a commit to ghubstan/bisq that referenced this pull request Sep 16, 2021
ghubstan added a commit to ghubstan/bisq that referenced this pull request Sep 16, 2021
Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

utACK

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.

2 participants