Skip to content

Commit

Permalink
code review changes requested by @ripcurlx
Browse files Browse the repository at this point in the history 
renamed alertCountProperty to badgeCountProperty
changed the badge counting policy so that the count of chat messages does not contribute to the badge number, but merely the presence of chat messages, and/or the new flag.
show an explanatory message when trader is not allowed to close own dispute ticket
removed some comments and code that were accidentally included from PR5160
renamed variable tpe to tradePeriodEnd
renamed variable cptyName to counterpartyName
clear the new badge on double-click of dispute tablerow
  • Loading branch information
jmacxx committed Mar 15, 2021
1 parent 35f6ea2 commit ad4927a
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 66 deletions.
20 changes: 9 additions & 11 deletions core/src/main/java/bisq/core/support/dispute/Dispute.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public static protobuf.Dispute.State toProtoMessage(Dispute.State state) {
private transient long payoutTxConfirms = -1;

private transient final BooleanProperty isClosedProperty = new SimpleBooleanProperty();
private transient final IntegerProperty alertCountProperty = new SimpleIntegerProperty();
private transient final IntegerProperty badgeCountProperty = new SimpleIntegerProperty();


///////////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -340,12 +340,6 @@ public void reOpen() {
setState(State.REOPENED);
}

public void clearNewFlag() {
if (this.disputeState == State.NEW) {
this.disputeState = State.OPEN;
}
}

