Fix recent chat entries being incorrectly displayed

Recent chat entries were displayed as a randomly generated numbers instead of impromptus (if invitees were offline). Title displayed in the Recent Chat List for MUC is now based on both the occupants and invitees. To do that a collection with all the invitees is being stored (new) along the occupants (existing). Test-Information: From Swift menu choose Actions, Start Chat... Add some offline contacts to the List of Participants and press Finish. Recent chat entry will have correct name (combined of contact names). Change-Id: Ie076165e8dbb493aa261cc49ca3ab1e0c1c542a8
author: Joanna Hulboj <joanna.hulboj@isode.com> 2017-03-13 21:51:04 (GMT)
committer: Kevin Smith <kevin.smith@isode.com> 2017-05-04 10:34:00 (GMT)
commit: cfea60eda7f3ce5fa10ed92c50c19fc1ee264eb1 (patch)
tree: fb2564927189eb1cc8e28ac82fd96f6161b54d31
parent: dc70dbb6af039fba8f7a1cece8db4bb119deaabd (diff)
download: swift-cfea60eda7f3ce5fa10ed92c50c19fc1ee264eb1.zip
swift-cfea60eda7f3ce5fa10ed92c50c19fc1ee264eb1.tar.bz2
7 files changed, 140 insertions, 17 deletions
diff --git a/Swift/Controllers/Chat/ChatsManager.cpp b/Swift/Controllers/Chat/ChatsManager.cpp
index db3b3b7..9db343f 100644
--- a/Swift/Controllers/Chat/ChatsManager.cpp
+++ b/Swift/Controllers/Chat/ChatsManager.cpp
@@ -67,7 +67,7 @@
 #include <Swift/Controllers/WhiteboardManager.h>
 #include <Swift/Controllers/XMPPEvents/EventController.h>
 
-BOOST_CLASS_VERSION(Swift::ChatListWindow::Chat, 1)
+BOOST_CLASS_VERSION(Swift::ChatListWindow::Chat, 2)
 
 namespace boost {
 namespace serialization {
@@ -96,6 +96,9 @@ namespace serialization {
         if (version > 0) {
             ar & chat.password;
         }
+        if (version > 1) {
+            ar & chat.inviteesNames;
+        }
     }
 }
 }
@@ -402,6 +405,12 @@ ChatListWindow::Chat ChatsManager::createChatListChatItem(const JID& jid, const
                 ChatListWindow::Chat chat = ChatListWindow::Chat(jid, jid.toString(), activity, unreadCount, type, boost::filesystem::path(), true, privateMessage, nick, password);
                 std::map<std::string, JID> participants = controller->getParticipantJIDs();
                 chat.impromptuJIDs = participants;
+
+                std::map<JID, std::string> participantsNames;
+                for (auto& i : invitees_[jid]) {
+                    participantsNames.emplace(i, roster_->getNameForJID(i));
+                }
+                chat.inviteesNames = participantsNames;
                 return chat;
             }
         }
