From 20cb98aa93903d804c474efd579044362cbedb80 Mon Sep 17 00:00:00 2001
From: chimp1984 <chimp1984@gmx.com>
Date: Wed, 9 Sep 2020 11:21:43 -0500
Subject: [PATCH] Remove side effect in setUserName method and add extra
 handling for the moment we save the account. Only at that moment we check if
 we need to set the accountId with the value of the userName. We do that in
 the domain layer to avoid more domain logic code in the UI layer.

Fixes bug found at:
https://github.com/bisq-network/bisq/pull/4481#pullrequestreview-485066342
---
 .../bisq/core/payment/PaymentAccount.java     |  5 +++++
 .../bisq/core/payment/RevolutAccount.java     | 22 ++++++++++++++-----
 .../payload/RevolutAccountPayload.java        |  8 +++++--
 core/src/main/java/bisq/core/user/User.java   |  2 ++
 4 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/core/src/main/java/bisq/core/payment/PaymentAccount.java b/core/src/main/java/bisq/core/payment/PaymentAccount.java
index f63ad2ca8ff..12a9565b710 100644
--- a/core/src/main/java/bisq/core/payment/PaymentAccount.java
+++ b/core/src/main/java/bisq/core/payment/PaymentAccount.java
@@ -173,4 +173,9 @@ public String getSaltAsHex() {
     public String getOwnerId() {
         return paymentAccountPayload.getOwnerId();
     }
+
+    public void onAddToUser() {
+        // We are in the process to get added to the user. This is called just before saving the account and the
+        // last moment we could apply some special handling if needed (e.g. as it happens for Revolut)
+    }
 }
diff --git a/core/src/main/java/bisq/core/payment/RevolutAccount.java b/core/src/main/java/bisq/core/payment/RevolutAccount.java
index 9e8d41496f6..93191bbac01 100644
--- a/core/src/main/java/bisq/core/payment/RevolutAccount.java
+++ b/core/src/main/java/bisq/core/payment/RevolutAccount.java
@@ -37,22 +37,34 @@ protected PaymentAccountPayload createPayload() {
     }
 
     public void setUserName(String userName) {
-        ((RevolutAccountPayload) paymentAccountPayload).setUserName(userName);
+        revolutAccountPayload().setUserName(userName);
     }
 
     public String getUserName() {
-        return ((RevolutAccountPayload) paymentAccountPayload).getUserName();
+        return (revolutAccountPayload()).getUserName();
     }
 
     public String getAccountId() {
-        return ((RevolutAccountPayload) paymentAccountPayload).getAccountId();
+        return (revolutAccountPayload()).getAccountId();
     }
 
     public boolean userNameNotSet() {
-        return ((RevolutAccountPayload) paymentAccountPayload).userNameNotSet();
+        return (revolutAccountPayload()).userNameNotSet();
     }
 
     public boolean hasOldAccountId() {
-        return ((RevolutAccountPayload) paymentAccountPayload).hasOldAccountId();
+        return (revolutAccountPayload()).hasOldAccountId();
+    }
+
+    private RevolutAccountPayload revolutAccountPayload() {
+        return (RevolutAccountPayload) paymentAccountPayload;
+    }
+
+    @Override
+    public void onAddToUser() {
+        super.onAddToUser();
+
+        // At save we apply the userName to accountId in case it is empty for backward compatibility
+        revolutAccountPayload().maybeApplyUserNameToAccountId();
     }
 }
diff --git a/core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java b/core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java
index 72548a14d48..19050325cfa 100644
--- a/core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java
+++ b/core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java
@@ -81,7 +81,7 @@ private RevolutAccountPayload(String paymentMethod,
                 excludeFromJsonDataMap);
 
         this.accountId = accountId;
-        setUserName(userName);
+        this.userName = userName;
     }
 
     @Override
@@ -169,7 +169,11 @@ public boolean hasOldAccountId() {
 
     public void setUserName(String userName) {
         this.userName = userName;
-        // We need to set accountId as pre v1.3.8 clients expect the accountId field
+    }
+
+    // In case it is a new account we need to fill the accountId field to support not-updated traders who are not
+    // aware of the new userName field
+    public void maybeApplyUserNameToAccountId() {
         if (accountId.isEmpty()) {
             accountId = userName;
         }
diff --git a/core/src/main/java/bisq/core/user/User.java b/core/src/main/java/bisq/core/user/User.java
index 75891395d22..d9adac9a09c 100644
--- a/core/src/main/java/bisq/core/user/User.java
+++ b/core/src/main/java/bisq/core/user/User.java
@@ -190,6 +190,8 @@ public boolean hasPaymentAccountForCurrency(TradeCurrency tradeCurrency) {
     ///////////////////////////////////////////////////////////////////////////////////////////
 
     public void addPaymentAccount(PaymentAccount paymentAccount) {
+        paymentAccount.onAddToUser();
+
         boolean changed = paymentAccountsAsObservable.add(paymentAccount);
         setCurrentPaymentAccount(paymentAccount);
         if (changed)