public void setState(Dispute.State disputeState) {
this.disputeState = disputeState;
this.isClosedProperty.set(disputeState == State.CLOSED);
Expand Down Expand Up @@ -376,8 +370,8 @@ public String getShortTradeId() {
public ReadOnlyBooleanProperty isClosedProperty() {
return isClosedProperty;
}
public ReadOnlyIntegerProperty getAlertCountProperty() {
return alertCountProperty;
public ReadOnlyIntegerProperty getBadgeCountProperty() {
return badgeCountProperty;
}
public ReadOnlyObjectProperty<DisputeResult> disputeResultProperty() {
return disputeResultProperty;
Expand Down Expand Up @@ -405,8 +399,12 @@ public boolean isClosed() {

public void refreshAlertLevel(boolean senderFlag) {
// if the dispute is "new" that is 1 alert that has to be propagated upstream
// unread message count also has to be propagated upstream
alertCountProperty.setValue((isNew() ? 1 : 0) + unreadMessageCount(senderFlag));
// or if there are unread messages that is 1 alert that has to be propagated upstream
if (isNew() || unreadMessageCount(senderFlag) > 0) {
badgeCountProperty.setValue(1);
} else {
badgeCountProperty.setValue(0);
}
}

public long unreadMessageCount(boolean senderFlag) {
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/bisq/core/support/dispute/DisputeListService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,13 @@ private void onDisputesChangeListener(List<? extends Dispute> addedList,
});
}
addedList.forEach(dispute -> {
// for each dispute added, keep track of its "AlertCountProperty"
EasyBind.subscribe(dispute.getAlertCountProperty(),
// for each dispute added, keep track of its "BadgeCountProperty"
EasyBind.subscribe(dispute.getBadgeCountProperty(),
isAlerting -> {
// We get the event before the list gets updated, so we execute on next frame
UserThread.execute(() -> {
int numAlerts = (int) disputeList.getList().stream()
.mapToLong(x -> x.getAlertCountProperty().getValue())
.mapToLong(x -> x.getBadgeCountProperty().getValue())
.sum();
numOpenDisputes.set(numAlerts);
});
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/bisq/core/support/dispute/DisputeManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -859,13 +859,12 @@ private void addMediationResultMessage(Dispute dispute) {
}

public void addMediationReOpenedMessage(Dispute dispute, boolean senderIsTrader) {
String message = "Dispute ticket has been re-opened.";
ChatMessage chatMessage = new ChatMessage(
getSupportType(),
dispute.getTradeId(),
dispute.getTraderId(),
senderIsTrader,
message,
Res.get("support.info.disputeReOpened"),
p2PService.getAddress());
chatMessage.setSystemMessage(false);
dispute.addAndPersistChatMessage(chatMessage);
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/resources/i18n/displayStrings.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,8 @@ support.warning.disputesWithInvalidDonationAddress=The delayed payout transactio
{3}
support.warning.disputesWithInvalidDonationAddress.mediator=\n\nDo you still want to close the dispute?
support.warning.disputesWithInvalidDonationAddress.refundAgent=\n\nYou must not do the payout.

support.warning.traderCloseOwnDisputeWarning=Traders can only self-close their support tickets when the trade has been paid out.
support.info.disputeReOpened=Dispute ticket has been re-opened.

####################################################################
# Settings
Expand Down
42 changes: 2 additions & 40 deletions desktop/src/main/java/bisq/desktop/main/overlays/windows/DisputeSummaryWindow.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ public class DisputeSummaryWindow extends Overlay<DisputeSummaryWindow> {
private final TradeWalletService tradeWalletService;
private final BtcWalletService btcWalletService;
private final TxFeeEstimationService txFeeEstimationService;
// PR5160 will add private final MempoolService mempoolService;
private final DaoFacade daoFacade;
private Dispute dispute;
private Optional<Runnable> finalizeDisputeHandlerOptional = Optional.empty();
Expand All @@ -121,7 +120,6 @@ public class DisputeSummaryWindow extends Overlay<DisputeSummaryWindow> {
// Dispute object of other trade peer. The dispute field is the one from which we opened the close dispute window.
private Optional<Dispute> peersDisputeOptional;
private String role;
private Label delayedPayoutTxStatus;
private TextArea summaryNotesTextArea;

private ChangeListener<Boolean> customRadioButtonSelectedListener;
Expand All @@ -143,7 +141,6 @@ public DisputeSummaryWindow(@Named(FormattingUtils.BTC_FORMATTER_KEY) CoinFormat
TradeWalletService tradeWalletService,
BtcWalletService btcWalletService,
TxFeeEstimationService txFeeEstimationService,
// PR5160 will add MempoolService mempoolService,
DaoFacade daoFacade) {

this.formatter = formatter;
Expand All @@ -152,7 +149,6 @@ public DisputeSummaryWindow(@Named(FormattingUtils.BTC_FORMATTER_KEY) CoinFormat
this.tradeWalletService = tradeWalletService;
this.btcWalletService = btcWalletService;
this.txFeeEstimationService = txFeeEstimationService;
// PR5160 will add this.mempoolService = mempoolService;
this.daoFacade = daoFacade;

type = Type.Confirmation;
Expand All @@ -165,7 +161,6 @@ public void show(Dispute dispute) {
width = 1150;
createGridPane();
addContent();
checkDelayedPayoutTransaction();
display();

if (DevEnv.isDevMode()) {
Expand Down Expand Up @@ -308,9 +303,9 @@ private void addInfoPane() {
if (isMediationDispute) {
if (dispute.getTradePeriodEnd().getTime() > 0) {
String status = DisplayUtils.formatDateTime(dispute.getTradePeriodEnd());
Label tpe = addConfirmationLabelLabel(gridPane, ++rowIndex, Res.get("disputeSummaryWindow.tradePeriodEnd"), status).second;
Label tradePeriodEnd = addConfirmationLabelLabel(gridPane, ++rowIndex, Res.get("disputeSummaryWindow.tradePeriodEnd"), status).second;
if (dispute.getTradePeriodEnd().toInstant().isAfter(Instant.now())) {
tpe.getStyleClass().add("version-new"); // highlight field when the trade period is still active
tradePeriodEnd.getStyleClass().add("version-new"); // highlight field when the trade period is still active
}
}
if (dispute.getExtraDataMap() != null && dispute.getExtraDataMap().size() > 0) {
Expand All @@ -320,8 +315,6 @@ private void addInfoPane() {
}
addConfirmationLabelLabelWithCopyIcon(gridPane, ++rowIndex, Res.get("disputeSummaryWindow.extraInfo"), extraDataSummary);
}
} else { // it is arbitration, show the delayed payout status
delayedPayoutTxStatus = addConfirmationLabelLabel(gridPane, ++rowIndex, Res.get("disputeSummaryWindow.delayedPayoutStatus"), "Checking...").second;
}
}

Expand Down Expand Up @@ -648,10 +641,6 @@ private void addButtons(Contract contract) {
Button cancelButton = tuple.second;

closeTicketButton.setOnAction(e -> {
if (dispute.getPayoutTxConfirms() == 0) {
log.warn("dispute payout tx is not confirmed");
return;
}
if (dispute.getDepositTxSerialized() == null) {
log.warn("dispute.getDepositTxSerialized is null");
return;
Expand Down Expand Up @@ -983,31 +972,4 @@ private void applyTradeAmountRadioButtonStates() {
customRadioButton.setSelected(true);
}
}

private void checkDelayedPayoutTransaction() {
if (dispute.getDelayedPayoutTxId() == null)
return;
if (dispute.getPayoutTxConfirms() < 1) {
log.warn("TODO: // PR5160 will add Mempool check of DelayedPayoutTxId");
/*
mempoolService.checkTxIsConfirmed(dispute.getDelayedPayoutTxId(), (status -> {
log.warn("Mempool check confirmation status of DelayedPayoutTxId returned: [{}]", status);
dispute.setPayoutTxConfirms(status);
displayPayoutStatus(status);
}));
*/
}
displayPayoutStatus(dispute.getPayoutTxConfirms());
}

private void displayPayoutStatus(long nConfirmStatus) {
if (delayedPayoutTxStatus != null) {
String status = Res.get("confidence.unknown");
if (nConfirmStatus == 0)
status = Res.get("confidence.seen", 1);
else if (nConfirmStatus > 0)
status = Res.get("confidence.confirmed", nConfirmStatus);
delayedPayoutTxStatus.setText(status);
}
}
}
8 changes: 4 additions & 4 deletions desktop/src/main/java/bisq/desktop/main/shared/ChatView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,12 @@ public class ChatView extends AnchorPane {
private EventHandler<KeyEvent> keyEventEventHandler;
private SupportManager supportManager;
private Optional<SupportSession> optionalSupportSession = Optional.empty();
private String cptyName;
private String counterpartyName;

public ChatView(SupportManager supportManager, CoinFormatter formatter, String cptyName) {
public ChatView(SupportManager supportManager, CoinFormatter formatter, String counterpartyName) {
this.supportManager = supportManager;
this.formatter = formatter;
this.cptyName = cptyName;
this.counterpartyName = counterpartyName;
allowAttachments = true;
displayHeader = true;
}
Expand Down Expand Up @@ -419,7 +419,7 @@ protected void updateItem(ChatMessage message, boolean empty) {
String metaData = DisplayUtils.formatDateTime(new Date(message.getDate()));
if (!message.isSystemMessage())
metaData = (isMyMsg ? "Sent " : "Received ") + metaData
+ (isMyMsg ? "" : " from " + cptyName);
+ (isMyMsg ? "" : " from " + counterpartyName);
headerLabel.setText(metaData);
messageLabel.setText(message.getMessage());
attachmentsBox.getChildren().clear();
Expand Down
4 changes: 2 additions & 2 deletions desktop/src/main/java/bisq/desktop/main/support/dispute/DisputeChatPopup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@ public void closeChat() {
chatPopupStage.close();
}

public void openChat(Dispute selectedDispute, DisputeSession concreteDisputeSession, String cptyName) {
public void openChat(Dispute selectedDispute, DisputeSession concreteDisputeSession, String counterpartyName) {
closeChat();
selectedDispute.getChatMessages().forEach(m -> m.setWasDisplayed(true));
disputeManager.requestPersistence();

ChatView chatView = new ChatView(disputeManager, formatter, cptyName);
ChatView chatView = new ChatView(disputeManager, formatter, counterpartyName);
chatView.setAllowAttachments(true);
chatView.setDisplayHeader(false);
chatView.initialize();
Expand Down
22 changes: 19 additions & 3 deletions desktop/src/main/java/bisq/desktop/main/support/dispute/DisputeView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
import javafx.scene.control.Label;
import javafx.scene.control.TableCell;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableRow;
import javafx.scene.control.TableView;
import javafx.scene.control.Tooltip;
import javafx.scene.layout.HBox;
Expand Down Expand Up @@ -330,6 +331,19 @@ protected void activate() {
sortedList.comparatorProperty().bind(tableView.comparatorProperty());
tableView.setItems(sortedList);

// double-click on a row clears its "new" badge
tableView.setRowFactory( tv -> {
TableRow<Dispute> row = new TableRow<>();
row.setOnMouseClicked(event -> {
if (event.getClickCount() == 2 && (!row.isEmpty())) {
Dispute dispute = row.getItem();
dispute.setDisputeSeen(senderFlag());
newBadgeByDispute.get(dispute.getId()).setVisible(dispute.isNew());
}
});
return row;
});

selectedDisputeSubscription = EasyBind.subscribe(tableView.getSelectionModel().selectedItemProperty(), this::onSelectDispute);

Dispute selectedItem = tableView.getSelectionModel().getSelectedItem();
Expand Down Expand Up @@ -495,6 +509,8 @@ protected void closeDisputeFromButton() {
selectedDispute.setIsClosed();
disputeManager.requestPersistence();
onSelectDispute(selectedDispute);
} else {
new Popup().warning(Res.get("support.warning.traderCloseOwnDisputeWarning")).show();
}
}

Expand Down Expand Up @@ -971,7 +987,7 @@ public void updateItem(final Dispute item, boolean empty) {
button.setOnAction(e -> {
tableView.getSelectionModel().select(this.getIndex());
onOpenContract(item);
item.clearNewFlag();
item.setDisputeSeen(senderFlag());
badge.setVisible(item.isNew());
});
} else {
Expand Down Expand Up @@ -1007,7 +1023,7 @@ public void updateItem(final Dispute item, boolean empty) {
button.setOnAction(e -> {
tableView.getSelectionModel().select(this.getIndex());
handleOnProcessDispute(item);
item.clearNewFlag();
item.setDisputeSeen(senderFlag());
newBadgeByDispute.get(item.getId()).setVisible(item.isNew());
});
HBox hBox = new HBox(button);
Expand Down Expand Up @@ -1062,7 +1078,7 @@ public void updateItem(final Dispute item, boolean empty) {
button.setOnAction(e -> {
tableView.getSelectionModel().select(this.getIndex());
chatPopup.openChat(item, getConcreteDisputeChatSession(item), getCounterpartyName());
item.clearNewFlag();
item.setDisputeSeen(senderFlag());
newBadgeByDispute.get(id).setVisible(item.isNew());
updateChatMessageCount(item, chatBadge);
});
Expand Down

0 comments on commit ad4927a

Please sign in to comment.