[bisq-network/bisq] Add support to duplicate BSQ swap offers (PR #5886)

[chimp1984]

@chimp1984 requested changes on this pull request.



> @@ -124,10 +125,20 @@ public BsqSwapOfferModel(OfferUtil offerUtil,
     // API
     ///////////////////////////////////////////////////////////////////////////////////////////
 
-    public void init(OfferDirection direction, boolean isMaker) {
+    public void init(OfferDirection direction, boolean isMaker, Offer offer) {

Can you add a @Nullable annotation to offer?

> @@ -91,8 +92,8 @@
     // API
     ///////////////////////////////////////////////////////////////////////////////////////////
 
-    void initWithData(OfferDirection direction) {
-        bsqSwapOfferModel.init(direction, true);
+    void initWithData(OfferDirection direction, BsqSwapOfferPayload offerPayload) {

Can you add a @nullable annotation to offerPayload?

> @@ -159,9 +160,12 @@ protected void deactivate() {
     // API
     ///////////////////////////////////////////////////////////////////////////////////////////
 
-    public void initWithData(OfferDirection direction, OfferView.OfferActionHandler offerActionHandler) {
+    public void initWithData(OfferDirection direction,
+                             OfferView.OfferActionHandler offerActionHandler,
+                             BsqSwapOfferPayload offerPayload) {

Can you add @Nullable?

> @@ -159,8 +162,8 @@ protected void deactivate() {
     // API
     ///////////////////////////////////////////////////////////////////////////////////////////
 
-    void initWithData(OfferDirection direction) {
-        dataModel.initWithData(direction);
+    void initWithData(OfferDirection direction, BsqSwapOfferPayload offerPayload) {

Can you add a @nullable annotation to offerPayload?

> @@ -572,6 +575,33 @@ private void updateButtonDisableState() {
         isPlaceOfferButtonDisabled.set(createOfferRequested || !inputDataValid || miningPoW.get());
     }
 
+    private void maybeInitializeWithData() {
+        ObjectProperty<Coin> btcMinAmount = dataModel.getMinAmount();
+        if (btcMinAmount.get() != null) {
+            minAmountAsCoinListener.changed(btcMinAmount, null, btcMinAmount.get());
+        }
+
+        ObjectProperty<Coin> btcAmount = dataModel.getBtcAmount();
+
+        if (btcAmount.get() != null && btcAmount.get() != null) {

One should be probably minAmount

> @@ -572,6 +575,33 @@ private void updateButtonDisableState() {
         isPlaceOfferButtonDisabled.set(createOfferRequested || !inputDataValid || miningPoW.get());
     }
 
+    private void maybeInitializeWithData() {

Maybe rename to `maybeApplyFromDuplicateOffer` or the like, to make the use case more clear. I check if the offer is null at the caller would also help to make it more clear when its applied.

> @@ -263,18 +273,7 @@ public void initialize() {
                     TableRow<Tradable> row = new TableRow<>();
                     ContextMenu rowMenu = new ContextMenu();
                     MenuItem editItem = new MenuItem(Res.get("portfolio.context.offerLikeThis"));

editItem -> duplicateItem

> @@ -61,7 +61,7 @@ protected void doActivate() {
         onPaymentAccountsComboBoxSelected();
     }
 
-    public void initWithData(OfferPayload offerPayload) {
+    public void initWithData(OfferPayloadBase offerPayload) {

Is it required to change the type to OfferPayloadBase as it is only used by non-BSQ-Swap offers.

> @@ -228,11 +233,18 @@ private void loadView(Class<? extends View> viewClass, @Nullable Object data) {
                 selectOpenOffersView((OpenOffersView) view);
             }
         } else if (view instanceof DuplicateOfferView) {
-            if (duplicateOfferView == null && data instanceof OfferPayload) {
+            if (duplicateOfferView == null && data instanceof OfferPayloadBase) {
+                // Switch to create BSQ swap offer
+                if (data instanceof BsqSwapOfferPayload) {

I think it would be better to make that distinction in the caller directly (or a util if it would lead to code duplication). Otherwise its a bit confusing to expect first the DuplicateOfferView which does not support BSQ swaps and later find here the hack to rearrange the navigation.

-- 
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/5886#pullrequestreview-822895388
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20211203/647898cb/attachment-0001.htm>