Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve dispute views #4446

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
import bisq.core.notifications.MobileMessageType;
import bisq.core.notifications.MobileNotificationService;
import bisq.core.support.dispute.Dispute;
import bisq.core.support.dispute.arbitration.ArbitrationManager;
import bisq.core.support.dispute.mediation.MediationManager;
import bisq.core.support.dispute.refund.RefundManager;
import bisq.core.support.messages.ChatMessage;

import bisq.network.p2p.P2PService;
Expand All @@ -31,6 +32,7 @@
import javax.inject.Singleton;

import javafx.collections.ListChangeListener;
import javafx.collections.ObservableList;

import java.util.UUID;

Expand All @@ -39,28 +41,38 @@
@Slf4j
@Singleton
public class DisputeMsgEvents {
private final RefundManager refundManager;
private final MediationManager mediationManager;
private final P2PService p2PService;
private final MobileNotificationService mobileNotificationService;

@Inject
public DisputeMsgEvents(ArbitrationManager arbitrationManager,
public DisputeMsgEvents(RefundManager refundManager,
MediationManager mediationManager,
P2PService p2PService,
MobileNotificationService mobileNotificationService) {
this.refundManager = refundManager;
this.mediationManager = mediationManager;
this.p2PService = p2PService;
this.mobileNotificationService = mobileNotificationService;
}

// We need to handle it here in the constructor otherwise we get repeated the messages sent.
arbitrationManager.getDisputesAsObservableList().addListener((ListChangeListener<Dispute>) c -> {
public void onAllServicesInitialized() {
refundManager.getDisputesAsObservableList().addListener((ListChangeListener<Dispute>) c -> {
c.next();
if (c.wasAdded()) {
c.getAddedSubList().forEach(this::setDisputeListener);
}
});
arbitrationManager.getDisputesAsObservableList().forEach(this::setDisputeListener);
}
refundManager.getDisputesAsObservableList().forEach(this::setDisputeListener);

// We ignore that onAllServicesInitialized here
public void onAllServicesInitialized() {
mediationManager.getDisputesAsObservableList().addListener((ListChangeListener<Dispute>) c -> {
c.next();
if (c.wasAdded()) {
c.getAddedSubList().forEach(this::setDisputeListener);
}
});
mediationManager.getDisputesAsObservableList().forEach(this::setDisputeListener);
}

private void setDisputeListener(Dispute dispute) {
Expand All @@ -70,24 +82,24 @@ private void setDisputeListener(Dispute dispute) {
log.debug("We got a ChatMessage added. id={}, tradeId={}", dispute.getId(), dispute.getTradeId());
c.next();
if (c.wasAdded()) {
c.getAddedSubList().forEach(this::setChatMessage);
c.getAddedSubList().forEach(chatMessage -> onChatMessage(chatMessage, dispute));
}
});

//TODO test
if (!dispute.getChatMessages().isEmpty())
setChatMessage(dispute.getChatMessages().get(0));
onChatMessage(dispute.getChatMessages().get(0), dispute);
}

private void setChatMessage(ChatMessage disputeMsg) {
private void onChatMessage(ChatMessage chatMessage, Dispute dispute) {
// TODO we need to prevent to send msg for old dispute messages again at restart
// Maybe we need a new property in ChatMessage
// As key is not set in initial iterations it seems we don't need an extra handling.
// the mailbox msg is set a bit later so that triggers a notification, but not the old messages.

// We only send msg in case we are not the sender
if (!disputeMsg.getSenderNodeAddress().equals(p2PService.getAddress())) {
String shortId = disputeMsg.getShortId();
if (!chatMessage.getSenderNodeAddress().equals(p2PService.getAddress())) {
String shortId = chatMessage.getShortId();
MobileMessage message = new MobileMessage(Res.get("account.notifications.dispute.message.title"),
Res.get("account.notifications.dispute.message.msg", shortId),
shortId,
Expand All @@ -99,6 +111,16 @@ private void setChatMessage(ChatMessage disputeMsg) {
e.printStackTrace();
}
}

// We check at every new message if it might be a message sent after the dispute had been closed. If that is the
// case we revert the isClosed flag so that the UI can reopen the dispute and indicate that a new dispute
// message arrived.
ObservableList<ChatMessage> chatMessages = dispute.getChatMessages();
// If last message is not a result message we re-open as we might have received a new message from the
// trader/mediator/arbitrator who has reopened the case
if (dispute.isClosed() && !chatMessages.isEmpty() && !chatMessages.get(chatMessages.size() - 1).isResultMessage(dispute)) {
dispute.setIsClosed(false);
}
}

public static MobileMessage getTestMsg() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,8 @@ public void sendDisputeResultMessage(DisputeResult disputeResult, Dispute disput
text,
p2PService.getAddress());

dispute.addAndPersistChatMessage(chatMessage);
disputeResult.setChatMessage(chatMessage);
dispute.addAndPersistChatMessage(chatMessage);

NodeAddress peersNodeAddress;
Contract contract = dispute.getContract();
Expand Down
13 changes: 9 additions & 4 deletions core/src/main/java/bisq/core/support/dispute/DisputeResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,15 @@ public enum Reason {
OTHER,
BUG,
USABILITY,
SCAM,
PROTOCOL_VIOLATION,
NO_REPLY,
BANK_PROBLEMS
SCAM, // Not used anymore
PROTOCOL_VIOLATION, // Not used anymore
NO_REPLY, // Not used anymore
BANK_PROBLEMS,
OPTION_TRADE,
SELLER_NOT_RESPONDING,
WRONG_SENDER_ACCOUNT,
TRADE_ALREADY_SETTLED,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the TRADE_ALREADY_SETTLED is badly needed

PEER_WAS_LATE
}

private final String tradeId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,6 @@ public void onDisputeResultMessage(DisputeResultMessage disputeResultMessage) {
}
dispute.setIsClosed(true);

if (dispute.disputeResultProperty().get() != null) {
log.warn("We got already a dispute result. That should only happen if a dispute needs to be closed " +
"again because the first close did not succeed. TradeId = " + tradeId);
}

dispute.setDisputeResult(disputeResult);

Optional<Trade> tradeOptional = tradeManager.getTradeById(tradeId);
Expand Down
13 changes: 12 additions & 1 deletion core/src/main/java/bisq/core/support/messages/ChatMessage.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import bisq.core.support.SupportType;
import bisq.core.support.dispute.Attachment;
import bisq.core.support.dispute.Dispute;
import bisq.core.support.dispute.DisputeResult;

import bisq.network.p2p.NodeAddress;

Expand Down Expand Up @@ -59,7 +61,6 @@
@Getter
@Slf4j
public final class ChatMessage extends SupportMessage {

public interface Listener {
void onMessageStateChanged();
}
Expand Down Expand Up @@ -328,6 +329,16 @@ public void addWeakMessageStateListener(Listener listener) {
this.listener = new WeakReference<>(listener);
}

public boolean isResultMessage(Dispute dispute) {
DisputeResult disputeResult = dispute.getDisputeResultProperty().get();
if (disputeResult == null) {
return false;
}

ChatMessage resultChatMessage = disputeResult.getChatMessage();
return resultChatMessage != null && resultChatMessage.getUid().equals(uid);
}

private void notifyChangeListener() {
if (listener != null) {
Listener listener = this.listener.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@

package bisq.core.trade.protocol.tasks.taker;

import bisq.core.btc.wallet.Restrictions;
import bisq.core.trade.Trade;
import bisq.core.trade.messages.InputsForDepositTxResponse;
import bisq.core.trade.protocol.TradingPeer;
import bisq.core.trade.protocol.tasks.TradeTask;

import bisq.common.config.Config;
import bisq.common.taskrunner.TaskRunner;

import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -58,8 +60,15 @@ protected void run() {
byte[] preparedDepositTx = inputsForDepositTxResponse.getPreparedDepositTx();
processModel.setPreparedDepositTx(checkNotNull(preparedDepositTx));
long lockTime = inputsForDepositTxResponse.getLockTime();
//todo for dev testing deactivated
//checkArgument(lockTime >= processModel.getBtcWalletService().getBestChainHeight() + 144 * 20);
if (Config.baseCurrencyNetwork().isMainnet()) {
int myLockTime = processModel.getBtcWalletService().getBestChainHeight() +
Restrictions.getLockTime(processModel.getOffer().getPaymentMethod().isAsset());
// We allow a tolerance of 3 blocks as BestChainHeight might be a bit different on maker and taker in case a new
// block was just found
checkArgument(Math.abs(lockTime - myLockTime) <= 3,
"Lock time of maker is more than 3 blocks different to the locktime I " +
"calculated. Makers lockTime= " + lockTime + ", myLockTime=" + myLockTime);
}
trade.setLockTime(lockTime);
log.info("lockTime={}, delay={}", lockTime, (processModel.getBtcWalletService().getBestChainHeight() - lockTime));

Expand Down
13 changes: 12 additions & 1 deletion core/src/main/resources/i18n/displayStrings.properties
Original file line number Diff line number Diff line change
Expand Up @@ -965,8 +965,13 @@ support.tab.mediation.support=Mediation
support.tab.arbitration.support=Arbitration
support.tab.legacyArbitration.support=Legacy Arbitration
support.tab.ArbitratorsSupportTickets={0}'s tickets
support.filter=Filter list
support.filter=Search disputes
support.filter.prompt=Enter trade ID, date, onion address or account data
support.reOpenByTrader.prompt=Are you sure you want to re-open the dispute?
support.reOpenButton.label=Re-open dispute
support.sendNotificationButton.label=Send private notification
support.reportButton.label=Generate report
support.fullReportButton.label=Get text dump of all disputes
support.noTickets=There are no open tickets
support.sendingMessage=Sending Message...
support.receiverNotOnline=Receiver is not online. Message is saved to their mailbox.
Expand Down Expand Up @@ -2376,6 +2381,12 @@ disputeSummaryWindow.reason.noReply=No reply
disputeSummaryWindow.reason.scam=Scam
disputeSummaryWindow.reason.other=Other
disputeSummaryWindow.reason.bank=Bank
disputeSummaryWindow.reason.optionTrade=Option trade
disputeSummaryWindow.reason.sellerNotResponding=Seller not responding
disputeSummaryWindow.reason.wrongSenderAccount=Wrong sender account
disputeSummaryWindow.reason.peerWasLate=Peer was late
disputeSummaryWindow.reason.tradeAlreadySettled=Trade already settled

disputeSummaryWindow.summaryNotes=Summary notes
disputeSummaryWindow.addSummaryNotes=Add summary notes
disputeSummaryWindow.close.button=Close ticket
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,10 @@ public class DisputeSummaryWindow extends Overlay<DisputeSummaryWindow> {
buyerGetsAllRadioButton, sellerGetsAllRadioButton, customRadioButton;
private RadioButton reasonWasBugRadioButton, reasonWasUsabilityIssueRadioButton,
reasonProtocolViolationRadioButton, reasonNoReplyRadioButton, reasonWasScamRadioButton,
reasonWasOtherRadioButton, reasonWasBankRadioButton;
reasonWasOtherRadioButton, reasonWasBankRadioButton, reasonWasOptionTradeRadioButton,
reasonWasSellerNotRespondingRadioButton, reasonWasWrongSenderAccountRadioButton,
reasonWasPeerWasLateRadioButton, reasonWasTradeAlreadySettledRadioButton;

// 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;
Expand Down Expand Up @@ -155,7 +158,7 @@ public void show(Dispute dispute) {
this.dispute = dispute;

rowIndex = -1;
width = 700;
width = 1150;
createGridPane();
addContent();
display();
Expand Down Expand Up @@ -209,7 +212,7 @@ protected void createGridPane() {
gridPane.setPadding(new Insets(35, 40, 30, 40));
gridPane.getStyleClass().add("grid-pane");
gridPane.getColumnConstraints().get(0).setHalignment(HPos.LEFT);
gridPane.setPrefWidth(900);
gridPane.setPrefWidth(width);
}

private void addContent() {
Expand Down Expand Up @@ -258,6 +261,11 @@ private void addContent() {
reasonWasScamRadioButton.setDisable(true);
reasonWasOtherRadioButton.setDisable(true);
reasonWasBankRadioButton.setDisable(true);
reasonWasOptionTradeRadioButton.setDisable(true);
reasonWasSellerNotRespondingRadioButton.setDisable(true);
reasonWasWrongSenderAccountRadioButton.setDisable(true);
reasonWasPeerWasLateRadioButton.setDisable(true);
reasonWasTradeAlreadySettledRadioButton.setDisable(true);

isLoserPublisherCheckBox.setDisable(true);
isLoserPublisherCheckBox.setSelected(peersDisputeResult.isLoserPublisher());
Expand Down Expand Up @@ -485,12 +493,27 @@ private void addReasonControls() {
reasonWasScamRadioButton = new AutoTooltipRadioButton(Res.get("disputeSummaryWindow.reason.scam"));
reasonWasBankRadioButton = new AutoTooltipRadioButton(Res.get("disputeSummaryWindow.reason.bank"));
reasonWasOtherRadioButton = new AutoTooltipRadioButton(Res.get("disputeSummaryWindow.reason.other"));
reasonWasOptionTradeRadioButton = new AutoTooltipRadioButton(Res.get("disputeSummaryWindow.reason.optionTrade"));
reasonWasSellerNotRespondingRadioButton = new AutoTooltipRadioButton(Res.get("disputeSummaryWindow.reason.sellerNotResponding"));
reasonWasWrongSenderAccountRadioButton = new AutoTooltipRadioButton(Res.get("disputeSummaryWindow.reason.wrongSenderAccount"));
reasonWasPeerWasLateRadioButton = new AutoTooltipRadioButton(Res.get("disputeSummaryWindow.reason.peerWasLate"));
reasonWasTradeAlreadySettledRadioButton = new AutoTooltipRadioButton(Res.get("disputeSummaryWindow.reason.tradeAlreadySettled"));

HBox feeRadioButtonPane = new HBox();
feeRadioButtonPane.setSpacing(20);
feeRadioButtonPane.getChildren().addAll(reasonWasBugRadioButton, reasonWasUsabilityIssueRadioButton,
reasonProtocolViolationRadioButton, reasonNoReplyRadioButton,
reasonWasBankRadioButton, reasonWasScamRadioButton, reasonWasOtherRadioButton);
// We don't show no reply and protocol violation as those should be covered by more specific ones. We still leave
// the code to enable it if it turns out it is still requested by mediators.
feeRadioButtonPane.getChildren().addAll(
reasonWasTradeAlreadySettledRadioButton,
reasonWasPeerWasLateRadioButton,
reasonWasOptionTradeRadioButton,
reasonWasSellerNotRespondingRadioButton,
reasonWasWrongSenderAccountRadioButton,
reasonWasBugRadioButton,
reasonWasUsabilityIssueRadioButton,
reasonWasBankRadioButton,
reasonWasOtherRadioButton
);

VBox vBox = addTopLabelWithVBox(gridPane, ++rowIndex,
Res.get("disputeSummaryWindow.reason"),
Expand All @@ -505,22 +528,38 @@ private void addReasonControls() {
reasonWasScamRadioButton.setToggleGroup(reasonToggleGroup);
reasonWasOtherRadioButton.setToggleGroup(reasonToggleGroup);
reasonWasBankRadioButton.setToggleGroup(reasonToggleGroup);
reasonWasOptionTradeRadioButton.setToggleGroup(reasonToggleGroup);
reasonWasSellerNotRespondingRadioButton.setToggleGroup(reasonToggleGroup);
reasonWasWrongSenderAccountRadioButton.setToggleGroup(reasonToggleGroup);
reasonWasPeerWasLateRadioButton.setToggleGroup(reasonToggleGroup);
reasonWasTradeAlreadySettledRadioButton.setToggleGroup(reasonToggleGroup);

reasonToggleSelectionListener = (observable, oldValue, newValue) -> {
if (newValue == reasonWasBugRadioButton)
if (newValue == reasonWasBugRadioButton) {
disputeResult.setReason(DisputeResult.Reason.BUG);
else if (newValue == reasonWasUsabilityIssueRadioButton)
} else if (newValue == reasonWasUsabilityIssueRadioButton) {
disputeResult.setReason(DisputeResult.Reason.USABILITY);
else if (newValue == reasonProtocolViolationRadioButton)
} else if (newValue == reasonProtocolViolationRadioButton) {
disputeResult.setReason(DisputeResult.Reason.PROTOCOL_VIOLATION);
else if (newValue == reasonNoReplyRadioButton)
} else if (newValue == reasonNoReplyRadioButton) {
disputeResult.setReason(DisputeResult.Reason.NO_REPLY);
else if (newValue == reasonWasScamRadioButton)
} else if (newValue == reasonWasScamRadioButton) {
disputeResult.setReason(DisputeResult.Reason.SCAM);
else if (newValue == reasonWasBankRadioButton)
} else if (newValue == reasonWasBankRadioButton) {
disputeResult.setReason(DisputeResult.Reason.BANK_PROBLEMS);
else if (newValue == reasonWasOtherRadioButton)
} else if (newValue == reasonWasOtherRadioButton) {
disputeResult.setReason(DisputeResult.Reason.OTHER);
} else if (newValue == reasonWasOptionTradeRadioButton) {
disputeResult.setReason(DisputeResult.Reason.OPTION_TRADE);
} else if (newValue == reasonWasSellerNotRespondingRadioButton) {
disputeResult.setReason(DisputeResult.Reason.SELLER_NOT_RESPONDING);
} else if (newValue == reasonWasWrongSenderAccountRadioButton) {
disputeResult.setReason(DisputeResult.Reason.WRONG_SENDER_ACCOUNT);
} else if (newValue == reasonWasTradeAlreadySettledRadioButton) {
disputeResult.setReason(DisputeResult.Reason.TRADE_ALREADY_SETTLED);
} else if (newValue == reasonWasPeerWasLateRadioButton) {
disputeResult.setReason(DisputeResult.Reason.PEER_WAS_LATE);
}
};
reasonToggleGroup.selectedToggleProperty().addListener(reasonToggleSelectionListener);
}
Expand Down Expand Up @@ -549,6 +588,21 @@ private void setReasonRadioButtonState() {
case OTHER:
reasonToggleGroup.selectToggle(reasonWasOtherRadioButton);
break;
case OPTION_TRADE:
reasonToggleGroup.selectToggle(reasonWasOptionTradeRadioButton);
break;
case SELLER_NOT_RESPONDING:
reasonToggleGroup.selectToggle(reasonWasSellerNotRespondingRadioButton);
break;
case WRONG_SENDER_ACCOUNT:
reasonToggleGroup.selectToggle(reasonWasWrongSenderAccountRadioButton);
break;
case PEER_WAS_LATE:
reasonToggleGroup.selectToggle(reasonWasPeerWasLateRadioButton);
break;
case TRADE_ALREADY_SETTLED:
reasonToggleGroup.selectToggle(reasonWasTradeAlreadySettledRadioButton);
break;
}
}
}
Expand Down
Loading