[bisq-network/bisq] Add new payment method: Australian PayID (#4742)

[sqrrm]

@sqrrm commented on this pull request.

I think payid should be considered one word, more like in the original PR. It's pretty confusing with the PAY_ID_ID and as seen on https://payid.com.au/ they treat it like one word, although with odd capitalization.

@ripcurlx I added some suggestions as examples, but if you agree to change this, all the file names and class names and protobuf names also need to change.

> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Bisq. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package bisq.core.payment;
+
+import bisq.core.locale.FiatCurrency;
+import bisq.core.payment.payload.AustraliaPayIdPayload;
+import bisq.core.payment.payload.PaymentAccountPayload;
+import bisq.core.payment.payload.PaymentMethod;
+
+public final class AustraliaPayId extends PaymentAccount {

File name should agree with class name

I think we should treat PayID as a single word (mentioned in another comment) and use AustraliaPayid

> +
+import java.util.HashMap;
+import java.util.Map;
+
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.Setter;
+import lombok.ToString;
+import lombok.extern.slf4j.Slf4j;
+
+ at EqualsAndHashCode(callSuper = true)
+ at ToString
+ at Setter
+ at Getter
+ at Slf4j
+public final class AustraliaPayIdPayload extends PaymentAccountPayload {

Same file name, class name mismatch.

> +    ///////////////////////////////////////////////////////////////////////////////////////////
+    // API
+    ///////////////////////////////////////////////////////////////////////////////////////////
+
+    @Override
+    public String getPaymentDetails() {
+        return Res.get(paymentMethodId) + " - " + getPaymentDetailsForTradePopup().replace("\n", ", ");
+    }
+
+    @Override
+    public String getPaymentDetailsForTradePopup() {
+        return
+                Res.get("payment.australia.payId") + ": " + payId + "\n" +
+                        Res.get("payment.account.owner") + ": " + bankAccountName;
+    }
+

```suggestion
```

> @@ -74,6 +74,7 @@
     public static final String FASTER_PAYMENTS_ID = "FASTER_PAYMENTS";
     public static final String NATIONAL_BANK_ID = "NATIONAL_BANK";
     public static final String JAPAN_BANK_ID = "JAPAN_BANK";
+    public static final String AUSTRALIA_PAY_ID_ID = "AUSTRALIA_PAYID";

I think PayID should be considered one word. That seems to be how it's used at https://payid.com.au/

Their capitalization is kind of camel case, but I think it would make more sense to treat it as a single word. It's also not an id within bisq, just part of the name of this payment method so it would make things less confusing, like
```suggestion
    public static final String AUSTRALIA_PAYID_ID = "AUSTRALIA_PAYID";
```


> @@ -112,6 +113,7 @@
     public static PaymentMethod FASTER_PAYMENTS;
     public static PaymentMethod NATIONAL_BANK;
     public static PaymentMethod JAPAN_BANK;
+    public static PaymentMethod AUSTRALIA_PAY_ID;

```suggestion
    public static PaymentMethod AUSTRALIA_PAYID;
```

> @@ -185,6 +187,9 @@
             // Japan
             JAPAN_BANK = new PaymentMethod(JAPAN_BANK_ID, DAY, DEFAULT_TRADE_LIMIT_LOW_RISK),
 
+            // Australia
+            AUSTRALIA_PAY_ID = new PaymentMethod(AUSTRALIA_PAY_ID_ID, DAY, DEFAULT_TRADE_LIMIT_LOW_RISK),

```suggestion
            AUSTRALIA_PAYID = new PaymentMethod(AUSTRALIA_PAYID_ID, DAY, DEFAULT_TRADE_LIMIT_LOW_RISK),
```

> @@ -124,6 +125,8 @@ public PaymentAccountPayload fromProto(protobuf.PaymentAccountPayload proto) {
                     return InteracETransferAccountPayload.fromProto(proto);
                 case JAPAN_BANK_ACCOUNT_PAYLOAD:
                     return JapanBankAccountPayload.fromProto(proto);
+                case AUSTRALIA_PAY_ID_PAYLOAD:

```suggestion
                case AUSTRALIA_PAYID_PAYLOAD:
```

> @@ -3331,6 +3331,11 @@ payment.japan.bank=Bank
 payment.japan.branch=Branch
 payment.japan.account=Account
 payment.japan.recipient=Name
+payment.australia.payId=PayID

```suggestion
payment.australia.payid=PayID
```

> @@ -457,6 +470,8 @@ private PaymentMethodForm getPaymentMethodForm(PaymentMethod paymentMethod, Paym
                 return new SpecificBankForm(paymentAccount, accountAgeWitnessService, inputValidator, root, gridRow, formatter);
             case PaymentMethod.JAPAN_BANK_ID:
                 return new JapanBankTransferForm(paymentAccount, accountAgeWitnessService, japanBankTransferValidator, inputValidator, root, gridRow, formatter);
+            case PaymentMethod.AUSTRALIA_PAY_ID_ID:

```suggestion
            case PaymentMethod.AUSTRALIA_PAYID_ID:
```

-- 
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/4742#pullrequestreview-522403907
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20201103/72dd83d3/attachment.html>