3 files changed, 43 insertions, 15 deletions
diff --git a/Swiften/MUC/MUCBookmarkManager.cpp b/Swiften/MUC/MUCBookmarkManager.cpp
index 511c88a..e0922ae 100644
--- a/Swiften/MUC/MUCBookmarkManager.cpp
+++ b/Swiften/MUC/MUCBookmarkManager.cpp
@@ -1,11 +1,12 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <Swiften/MUC/MUCBookmarkManager.h>
 
+#include <algorithm>
 #include <memory>
 
 #include <boost/bind.hpp>
@@ -30,9 +31,10 @@ void MUCBookmarkManager::handleBookmarksReceived(std::shared_ptr<Storage> payloa
     }
 
     ready_ = true;
+    handlingReceivedBookmarks_ = true;
     onBookmarksReady();
 
-    storage = payload;
+    storage_ = payload;
 
     std::vector<MUCBookmark> receivedBookmarks;
     for (const auto& room : payload->getRooms()) {
@@ -47,18 +49,25 @@ void MUCBookmarkManager::handleBookmarksReceived(std::shared_ptr<Storage> payloa
             onBookmarkRemoved(oldBookmark);
         }
     }
-
+    std::vector<MUCBookmark> newAddedBookmarksToBeSignaled;
     for (const auto& newBookmark : receivedBookmarks) {
         if (!containsEquivalent(bookmarks_, newBookmark)) {
             newBookmarks.push_back(newBookmark);
-            onBookmarkAdded(newBookmark);
+            //If the bookmark does not exist in bookmark manager, after emmiting the signal, chatsmanager will try to join the room, if the bookmark has autojoin to true.
+            //The bookmark is not yet available in bookmark manager, therefore a new bookmark will be created which will be lost when newBookmarks replace bookmarks.
+            newAddedBookmarksToBeSignaled.push_back(newBookmark);
         }
     }
     bookmarks_ = newBookmarks;
+    for (auto bookmark : newAddedBookmarksToBeSignaled) {
+        onBookmarkAdded(bookmark);
+   }
+
+    handlingReceivedBookmarks_ = false;
 }
 
 bool MUCBookmarkManager::containsEquivalent(const std::vector<MUCBookmark>& list, const MUCBookmark& bookmark) {
-    return std::find(list.begin(), list.end(), bookmark) != list.end();
+    return std::find_if(list.begin(), list.end(), [&](const MUCBookmark& val) { return bookmark.getRoom() == val.getRoom(); }) != list.end();
 }
 
 void MUCBookmarkManager::replaceBookmark(const MUCBookmark& oldBookmark, const MUCBookmark& newBookmark) {
@@ -76,8 +85,15 @@ void MUCBookmarkManager::replaceBookmark(const MUCBookmark& oldBookmark, const M
 
 void MUCBookmarkManager::addBookmark(const MUCBookmark& bookmark) {
     if (!ready_) return;
-    bookmarks_.push_back(bookmark);
-    onBookmarkAdded(bookmark);
+    if (auto found = lookupBookmark(bookmark.getRoom())) {
+        if (found != bookmark) {
+            replaceBookmark(found.get(), bookmark);
+        }
+    }
+    else {
+        bookmarks_.push_back(bookmark);
+        onBookmarkAdded(bookmark);
+    }
     flush();
 }
 
@@ -96,17 +112,20 @@ void MUCBookmarkManager::removeBookmark(const MUCBookmark& bookmark) {
 }
 
 void MUCBookmarkManager::flush() {
-    if (!storage) {
-        storage = std::make_shared<Storage>();
+    if (handlingReceivedBookmarks_) {
+        return;
+    }
+    if (!storage_) {
+        storage_ = std::make_shared<Storage>();
     }
     // Update the storage element
-    storage->clearRooms();
+    storage_->clearRooms();
     for (const auto& bookmark : bookmarks_) {
-        storage->addRoom(bookmark.toStorage());
+        storage_->addRoom(bookmark.toStorage());
     }
 
     // Send an iq to save the storage element
-    SetPrivateStorageRequest<Storage>::ref request = SetPrivateStorageRequest<Storage>::create(storage, iqRouter_);
+    SetPrivateStorageRequest<Storage>::ref request = SetPrivateStorageRequest<Storage>::create(storage_, iqRouter_);
     // FIXME: We should care about the result
     //request->onResponse.connect(boost::bind(&MUCBookmarkManager::handleBookmarksSet, this, _1, _2));
     request->send();
@@ -116,4 +135,12 @@ const std::vector<MUCBookmark>& MUCBookmarkManager::getBookmarks() const {
     return bookmarks_;
 }
 
+boost::optional<MUCBookmark> MUCBookmarkManager::lookupBookmark(const JID& bookmarkJID) const {
+    auto bookmarkIterator = std::find_if(bookmarks_.begin(), bookmarks_.end(), [&](const MUCBookmark& val) { return bookmarkJID == val.getRoom(); });
+    if (bookmarkIterator != bookmarks_.end()) {
+        return *bookmarkIterator;
+    }
+    return boost::none;
+}
+
 }
diff --git a/Swiften/MUC/MUCBookmarkManager.h b/Swiften/MUC/MUCBookmarkManager.h
index 78fbbb0..1ef227d 100644
--- a/Swiften/MUC/MUCBookmarkManager.h
+++ b/Swiften/MUC/MUCBookmarkManager.h
@@ -27,8 +27,8 @@ namespace Swift {
             void addBookmark(const MUCBookmark& bookmark);
             void removeBookmark(const MUCBookmark& bookmark);
             void replaceBookmark(const MUCBookmark& oldBookmark, const MUCBookmark& newBookmark);
-
             const std::vector<MUCBookmark>& getBookmarks() const;
+            boost::optional<MUCBookmark> lookupBookmark(const JID& bookmarkJID) const;
 
         public:
             boost::signals2::signal<void (const MUCBookmark&)> onBookmarkAdded;
@@ -45,8 +45,9 @@ namespace Swift {
 
         private:
             bool ready_;
+            bool handlingReceivedBookmarks_;
             std::vector<MUCBookmark> bookmarks_;
             IQRouter* iqRouter_;
-            std::shared_ptr<Storage> storage;
+            std::shared_ptr<Storage> storage_;
     };
 }
diff --git a/Swiften/MUC/MUCImpl.cpp b/Swiften/MUC/MUCImpl.cpp
index 029bb4b..d2b33f3 100644
--- a/Swiften/MUC/MUCImpl.cpp
+++ b/Swiften/MUC/MUCImpl.cpp
@@ -371,7 +371,7 @@ void MUCImpl::changeAffiliation(const JID& jid, MUCOccupant::Affiliation affilia
     MUCAdminPayload::ref mucPayload = std::make_shared<MUCAdminPayload>();
     MUCItem item;
     item.affiliation = affiliation;
-    item.realJID = jid.toBare();
+    item.realJID = jid;
     mucPayload->addItem(item);
     std::shared_ptr<GenericRequest<MUCAdminPayload> > request = std::make_shared<GenericRequest<MUCAdminPayload> >(IQ::Set, getJID(), mucPayload, iqRouter_);
     request->onResponse.connect(boost::bind(&MUCImpl::handleAffiliationChangeResponse, this, _1, _2, jid, affiliation));