From 7edd420ffab2415921289af47977a68215630a0f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois-Simon=20Fauteux-Chapleau?=
 <francois-simon.fauteux-chapleau@savoirfairelinux.com>
Date: Fri, 4 Apr 2025 11:58:34 -0400
Subject: [PATCH] conversation: refactor addToHistory

The addToHistory function currently iterates over the list of commits to
add only once. This patch refactors the function so that the message
status computations are done in a separate loop from the rest of the
message handling logic. This will make it easier to fix a bug in the
message status computation in a subsequent patch.

GitLab: #1042
Change-Id: I3fceb10057292f84bc8c5e9d668fed7290ace8f2
---
 src/jamidht/conversation.cpp | 40 +++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/src/jamidht/conversation.cpp b/src/jamidht/conversation.cpp
index e5040e751f..7d21f18277 100644
--- a/src/jamidht/conversation.cpp
+++ b/src/jamidht/conversation.cpp
@@ -1146,7 +1146,6 @@ void Conversation::Impl::rectifyStatus(const std::shared_ptr<libjami::SwarmMessa
     }
 }
 
-
 std::vector<std::shared_ptr<libjami::SwarmMessage>>
 Conversation::Impl::addToHistory(History& history,
                                  const std::vector<std::map<std::string, std::string>>& commits,
@@ -1161,22 +1160,25 @@ Conversation::Impl::addToHistory(History& history,
         std::unique_lock lk(history.mutex);
         history.cv.wait(lk, [&] { return !history.loading; });
     }
-    std::vector<std::shared_ptr<libjami::SwarmMessage>> messages;
-    auto addCommit = [&](const auto& commit) {
+
+    std::vector<std::shared_ptr<libjami::SwarmMessage>> sharedCommits;
+    for (const auto& commit : commits) {
         auto commitId = commit.at("id");
         if (history.quickAccess.find(commitId) != history.quickAccess.end())
-            return; // Already present
+            continue; // Already present
         auto typeIt = commit.find("type");
-        auto reactToIt = commit.find("react-to");
-        auto editIt = commit.find("edit");
         // Nothing to show for the client, skip
         if (typeIt != commit.end() && typeIt->second == "merge")
-            return;
+            continue;
 
         auto sharedCommit = std::make_shared<libjami::SwarmMessage>();
         sharedCommit->fromMapStringString(commit);
-        // Set message status based on cache (only on history for client)
-        if (!commitFromSelf && &history == &loadedHistory_) {
+        sharedCommits.emplace_back(sharedCommit);
+    }
+
+    // Set message status based on cache (only on history for client)
+    if (!commitFromSelf && &history == &loadedHistory_) {
+        for (const auto& sharedCommit : sharedCommits) {
             std::lock_guard lk(messageStatusMtx_);
             for (const auto& member: repository_->members()) {
                 // If we have a status cached, use it
@@ -1213,11 +1215,12 @@ Conversation::Impl::addToHistory(History& history,
                     }
                 } else {
                     // If member is author of the message received, they already saw it
-                    if (member.uri == commit.at("author")) {
+                    auto author = sharedCommit->body.at("author");
+                    if (member.uri == author) {
                         // If member is the author of the commit, they are considered as displayed (same for all previous commits)
                         messagesStatus_[member.uri]["read"] = sharedCommit->id;
                         messagesStatus_[member.uri]["fetched"] = sharedCommit->id;
-                        sharedCommit->status[commit.at("author")] = static_cast<int32_t>(libjami::Account::MessageStates::DISPLAYED);
+                        sharedCommit->status[author] = static_cast<int32_t>(libjami::Account::MessageStates::DISPLAYED);
                         cache = static_cast<int32_t>(libjami::Account::MessageStates::DISPLAYED);
                         continue;
                     }
@@ -1234,18 +1237,23 @@ Conversation::Impl::addToHistory(History& history,
                 }
             }
         }
-        history.quickAccess[commitId] = sharedCommit;
+    }
+
+    std::vector<std::shared_ptr<libjami::SwarmMessage>> messages;
+    for (const auto& sharedCommit : sharedCommits) {
+        history.quickAccess[sharedCommit->id] = sharedCommit;
 
-        if (reactToIt != commit.end() && !reactToIt->second.empty()) {
+        auto reactToIt = sharedCommit->body.find("react-to");
+        auto editIt = sharedCommit->body.find("edit");
+        if (reactToIt != sharedCommit->body.end() && !reactToIt->second.empty()) {
             handleReaction(history, sharedCommit);
-        } else if (editIt != commit.end() && !editIt->second.empty()) {
+        } else if (editIt != sharedCommit->body.end() && !editIt->second.empty()) {
             handleEdition(history, sharedCommit, messageReceived);
         } else if (handleMessage(history, sharedCommit, messageReceived)) {
             messages.emplace_back(sharedCommit);
         }
         rectifyStatus(sharedCommit, history);
-    };
-    std::for_each(commits.begin(), commits.end(), addCommit);
+    }
 
     return messages;
 }
-- 
GitLab