[bisq-network/bisq] Bugfix: Set username when saving the account not on every text change (#4505)

chimp1984 notifications at github.com
Wed Sep 9 15:48:36 UTC 2020


@chimp1984 commented on this pull request.

NACK
I will make a PR with an alternative approach 

>                  && account.getTradeCurrencies().size() > 0);
     }
+
+    @Override
+    public PaymentAccount getPaymentAccountForAccountCreation() {
+        String userName = getUserName();
+        if (!userName.isEmpty()) {
+            account.setUserName(userName);
+        }
+        return super.getPaymentAccount();

Better use account instead of super.getPaymentAccount(); as that would suggest that this is different than the visible account we used for setting the username.

> @@ -73,7 +75,6 @@ public void addFormForAddAccount() {
         userNameInputTextField = FormBuilder.addInputTextField(gridPane, ++gridRow, Res.get("payment.account.userName"));
         userNameInputTextField.setValidator(validator);
         userNameInputTextField.textProperty().addListener((ov, oldValue, newValue) -> {
-            account.setUserName(newValue.trim());
             updateFromInputs();

I think the problem is that the setUserName method has side effects. I will make an alternative PR with solving the problem at the domain layer. Will be also more useful once used from API. 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/4505#pullrequestreview-485146378
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20200909/1e98361b/attachment.html>


More information about the bisq-github mailing list