From f2f519ada66a1dfeb39e428619f1b57ee2b5b76e Mon Sep 17 00:00:00 2001 From: jmacxx <47253594+jmacxx@users.noreply.github.com> Date: Tue, 12 May 2020 15:17:10 -0500 Subject: [PATCH] Display correct trade and tx fees in CSV export When exporting trade history to CSV, users would expect to see their own transaction and trade fees listed. Maker fees are stored on the Offer, Taker fees are stored on the Trade object. The code was not properly checking whether the user was the Offer Maker or Taker, and as such the report was always displaying the Taker information for Tx and Trade fee. Therefore any trades where the user was Maker showed the wrong fees. The column title `Maker Fee` is incorrect and misleading since the user doing the export is not necessarily the Maker. We rename this column to `Trade Fee` to indicate that it is the fee the user paid for trading regardless of Maker/Taker status. Fixes #4207 --- .../main/resources/i18n/displayStrings.properties | 2 +- .../portfolio/closedtrades/ClosedTradesView.fxml | 2 +- .../portfolio/closedtrades/ClosedTradesView.java | 14 +++++++------- .../closedtrades/ClosedTradesViewModel.java | 8 ++++++-- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/core/src/main/resources/i18n/displayStrings.properties b/core/src/main/resources/i18n/displayStrings.properties index 2e8e17cb622..aebb698dcf4 100644 --- a/core/src/main/resources/i18n/displayStrings.properties +++ b/core/src/main/resources/i18n/displayStrings.properties @@ -63,7 +63,7 @@ shared.priceInCurForCur=Price in {0} for 1 {1} shared.fixedPriceInCurForCur=Fixed price in {0} for 1 {1} shared.amount=Amount shared.txFee=Transaction Fee -shared.makerFee=Maker Fee +shared.tradeFee=Trade Fee shared.buyerSecurityDeposit=Buyer Deposit shared.sellerSecurityDeposit=Seller Deposit shared.amountWithCur=Amount in {0} diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/closedtrades/ClosedTradesView.fxml b/desktop/src/main/java/bisq/desktop/main/portfolio/closedtrades/ClosedTradesView.fxml index 04ed87f4bad..25900ef61af 100644 --- a/desktop/src/main/java/bisq/desktop/main/portfolio/closedtrades/ClosedTradesView.fxml +++ b/desktop/src/main/java/bisq/desktop/main/portfolio/closedtrades/ClosedTradesView.fxml @@ -41,7 +41,7 @@ - + diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/closedtrades/ClosedTradesView.java b/desktop/src/main/java/bisq/desktop/main/portfolio/closedtrades/ClosedTradesView.java index 193cc4561d9..c282e86db0d 100644 --- a/desktop/src/main/java/bisq/desktop/main/portfolio/closedtrades/ClosedTradesView.java +++ b/desktop/src/main/java/bisq/desktop/main/portfolio/closedtrades/ClosedTradesView.java @@ -82,7 +82,7 @@ public class ClosedTradesView extends ActivatableViewAndModel tableView; @FXML - TableColumn priceColumn, amountColumn, volumeColumn, txFeeColumn, makerFeeColumn, buyerSecurityDepositColumn, sellerSecurityDepositColumn, + TableColumn priceColumn, amountColumn, volumeColumn, txFeeColumn, tradeFeeColumn, buyerSecurityDepositColumn, sellerSecurityDepositColumn, marketColumn, directionColumn, dateColumn, tradeIdColumn, stateColumn, avatarColumn; @FXML HBox footerBox; @@ -121,7 +121,7 @@ public ClosedTradesView(ClosedTradesViewModel model, @Override public void initialize() { txFeeColumn.setGraphic(new AutoTooltipLabel(Res.get("shared.txFee"))); - makerFeeColumn.setGraphic(new AutoTooltipLabel(Res.get("shared.makerFee"))); + tradeFeeColumn.setGraphic(new AutoTooltipLabel(Res.get("shared.tradeFee"))); buyerSecurityDepositColumn.setGraphic(new AutoTooltipLabel(Res.get("shared.buyerSecurityDeposit"))); sellerSecurityDepositColumn.setGraphic(new AutoTooltipLabel(Res.get("shared.sellerSecurityDeposit"))); priceColumn.setGraphic(new AutoTooltipLabel(Res.get("shared.price"))); @@ -141,7 +141,7 @@ public void initialize() { setDirectionColumnCellFactory(); setAmountColumnCellFactory(); setTxFeeColumnCellFactory(); - setMakerFeeColumnCellFactory(); + setTradeFeeColumnCellFactory(); setBuyerSecurityDepositColumnCellFactory(); setSellerSecurityDepositColumnCellFactory(); setPriceColumnCellFactory(); @@ -167,7 +167,7 @@ public void initialize() { txFeeColumn.setComparator(nullsFirstComparing(o -> o instanceof Trade ? ((Trade) o).getTxFee() : o.getOffer().getTxFee() )); - makerFeeColumn.setComparator(nullsFirstComparing(o -> + tradeFeeColumn.setComparator(nullsFirstComparing(o -> o instanceof Trade ? ((Trade) o).getTakerFee() : o.getOffer().getMakerFee() )); buyerSecurityDepositColumn.setComparator(nullsFirstComparing(o -> @@ -518,9 +518,9 @@ public void updateItem(final ClosedTradableListItem item, boolean empty) { }); } - private void setMakerFeeColumnCellFactory() { - makerFeeColumn.setCellValueFactory((offer) -> new ReadOnlyObjectWrapper<>(offer.getValue())); - makerFeeColumn.setCellFactory( + private void setTradeFeeColumnCellFactory() { + tradeFeeColumn.setCellValueFactory((offer) -> new ReadOnlyObjectWrapper<>(offer.getValue())); + tradeFeeColumn.setCellFactory( new Callback<>() { @Override public TableCell call( diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/closedtrades/ClosedTradesViewModel.java b/desktop/src/main/java/bisq/desktop/main/portfolio/closedtrades/ClosedTradesViewModel.java index 26ae7a903d7..e359e4f0ee8 100644 --- a/desktop/src/main/java/bisq/desktop/main/portfolio/closedtrades/ClosedTradesViewModel.java +++ b/desktop/src/main/java/bisq/desktop/main/portfolio/closedtrades/ClosedTradesViewModel.java @@ -91,7 +91,7 @@ String getTxFee(ClosedTradableListItem item) { if (item == null) return ""; Tradable tradable = item.getTradable(); - if (tradable instanceof Trade) + if (!wasMyOffer(tradable) && (tradable instanceof Trade)) return formatter.formatCoin(((Trade) tradable).getTxFee()); else return formatter.formatCoin(tradable.getOffer().getTxFee()); @@ -101,7 +101,7 @@ String getMakerFee(ClosedTradableListItem item) { if (item == null) return ""; Tradable tradable = item.getTradable(); - if (tradable instanceof Trade) + if (!wasMyOffer(tradable) && (tradable instanceof Trade)) return formatter.formatCoin(((Trade) tradable).getTakerFee()); else return formatter.formatCoin(tradable.getOffer().getMakerFee()); @@ -195,4 +195,8 @@ int getNumPastTrades(Tradable tradable) { .collect(Collectors.toSet()) .size(); } + + boolean wasMyOffer(Tradable tradable) { + return dataModel.closedTradableManager.wasMyOffer(tradable.getOffer()); + } }