From 773c181d57085905d8a989f2f1cb644c747e63ab Mon Sep 17 00:00:00 2001
From: Joanna Hulboj <joanna.hulboj@isode.com>
Date: Thu, 9 Feb 2017 11:55:31 +0000
Subject: Fix double entries in MUC participant lists after merging nicks

Test-Information:
Tested using Psi and Swift.
Log in to Psi and Swift as UserOne. Enter Room (e.g. testRoom) using Swift and join the same room from Psi using drop down menu Join Groupchat.
Splitting: change nick from UserOne to UserTwo. Swift correctly
displays: UserOne, UserTwo.
Merging: change nick back from UserTwo to UsetOne. Swift correctly
displays: UserOne.

Change-Id: I291eddd5aed154fb0babe1b0ada0a15a317eacdb

diff --git a/Swift/Controllers/Chat/MUCController.cpp b/Swift/Controllers/Chat/MUCController.cpp
index 8f43b08..9d1459d 100644
--- a/Swift/Controllers/Chat/MUCController.cpp
+++ b/Swift/Controllers/Chat/MUCController.cpp
@@ -87,7 +87,7 @@ MUCController::MUCController (
         TimerFactory* timerFactory,
         EventController* eventController,
         EntityCapsProvider* entityCapsProvider,
-        XMPPRoster* roster,
+        XMPPRoster* xmppRoster,
         HistoryController* historyController,
         MUCRegistry* mucRegistry,
         HighlightManager* highlightManager,
@@ -103,12 +103,12 @@ MUCController::MUCController (
     lastWasPresence_ = false;
     shouldJoinOnReconnect_ = true;
     doneGettingHistory_ = false;
-    xmppRoster_ = roster;
+    xmppRoster_ = xmppRoster;
 
-    roster_ = new Roster(false, true);
-    rosterVCardProvider_ = new RosterVCardProvider(roster_, vcardManager, JID::WithResource);
+    roster_ = std::unique_ptr<Roster>(new Roster(false, true));
+    rosterVCardProvider_ = new RosterVCardProvider(roster_.get(), vcardManager, JID::WithResource);
     completer_ = new TabComplete();
-    chatWindow_->setRosterModel(roster_);
+    chatWindow_->setRosterModel(roster_.get());
     chatWindow_->setTabComplete(completer_);
     chatWindow_->onClosed.connect(boost::bind(&MUCController::handleWindowClosed, this));
     chatWindow_->onOccupantSelectionChanged.connect(boost::bind(&MUCController::handleWindowOccupantSelectionChanged, this, _1));
@@ -179,7 +179,6 @@ MUCController::~MUCController() {
     eventStream_->onUIEvent.disconnect(boost::bind(&MUCController::handleUIEvent, this, _1));
     chatWindow_->setRosterModel(nullptr);
     delete rosterVCardProvider_;
-    delete roster_;
     if (loginCheckTimer_) {
         loginCheckTimer_->stop();
     }
@@ -764,6 +763,11 @@ void MUCController::handleOccupantNicknameChanged(const std::string& oldNickname
 
     // update contact
     roster_->removeContact(oldJID);
+    auto it = currentOccupants_.find(newNickname);
+    if (it != currentOccupants_.end()) {
+        roster_->removeContact(newJID);
+    }
+
     MUCOccupant occupant = muc_->getOccupant(newNickname);
 
     JID realJID;
diff --git a/Swift/Controllers/Chat/MUCController.h b/Swift/Controllers/Chat/MUCController.h
index 467f2e7..7aff627 100644
--- a/Swift/Controllers/Chat/MUCController.h
+++ b/Swift/Controllers/Chat/MUCController.h
@@ -54,7 +54,7 @@ namespace Swift {
 
     class MUCController : public ChatControllerBase {
         public:
-            MUCController(const JID& self, MUC::ref muc, const boost::optional<std::string>& password, const std::string &nick, StanzaChannel* stanzaChannel, IQRouter* iqRouter, ChatWindowFactory* chatWindowFactory, PresenceOracle* presenceOracle, AvatarManager* avatarManager, UIEventStream* events, bool useDelayForLatency, TimerFactory* timerFactory, EventController* eventController, EntityCapsProvider* entityCapsProvider, XMPPRoster* roster, HistoryController* historyController, MUCRegistry* mucRegistry, HighlightManager* highlightManager, ClientBlockListManager* clientBlockListManager, std::shared_ptr<ChatMessageParser> chatMessageParser, bool isImpromptu, AutoAcceptMUCInviteDecider* autoAcceptMUCInviteDecider, VCardManager* vcardManager, MUCBookmarkManager* mucBookmarkManager);
+            MUCController(const JID& self, MUC::ref muc, const boost::optional<std::string>& password, const std::string &nick, StanzaChannel* stanzaChannel, IQRouter* iqRouter, ChatWindowFactory* chatWindowFactory, PresenceOracle* presenceOracle, AvatarManager* avatarManager, UIEventStream* events, bool useDelayForLatency, TimerFactory* timerFactory, EventController* eventController, EntityCapsProvider* entityCapsProvider, XMPPRoster* xmppRoster, HistoryController* historyController, MUCRegistry* mucRegistry, HighlightManager* highlightManager, ClientBlockListManager* clientBlockListManager, std::shared_ptr<ChatMessageParser> chatMessageParser, bool isImpromptu, AutoAcceptMUCInviteDecider* autoAcceptMUCInviteDecider, VCardManager* vcardManager, MUCBookmarkManager* mucBookmarkManager);
             virtual ~MUCController();
             boost::signals2::signal<void ()> onUserLeft;
             boost::signals2::signal<void ()> onUserJoined;
@@ -148,7 +148,6 @@ namespace Swift {
             MUC::ref muc_;
             std::string nick_;
             std::string desiredNick_;
-            Roster* roster_;
             TabComplete* completer_;
             bool parting_;
             bool joined_;
@@ -161,6 +160,7 @@ namespace Swift {
             boost::posix_time::ptime lastActivity_;
             boost::optional<std::string> password_;
             XMPPRoster* xmppRoster_;
+            std::unique_ptr<Roster> roster_;
             std::vector<HistoryMessage> joinContext_;
             size_t renameCounter_;
             bool isImpromptu_;
diff --git a/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp b/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp
index 32639f6..59dcd77 100644
--- a/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp
+++ b/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp
@@ -62,6 +62,8 @@ class MUCControllerTest : public CppUnit::TestFixture {
     CPPUNIT_TEST(testSubjectChangeIncorrectA);
     CPPUNIT_TEST(testSubjectChangeIncorrectB);
     CPPUNIT_TEST(testSubjectChangeIncorrectC);
+    CPPUNIT_TEST(testHandleOccupantNicknameChanged);
+    CPPUNIT_TEST(testHandleOccupantNicknameChangedRoster);
     CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -501,6 +503,61 @@ public:
         }
     }
 
+    void testHandleOccupantNicknameChanged() {
+        const auto occupantCount = [&](const std::string & nick) {
+            auto roster = window_->getRosterModel();
+            CPPUNIT_ASSERT(roster != nullptr);
+            const auto currentOccupantsJIDs = roster->getJIDs();
+            int count = 0;
+            for (auto & p : currentOccupantsJIDs) {
+                if (p.getResource() == nick) {
+                    ++count;
+                }
+            }
+            return count;
+        };
+
+        muc_->insertOccupant(MUCOccupant("TestUserOne", MUCOccupant::Participant, MUCOccupant::Owner));
+        muc_->insertOccupant(MUCOccupant("TestUserTwo", MUCOccupant::Participant, MUCOccupant::Owner));
+        muc_->insertOccupant(MUCOccupant("TestUserThree", MUCOccupant::Participant, MUCOccupant::Owner));
+
+        muc_->onOccupantNicknameChanged("TestUserOne", "TestUserTwo");
+
+        CPPUNIT_ASSERT_EQUAL(0, occupantCount("TestUserOne"));
+        CPPUNIT_ASSERT_EQUAL(1, occupantCount("TestUserTwo"));
+        CPPUNIT_ASSERT_EQUAL(1, occupantCount("TestUserThree"));
+    }
+
+    void testHandleOccupantNicknameChangedRoster() {
+        const auto occupantCount = [&](const std::string & nick) {
+            auto roster = window_->getRosterModel();
+            CPPUNIT_ASSERT(roster != nullptr);
+            const auto participants = roster->getGroup("Participants");
+            CPPUNIT_ASSERT(participants != nullptr);
+            const auto displayedParticipants = participants->getDisplayedChildren();
+            int count = 0;
+            for (auto & p : displayedParticipants) {
+                if (p->getDisplayName() == nick) {
+                    ++count;
+                }
+            }
+            return count;
+        };
+
+        muc_->insertOccupant(MUCOccupant("TestUserOne", MUCOccupant::Participant, MUCOccupant::Owner));
+        muc_->insertOccupant(MUCOccupant("TestUserTwo", MUCOccupant::Participant, MUCOccupant::Owner));
+        muc_->insertOccupant(MUCOccupant("TestUserThree", MUCOccupant::Participant, MUCOccupant::Owner));
+        CPPUNIT_ASSERT_EQUAL(1, occupantCount("TestUserOne"));
+        CPPUNIT_ASSERT_EQUAL(1, occupantCount("TestUserTwo"));
+        CPPUNIT_ASSERT_EQUAL(1, occupantCount("TestUserThree"));
+
+        muc_->onOccupantNicknameChanged("TestUserOne", "TestUserTwo");
+
+        CPPUNIT_ASSERT_EQUAL(0, occupantCount("TestUserOne"));
+        CPPUNIT_ASSERT_EQUAL(1, occupantCount("TestUserTwo"));
+        CPPUNIT_ASSERT_EQUAL(1, occupantCount("TestUserThree"));
+    }
+
     void testRoleAffiliationStatesVerify(const std::map<std::string, MUCOccupant> &occupants) {
         /* verify that the roster is in sync */
         GroupRosterItem* group = window_->getRosterModel()->getRoot();
-- 
cgit v0.10.2-6-g49f6