From d5349490f5231126ae660855bceb2e5c8a4b5825 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Wed, 31 Jan 2024 10:29:50 -0500
Subject: [PATCH] conversation: follow daemon changes for sending status
cf jami-docs/developer/delivery-status
+ Basically this patch uses the new SwarmMessage.status to show
current interaction status.
+ setRead only updates the status if the interaction is newer (else,
because signal are not guaranteed to be ordered, this can cause the
lastDisplayed_ to be incorrect).
+ Some old code is removed and unused signal
+ MessageReceived updates status if needed
GitLab: #1487
Change-Id: I4d4d5dce8dc12ab638e89e3f8431810b29a72087
---
daemon | 2 +-
src/libclient/api/conversationmodel.h | 11 --
src/libclient/api/interaction.h | 16 ++
src/libclient/conversationmodel.cpp | 176 ++++--------------
src/libclient/dbus/metatypes.h | 2 +
src/libclient/messagelistmodel.cpp | 15 +-
.../qtwrapper/configurationmanager_wrap.h | 9 +-
src/libclient/qtwrapper/conversions_wrap.hpp | 20 ++
src/libclient/typedefs.h | 1 +
9 files changed, 97 insertions(+), 155 deletions(-)
diff --git a/daemon b/daemon
index 66d810026..09ec8e6f4 160000
--- a/daemon
+++ b/daemon
@@ -1 +1 @@
-Subproject commit 66d81002647f712d1ee66aa8b78c3eefcc747fda
+Subproject commit 09ec8e6f43ea5c68742bffb5e40ece15fa687c46
diff --git a/src/libclient/api/conversationmodel.h b/src/libclient/api/conversationmodel.h
index edccb2121..414ba3fbf 100644
--- a/src/libclient/api/conversationmodel.h
+++ b/src/libclient/api/conversationmodel.h
@@ -513,17 +513,6 @@ Q_SIGNALS:
void composingStatusChanged(const QString& uid,
const QString& contactUri,
bool isComposing) const;
- /**
- * Emitted when last displayed interaction changed
- * @param uid of conversation
- * @param participant URI
- * @param previousUid uid of a previous displayed interaction
- * @param newdUid uid of a new displayed interaction
- */
- void displayedInteractionChanged(const QString& uid,
- const QString& participantURI,
- const QString& previousUid,
- const QString& newdUid) const;
/**
* Emitted when search status changed
diff --git a/src/libclient/api/interaction.h b/src/libclient/api/interaction.h
index 8d73329f5..12dedbb94 100644
--- a/src/libclient/api/interaction.h
+++ b/src/libclient/api/interaction.h
@@ -465,6 +465,22 @@ struct Info
}
for (auto i = mapStringEmoji.begin(); i != mapStringEmoji.end(); i++)
reactions.insert(i.key(), i.value());
+ // Compute the status of the message.
+ // Basically, we got the status per member.
+ // We consider the message as sent if at least one member has received it or displayed if someone displayed it.
+ auto maxStatus = 0;
+ status = Status::SENDING;
+ for (const auto& member: msg.status.keys()) {
+ if (member == accountUri)
+ continue;
+ auto stValue = msg.status.value(member);
+ if (stValue > maxStatus) {
+ maxStatus = stValue;
+ status = maxStatus <= 1 ? Status::SENDING : (stValue == 2 ? Status::SUCCESS : Status::DISPLAYED);
+ }
+ if (maxStatus == 3)
+ break;
+ }
}
};
diff --git a/src/libclient/conversationmodel.cpp b/src/libclient/conversationmodel.cpp
index 277804e4c..677bcaaa8 100644
--- a/src/libclient/conversationmodel.cpp
+++ b/src/libclient/conversationmodel.cpp
@@ -2433,6 +2433,14 @@ ConversationModelPimpl::slotMessageReceived(const QString& accountId,
QString(message.body.value("totalSize")).toInt());
}
Q_EMIT linked.dataChanged(indexOf(conversationId));
+ // Update status
+ using namespace libjami::Account;
+ for (const auto& uri : message.status.keys()) {
+ if (uri == linked.owner.profileInfo.uri)
+ continue;
+ if (message.status.value(uri) == static_cast<int>(MessageStates::DISPLAYED))
+ conversation.interactions->setRead(uri, message.id);
+ }
} catch (const std::exception& e) {
qDebug() << "messages received for not existing conversation";
}
@@ -2852,7 +2860,7 @@ ConversationModelPimpl::addConversationRequest(const MapStringString& convReques
void
ConversationModelPimpl::slotPendingContactAccepted(const QString& uri)
{
- profile::Type type {};
+ profile::Type type = profile::Type::INVALID;
try {
type = linked.owner.contactModel->getContact(uri).profileInfo.type;
} catch (std::out_of_range& e) {
@@ -3470,143 +3478,39 @@ ConversationModelPimpl::updateInteractionStatus(const QString& accountId,
if (accountId != linked.owner.id) {
return;
}
- // non-swarm conversation
- if (conversationId.isEmpty() || conversationId == linked.owner.profileInfo.uri) {
- auto convIds = storage::getConversationsWithPeer(db, peerUri);
- if (convIds.empty()) {
- return;
- }
- auto conversationIdx = indexOf(convIds[0]);
- auto& conversation = conversations[conversationIdx];
- auto newStatus = interaction::Status::INVALID;
- switch (static_cast<libjami::Account::MessageStates>(status)) {
- case libjami::Account::MessageStates::SENDING:
- newStatus = interaction::Status::SENDING;
- break;
- case libjami::Account::MessageStates::CANCELLED:
- newStatus = interaction::Status::TRANSFER_CANCELED;
- break;
- case libjami::Account::MessageStates::SENT:
- newStatus = interaction::Status::SUCCESS;
- break;
- case libjami::Account::MessageStates::FAILURE:
- newStatus = interaction::Status::FAILURE;
- break;
- case libjami::Account::MessageStates::DISPLAYED:
- newStatus = interaction::Status::DISPLAYED;
- break;
- case libjami::Account::MessageStates::UNKNOWN:
- default:
- newStatus = interaction::Status::UNKNOWN;
- break;
- }
- auto idString = messageId;
- // for not swarm conversation messageId in hexdesimal string format. Convert to normal string
- // TODO messageId should be received from daemon in string format
- if (static_cast<libjami::Account::MessageStates>(status)
- == libjami::Account::MessageStates::DISPLAYED) {
- std::istringstream ss(messageId.toStdString());
- ss >> std::hex;
- uint64_t id;
- if (!(ss >> id))
- return;
- idString = QString::number(id);
- }
- // Update database
- auto msgId = storage::getInteractionIdByDaemonId(db, idString);
- if (msgId.isEmpty()) {
- return;
- }
- storage::updateInteractionStatus(db, msgId, newStatus);
- // Update conversations
- bool updated = false;
- bool updateDisplayedUid = false;
- QString oldDisplayedUid = 0;
- {
- auto& interactions = conversation.interactions;
- // Try to update the status.
- if (interactions->updateStatus(msgId, newStatus)) {
- updated = true;
- interactions->with(msgId, [&](const QString& id, interaction::Info& interaction) {
- // Determine if the interaction is outgoing and has been displayed.
- bool interactionIsDisplayed = newStatus == interaction::Status::DISPLAYED
- && interaction::isOutgoing(interaction);
-
- // Get the last displayed interaction ID and timestamp for this peer.
- auto [lastIdForPeer, lastTimestampForPeer]
- = interactions->getDisplayedInfoForPeer(peerUri);
-
- if (lastIdForPeer.isEmpty()) {
- oldDisplayedUid = "";
- if (peerUri != linked.owner.profileInfo.uri)
- conversation.interactions->setRead(peerUri, msgId);
- updateDisplayedUid = true;
- } else {
- bool interactionIsLast = lastTimestampForPeer < interaction.timestamp;
- updateDisplayedUid = interactionIsDisplayed && interactionIsLast;
- if (updateDisplayedUid) {
- oldDisplayedUid = messageId;
- if (peerUri != linked.owner.profileInfo.uri)
- conversation.interactions->setRead(peerUri, msgId);
- }
- }
- });
- }
- }
- if (updateDisplayedUid) {
- Q_EMIT linked.displayedInteractionChanged(conversation.uid,
- peerUri,
- oldDisplayedUid,
- msgId);
- }
- if (updated) {
- invalidateModel();
- }
- return;
- }
- // swarm conversation
try {
auto& conversation = getConversationForUid(conversationId).get();
- if (conversation.isSwarm()) {
- using namespace libjami::Account;
- auto msgState = static_cast<MessageStates>(status);
- auto& interactions = conversation.interactions;
- interactions->with(messageId,
- [&](const QString& id, const interaction::Info& interaction) {
- if (interaction.type == interaction::Type::TEXT) {
- interaction::Status newState;
- if (msgState == MessageStates::SENDING) {
- newState = interaction::Status::SENDING;
- } else if (msgState == MessageStates::SENT) {
- newState = interaction::Status::SUCCESS;
- } else {
- return;
- }
- interactions->updateStatus(id, newState);
- }
- });
-
- if (msgState == MessageStates::DISPLAYED) {
- auto previous = conversation.interactions->getRead(peerUri);
- if (peerUri != linked.owner.profileInfo.uri)
- conversation.interactions->setRead(peerUri, messageId);
- else {
- // Here, this means that the daemon synced the displayed message
- // so, compute the number of unread messages.
- conversation.unreadMessages = ConfigurationManager::instance()
- .countInteractions(linked.owner.id,
- conversationId,
- messageId,
- "",
- peerUri);
- Q_EMIT linked.dataChanged(indexOf(conversationId));
- }
- Q_EMIT linked.displayedInteractionChanged(conversationId,
- peerUri,
- previous,
- messageId);
- }
+ // Proceed only if the conversation is a swarm
+ if (!conversation.isSwarm() || peerUri == linked.owner.profileInfo.uri) {
+ return;
}
+
+ auto emitDisplayed = false;
+ using namespace libjami::Account;
+ auto msgState = static_cast<MessageStates>(status);
+ auto& interactions = conversation.interactions;
+ interactions->with(messageId,
+ [&](const QString& id, const interaction::Info& interaction) {
+ if (interaction.type == interaction::Type::TEXT) {
+ interaction::Status newState;
+ if (msgState == MessageStates::SENDING) {
+ newState = interaction::Status::SENDING;
+ } else if (msgState == MessageStates::SENT) {
+ newState = interaction::Status::SUCCESS;
+ } else if (msgState == MessageStates::DISPLAYED) {
+ newState = interaction::Status::DISPLAYED;
+ } else {
+ return;
+ }
+ if (interactions->updateStatus(id, newState)
+ && newState == interaction::Status::DISPLAYED) {
+ emitDisplayed = true;
+ }
+ }
+ });
+
+ if (emitDisplayed)
+ conversation.interactions->setRead(peerUri, messageId);
} catch (const std::out_of_range& e) {
qDebug() << "could not update message status for not existing conversation";
}
@@ -3999,7 +3903,7 @@ ConversationModelPimpl::acceptTransfer(const QString& convUid, const QString& in
return;
auto& interactions = conversation.interactions;
- if (!interactions->with(interactionId, [&](const QString& id, interaction::Info& interaction) {
+ if (!interactions->with(interactionId, [&](const QString&, interaction::Info& interaction) {
auto fileId = interaction.commit["fileId"];
if (fileId.isEmpty()) {
qWarning() << "Cannot download file without fileId";
diff --git a/src/libclient/dbus/metatypes.h b/src/libclient/dbus/metatypes.h
index 0441d1bc3..a0ecbca75 100644
--- a/src/libclient/dbus/metatypes.h
+++ b/src/libclient/dbus/metatypes.h
@@ -97,6 +97,7 @@ operator<<(QDBusArgument& argument, const SwarmMessage& m)
argument << m.body;
argument << m.reactions;
argument << m.editions;
+ argument << m.status;
argument.endStructure();
return argument;
@@ -112,6 +113,7 @@ operator>>(const QDBusArgument& argument, SwarmMessage& m)
argument >> m.body;
argument >> m.reactions;
argument >> m.editions;
+ argument >> m.status;
argument.endStructure();
return argument;
diff --git a/src/libclient/messagelistmodel.cpp b/src/libclient/messagelistmodel.cpp
index 661e50a8b..e466448ab 100644
--- a/src/libclient/messagelistmodel.cpp
+++ b/src/libclient/messagelistmodel.cpp
@@ -209,6 +209,9 @@ MessageListModel::updateStatus(const QString& id,
return false;
}
VectorInt roles;
+ if (it->second.status == newStatus) {
+ return false;
+ }
it->second.status = newStatus;
roles.push_back(Role::Status);
if (!newBody.isEmpty()) {
@@ -350,13 +353,18 @@ MessageListModel::setRead(const QString& peer, const QString& messageId)
{
std::lock_guard<std::recursive_mutex> lk(mutex_);
auto i = lastDisplayedMessageUid_.find(peer);
+ auto msgIdx = indexOfMessage(messageId);
if (i != lastDisplayedMessageUid_.end()) {
auto old = i.value();
+ auto oldMsgIdx = indexOfMessage(old);
+ if (oldMsgIdx > msgIdx) {
+ // Previous latest read is newer than this message, ignore.
+ return;
+ }
messageToReaders_[old].removeAll(peer);
- auto msgIdx = indexOfMessage(old);
// Remove from latest read
- if (msgIdx != -1) {
- QModelIndex modelIndex = QAbstractListModel::index(msgIdx, 0);
+ if (oldMsgIdx != -1) {
+ QModelIndex modelIndex = QAbstractListModel::index(oldMsgIdx, 0);
Q_EMIT dataChanged(modelIndex, modelIndex, {Role::Readers});
}
}
@@ -364,7 +372,6 @@ MessageListModel::setRead(const QString& peer, const QString& messageId)
lastDisplayedMessageUid_[peer] = messageId;
messageToReaders_[messageId].append(peer);
// update interaction
- auto msgIdx = indexOfMessage(messageId);
// Remove from latest read
if (msgIdx != -1) {
QModelIndex modelIndex = QAbstractListModel::index(msgIdx, 0);
diff --git a/src/libclient/qtwrapper/configurationmanager_wrap.h b/src/libclient/qtwrapper/configurationmanager_wrap.h
index 45c6fe9c8..8692f0e39 100644
--- a/src/libclient/qtwrapper/configurationmanager_wrap.h
+++ b/src/libclient/qtwrapper/configurationmanager_wrap.h
@@ -271,7 +271,8 @@ public:
msg.linearizedParent.c_str(),
convertMap(msg.body),
convertVecMap(msg.reactions),
- convertVecMap(msg.editions)});
+ convertVecMap(msg.editions),
+ convertMap(msg.status)});
}
Q_EMIT swarmLoaded(id,
@@ -298,7 +299,8 @@ public:
message.linearizedParent.c_str(),
convertMap(message.body),
convertVecMap(message.reactions),
- convertVecMap(message.editions)};
+ convertVecMap(message.editions),
+ convertMap(message.status)};
Q_EMIT swarmMessageReceived(QString(accountId.c_str()),
QString(conversationId.c_str()),
msg);
@@ -312,7 +314,8 @@ public:
message.linearizedParent.c_str(),
convertMap(message.body),
convertVecMap(message.reactions),
- convertVecMap(message.editions)};
+ convertVecMap(message.editions),
+ convertMap(message.status)};
Q_EMIT swarmMessageUpdated(QString(accountId.c_str()),
QString(conversationId.c_str()),
msg);
diff --git a/src/libclient/qtwrapper/conversions_wrap.hpp b/src/libclient/qtwrapper/conversions_wrap.hpp
index dc125c4a8..dfa00b851 100644
--- a/src/libclient/qtwrapper/conversions_wrap.hpp
+++ b/src/libclient/qtwrapper/conversions_wrap.hpp
@@ -83,6 +83,26 @@ convertMap(const MapStringString& m)
return temp;
}
+inline MapStringInt
+convertMap(const std::map<std::string, int32_t>& m)
+{
+ MapStringInt temp;
+ for (const auto& [key, value] : m) {
+ temp[QString(key.c_str())] = value;
+ }
+ return temp;
+}
+
+inline std::map<std::string, int32_t>
+convertMap(const MapStringInt& m)
+{
+ std::map<std::string, int32_t> temp;
+ for (const auto& [key, value] : m.toStdMap()) {
+ temp[key.toStdString()] = value;
+ }
+ return temp;
+}
+
inline VectorMapStringString
convertVecMap(const std::vector<std::map<std::string, std::string>>& m)
{
diff --git a/src/libclient/typedefs.h b/src/libclient/typedefs.h
index a3e9ff88d..7c79f8930 100644
--- a/src/libclient/typedefs.h
+++ b/src/libclient/typedefs.h
@@ -89,6 +89,7 @@ struct SwarmMessage
MapStringString body;
VectorMapStringString reactions;
VectorMapStringString editions;
+ MapStringInt status;
};
typedef QVector<SwarmMessage> VectorSwarmMessage;
--
GitLab