@@ -488,7 +497,8 @@ void ChatsManager::cleanupPrivateMessageRecents() {
 void ChatsManager::appendRecent(const ChatListWindow::Chat& chat) {
     boost::optional<ChatListWindow::Chat> oldChat = removeExistingChat(chat);
     ChatListWindow::Chat mergedChat = chat;
-    if (oldChat && !oldChat->impromptuJIDs.empty()) {
+    if (oldChat) {
+        mergedChat.inviteesNames.insert(oldChat->inviteesNames.begin(), oldChat->inviteesNames.end());
         mergedChat.impromptuJIDs.insert(oldChat->impromptuJIDs.begin(), oldChat->impromptuJIDs.end());
     }
     recentChats_.push_front(mergedChat);
@@ -497,7 +507,8 @@ void ChatsManager::appendRecent(const ChatListWindow::Chat& chat) {
 void ChatsManager::prependRecent(const ChatListWindow::Chat& chat) {
     boost::optional<ChatListWindow::Chat> oldChat = removeExistingChat(chat);
     ChatListWindow::Chat mergedChat = chat;
-    if (oldChat && !oldChat->impromptuJIDs.empty()) {
+    if (oldChat) {
+        mergedChat.inviteesNames.insert(oldChat->inviteesNames.begin(), oldChat->inviteesNames.end());
         mergedChat.impromptuJIDs.insert(oldChat->impromptuJIDs.begin(), oldChat->impromptuJIDs.end());
     }
     recentChats_.push_back(mergedChat);
@@ -590,6 +601,10 @@ void ChatsManager::handleUIEvent(std::shared_ptr<UIEvent> event) {
         // The room JID is random for new impromptu rooms, or a predefined JID for impromptu rooms resumed from the 'Recent chats' list.
         JID roomJID = createImpromptuMUCEvent->getRoomJID().toString().empty() ? JID(idGenerator_.generateID(), localMUCServiceJID_) : createImpromptuMUCEvent->getRoomJID();
 
+        std::vector<JID> missingJIDsToInvite = createImpromptuMUCEvent->getJIDs();
+        for (const JID& jid : missingJIDsToInvite) {
+            invitees_[roomJID].insert(jid);
+        }
         // join muc
         MUC::ref muc = handleJoinMUCRequest(roomJID, boost::optional<std::string>(), nickResolver_->jidToNick(jid_), false, true, true);
         mucControllers_[roomJID]->onImpromptuConfigCompleted.connect(boost::bind(&ChatsManager::finalizeImpromptuJoin, this, muc, createImpromptuMUCEvent->getJIDs(), createImpromptuMUCEvent->getReason(), boost::optional<JID>()));
diff --git a/Swift/Controllers/Chat/ChatsManager.h b/Swift/Controllers/Chat/ChatsManager.h
index 593624d..6004347 100644
--- a/Swift/Controllers/Chat/ChatsManager.h
+++ b/Swift/Controllers/Chat/ChatsManager.h
@@ -9,6 +9,7 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <set>
 
 #include <Swiften/Base/IDGenerator.h>
 #include <Swiften/Elements/DiscoInfo.h>
@@ -181,5 +182,7 @@ namespace Swift {
             AutoAcceptMUCInviteDecider* autoAcceptMUCInviteDecider_;
             IDGenerator idGenerator_;
             VCardManager* vcardManager_;
+
+            std::map<JID, std::set<JID>> invitees_;
     };
 }
diff --git a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp
index bf645d0..52537a9 100644
--- a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp
+++ b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp
@@ -16,6 +16,8 @@
 #include <Swiften/Avatars/AvatarMemoryStorage.h>
 #include <Swiften/Avatars/NullAvatarManager.h>
 #include <Swiften/Base/Algorithm.h>
+#include <Swiften/Base/Log.h>
+#include <Swiften/Base/LogSerializers.h>
 #include <Swiften/Client/Client.h>
 #include <Swiften/Client/ClientBlockListManager.h>
 #include <Swiften/Client/DummyStanzaChannel.h>
@@ -52,6 +54,7 @@
 #include <Swift/Controllers/ProfileSettingsProvider.h>
 #include <Swift/Controllers/SettingConstants.h>
 #include <Swift/Controllers/Settings/DummySettingsProvider.h>
+#include <Swift/Controllers/UIEvents/CreateImpromptuMUCUIEvent.h>
 #include <Swift/Controllers/UIEvents/JoinMUCUIEvent.h>
 #include <Swift/Controllers/UIEvents/RequestChatUIEvent.h>
 #include <Swift/Controllers/UIEvents/UIEventStream.h>
@@ -67,6 +70,9 @@
 
 #include <SwifTools/Notifier/Notifier.h>
 
+#include <Swift/QtUI/QtSwiftUtil.h>
+#include <Swiften/MUC/UnitTest/MockMUC.h>
+
 using namespace Swift;
 
 class DummyNotifier : public Notifier {
@@ -137,7 +143,6 @@ class ChatsManagerTest : public CppUnit::TestFixture {
     CPPUNIT_TEST(testCarbonsForwardedOutgoingMessageFromSecondResource);
     CPPUNIT_TEST(testCarbonsForwardedIncomingDuplicates);
 
-
     // Message correction tests
     CPPUNIT_TEST(testChatControllerMessageCorrectionCorrectReplaceID);
     CPPUNIT_TEST(testChatControllerMessageCorrectionIncorrectReplaceID);
@@ -145,6 +150,9 @@ class ChatsManagerTest : public CppUnit::TestFixture {
     CPPUNIT_TEST(testChatControllerMessageCorrectionReplaceByOtherResource);
     CPPUNIT_TEST(testMUCControllerMessageCorrectionNoIDMatchRequired);
 
+    //Imptomptu test
+    CPPUNIT_TEST(testImpromptuChatTitle);
+
     CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -1503,6 +1511,69 @@ public:
         CPPUNIT_ASSERT_EQUAL(std::string("Some correctly spelled message."), window->bodyFromMessage(window->lastReplacedMessage_));
     }
 
+    void testImpromptuChatTitle() {
+        stanzaChannel_->uniqueIDs_ = true;
+        JID mucJID("795B7BBE-9099-4A0D-81BA-C816F78E275C@test.com");
+        manager_->setOnline(true);
+
+        // Open chat window to a sender.
+        MockChatWindow* window = new MockChatWindow();
+        std::shared_ptr<IQ> infoRequest = std::dynamic_pointer_cast<IQ>(stanzaChannel_->sentStanzas[1]);
+        CPPUNIT_ASSERT(infoRequest);
+
+        std::shared_ptr<IQ> infoResponse = IQ::createResult(infoRequest->getFrom(), infoRequest->getTo(), infoRequest->getID());
+
+        DiscoInfo info;
+        info.addIdentity(DiscoInfo::Identity("Shakespearean Chat Service", "conference", "text"));
+        info.addFeature("http://jabber.org/protocol/muc");
+        infoResponse->addPayload(std::make_shared<DiscoInfo>(info));
+        stanzaChannel_->onIQReceived(infoResponse);
+
+        std::vector<JID> jids;
+        jids.emplace_back("foo@test.com");
+        jids.emplace_back("bar@test.com");
+
+        mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(mucJID, uiEventStream_).Return(window);
+        uiEventStream_->send(std::make_shared<CreateImpromptuMUCUIEvent>(jids, mucJID, ""));
+        CPPUNIT_ASSERT_EQUAL(std::string("bar@test.com, foo@test.com"), manager_->getRecentChats()[0].getTitle());
+
+        auto mucJoinPresence = std::dynamic_pointer_cast<Presence>(stanzaChannel_->sentStanzas[2]);
+        CPPUNIT_ASSERT(mucJoinPresence);
+
+        // MUC presence reply
+        auto mucResponse = Presence::create();
+        mucResponse->setTo(jid_);
+        mucResponse->setFrom(mucJoinPresence->getTo());
+        mucResponse->addPayload([]() {
+            auto mucUser = std::make_shared<MUCUserPayload>();
+            mucUser->addItem(MUCItem(MUCOccupant::Member, MUCOccupant::Participant));
+            mucUser->addStatusCode(MUCUserPayload::StatusCode(110));
+            mucUser->addStatusCode(MUCUserPayload::StatusCode(210));
+            return mucUser;
+        }());
+        stanzaChannel_->onPresenceReceived(mucResponse);
+
+        // Before people join the impromptu room, the title is based on names coming from Roster
+        CPPUNIT_ASSERT_EQUAL(std::string("bar@test.com, foo@test.com"), manager_->getRecentChats()[0].getTitle());
+
+        auto mucParticipantJoined = [&](const JID& jid) {
+            auto participantJoinedPresence = Presence::create();
+            participantJoinedPresence->setTo(jid_);
+            participantJoinedPresence->setFrom(mucJID.withResource(jid.toString()));
+            auto mucUser = std::make_shared<MUCUserPayload>();
+            mucUser->addItem(MUCItem(MUCOccupant::Member, MUCOccupant::Participant));
+            participantJoinedPresence->addPayload(mucUser);
+            return participantJoinedPresence;
+        };
+
+        for (const auto& participantJID : jids) {
+            stanzaChannel_->onPresenceReceived(mucParticipantJoined(participantJID));
+        }
+
+        // After people joined, the title is the list of participant nicknames or names coming from Roster (if nicknames are unavailable)
+        CPPUNIT_ASSERT_EQUAL(std::string("bar@test.com, foo@test.com"), manager_->getRecentChats()[0].getTitle());
+    }
+
 
 private:
     std::shared_ptr<Message> makeDeliveryReceiptTestMessage(const JID& from, const std::string& id) {
diff --git a/Swift/Controllers/UIInterfaces/ChatListWindow.h b/Swift/Controllers/UIInterfaces/ChatListWindow.h
index dde596e..29097e9 100644
--- a/Swift/Controllers/UIInterfaces/ChatListWindow.h
+++ b/Swift/Controllers/UIInterfaces/ChatListWindow.h
@@ -11,9 +11,12 @@
 #include <memory>
 #include <set>
 
+
+#include <boost/algorithm/string/join.hpp>
 #include <boost/filesystem/path.hpp>
 #include <boost/signals2.hpp>
 
+#include <Swiften/Base/Algorithm.h>
 #include <Swiften/Elements/StatusShow.h>
 #include <Swiften/MUC/MUCBookmark.h>
 
@@ -31,6 +34,9 @@ namespace Swift {
                             return jid.toBare() == other.jid.toBare()
                                     && isMUC == other.isMUC;
                         }
+                        if (chatName == other.chatName) {
+                            return true;
+                        }
                         else { /* compare the chat occupant lists */
                             for (const auto& jid : impromptuJIDs) {
                                 bool found = false;
@@ -44,7 +50,7 @@ namespace Swift {
                                     return false;
                                 }
                             }
-                            return true;
+                            return key_compare(inviteesNames, other.inviteesNames);
                         }
                     }
                     void setUnreadCount(int unread) {
@@ -57,15 +63,28 @@ namespace Swift {
                         avatarPath = path;
                     }
                     std::string getImpromptuTitle() const {
-                        std::string title;
-                        for (auto& pair : impromptuJIDs) {
-                            if (title.empty()) {
-                                title += pair.first;
-                            } else {
-                                title += ", " + pair.first;
+                        std::set<std::string> participants;
+                        std::map<JID, std::string> participantsMap;
+
+                        for (auto& pair : inviteesNames) {
+                            if (!pair.second.empty()) {
+                                participantsMap[pair.first] = pair.second;
                             }
+                            else {
+                                participantsMap[pair.first] = pair.first.toString();
+                            }
+                        }
+                        for (auto& pair : impromptuJIDs) {
+                            participantsMap[pair.second] = pair.first;
                         }
-                        return title;
+                        for (auto& participant : participantsMap) {
+                            participants.insert(participant.second);
+                        }
+                        return boost::algorithm::join(participants, ", ");
+                    }
+                    std::string getTitle() const {
+                        std::string title = getImpromptuTitle();
+                        return title.empty() ? chatName : title;
                     }
                     JID jid;
                     std::string chatName;
@@ -77,6 +96,7 @@ namespace Swift {
                     int unreadCount;
                     boost::filesystem::path avatarPath;
                     std::map<std::string, JID> impromptuJIDs;
+                    std::map<JID, std::string> inviteesNames;
                     bool isPrivateMessage;
             };
             virtual ~ChatListWindow();
diff --git a/Swift/QtUI/ChatList/ChatListRecentItem.cpp b/Swift/QtUI/ChatList/ChatListRecentItem.cpp
index faac937..9e55e1b 100644
--- a/Swift/QtUI/ChatList/ChatListRecentItem.cpp
+++ b/Swift/QtUI/ChatList/ChatListRecentItem.cpp
@@ -22,7 +22,7 @@ const ChatListWindow::Chat& ChatListRecentItem::getChat() const {
 
 QVariant ChatListRecentItem::data(int role) const {
     switch (role) {
-        case Qt::DisplayRole: return chat_.impromptuJIDs.empty() ? P2QSTRING(chat_.chatName) : P2QSTRING(chat_.getImpromptuTitle());
+        case Qt::DisplayRole: return P2QSTRING(chat_.getTitle());
         case DetailTextRole: return P2QSTRING(chat_.activity);
         case Qt::TextColorRole: return QColor(89,89,89);
         /*case Qt::BackgroundColorRole: return backgroundColor_;
diff --git a/Swiften/Base/Algorithm.h b/Swiften/Base/Algorithm.h
index c481aa8..108dbe3 100644
--- a/Swiften/Base/Algorithm.h
+++ b/Swiften/Base/Algorithm.h
@@ -152,4 +152,12 @@ namespace Swift {
         private:
             V value;
     };
+
+    template <typename Map>
+    bool key_compare(Map const& lhs, Map const& rhs) {
+
+        auto pred = [](decltype(*lhs.begin()) a, decltype(a) b) { return a.first == b.first; };
+
+        return lhs.size() == rhs.size() && std::equal(lhs.begin(), lhs.end(), rhs.begin(), pred);
+    }
 }
diff --git a/Swiften/Client/DummyStanzaChannel.h b/Swiften/Client/DummyStanzaChannel.h
index fc2f05b..4cc0f7e 100644
--- a/Swiften/Client/DummyStanzaChannel.h
+++ b/Swiften/Client/DummyStanzaChannel.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2017 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -13,7 +13,7 @@
 namespace Swift {
     class DummyStanzaChannel : public StanzaChannel {
         public:
-            DummyStanzaChannel() : available_(true) {}
+            DummyStanzaChannel() {}
 
             virtual void sendStanza(std::shared_ptr<Stanza> stanza) {
                 sentStanzas.push_back(stanza);
@@ -37,7 +37,11 @@ namespace Swift {
             }
 
             virtual std::string getNewIQID() {
-                return "test-id";
+                std::string id = "test-id";
+                if (uniqueIDs_) {
+                    id += "-" + std::to_string(idCounter_++);
+                }
+                return id;
             }
 
             virtual bool isAvailable() const {
@@ -94,6 +98,8 @@ namespace Swift {
             }
 
             std::vector<std::shared_ptr<Stanza> > sentStanzas;
-            bool available_;
+            bool available_ = true;
+            bool uniqueIDs_ = false;
+            unsigned int idCounter_ = 0;
     };
 }
author	Joanna Hulboj <joanna.hulboj@isode.com>	2017-03-13 21:51:04 (GMT)
committer	Kevin Smith <kevin.smith@isode.com>	2017-05-04 10:34:00 (GMT)
commit	cfea60eda7f3ce5fa10ed92c50c19fc1ee264eb1 (patch)
tree	fb2564927189eb1cc8e28ac82fd96f6161b54d31
parent	dc70dbb6af039fba8f7a1cece8db4bb119deaabd (diff)
download	swift-cfea60eda7f3ce5fa10ed92c50c19fc1ee264eb1.zip swift-cfea60eda7f3ce5fa10ed92c50c19fc1ee264eb1.tar.bz2