5 files changed, 85 insertions, 19 deletions
diff --git a/Swift/Controllers/Chat/ChatControllerBase.cpp b/Swift/Controllers/Chat/ChatControllerBase.cpp
index b339bd0..da9064e 100644
--- a/Swift/Controllers/Chat/ChatControllerBase.cpp
+++ b/Swift/Controllers/Chat/ChatControllerBase.cpp
@@ -270,7 +270,13 @@ void ChatControllerBase::handleIncomingMessage(std::shared_ptr<MessageEvent> mes
     std::string body = optionalBody.get_value_or("");
     if (message->isError()) {
         if (!message->getTo().getResource().empty()) {
-            std::string errorMessage = str(format(QT_TRANSLATE_NOOP("", "Couldn't send message: %1%")) % getErrorMessage(message->getPayload<ErrorPayload>()));
+            std::string errorMessage;
+            if (message->getPayload<Swift::ErrorPayload>()->getCondition() == ErrorPayload::ItemNotFound) {
+                errorMessage = QT_TRANSLATE_NOOP("", "This user could not be found in the room.");
+            }
+            else {
+                errorMessage = str(format(QT_TRANSLATE_NOOP("", "Couldn't send message: %1%")) % getErrorMessage(message->getPayload<ErrorPayload>()));
+            }
             chatWindow_->addErrorMessage(chatMessageParser_->parseMessageBody(errorMessage));
         }
     }
diff --git a/Swift/Controllers/Chat/ChatsManager.cpp b/Swift/Controllers/Chat/ChatsManager.cpp
index b9e2cf4..4f95d71 100644
--- a/Swift/Controllers/Chat/ChatsManager.cpp
+++ b/Swift/Controllers/Chat/ChatsManager.cpp
@@ -937,8 +937,18 @@ void ChatsManager::handleIncomingMessage(std::shared_ptr<Message> incomingMessag
         return;
     }
 
-    // Try to deliver it to a MUC
-    if (message->getType() == Message::Groupchat || message->getType() == Message::Error /*|| (isInvite && message->getType() == Message::Normal)*/) {
+    // Try to deliver MUC errors to a MUC PM window if a suitable window is open.
+    if (message->getType() == Message::Error) {
+        auto controller = getChatControllerIfExists(fromJID, messageCausesSessionBinding(message));
+        if (controller) {
+            controller->handleIncomingMessage(event);
+            return;
+        }
+    }
+
+    // Try to deliver it to a MUC.
+    if (message->getType() == Message::Groupchat || message->getType() == Message::Error) {
+        // Try to deliver it to a MUC room.
         std::map<JID, MUCController*>::iterator i = mucControllers_.find(fromJID.toBare());
         if (i != mucControllers_.end()) {
             i->second->handleIncomingMessage(event);
diff --git a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp
index a5e68cf..cff54f8 100644
--- a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp
+++ b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp
@@ -81,10 +81,13 @@ class ChatsManagerTest : public CppUnit::TestFixture {
     CPPUNIT_TEST(testChatControllerPresenceAccessUpdatedOnSubscriptionChangeToFrom);
     CPPUNIT_TEST(testChatControllerFullJIDBindingOnMessageAndNotReceipt);
     CPPUNIT_TEST(testChatControllerFullJIDBindingOnTypingAndNotActive);
-    CPPUNIT_TEST(testChatControllerPMPresenceHandling);
     CPPUNIT_TEST(testLocalMUCServiceDiscoveryResetOnDisconnect);
     CPPUNIT_TEST(testPresenceChangeDoesNotReplaceMUCInvite);
 
+    // MUC PM Tests
+    CPPUNIT_TEST(testChatControllerPMPresenceHandling);
+    CPPUNIT_TEST(testChatControllerMucPmUnavailableErrorHandling);
+
     // Highlighting tests
     CPPUNIT_TEST(testChatControllerHighlightingNotificationTesting);
     CPPUNIT_TEST(testChatControllerHighlightingNotificationDeduplicateSounds);
@@ -190,7 +193,7 @@ public:
     void testSecondOpenWindowIncoming() {
         JID messageJID1("testling@test.com/resource1");
 
-        MockChatWindow* window1 = new MockChatWindow();//mocks_->InterfaceMock<ChatWindow>();
+        MockChatWindow* window1 = new MockChatWindow();
         mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(messageJID1, uiEventStream_).Return(window1);
 
         std::shared_ptr<Message> message1(new Message());
@@ -213,7 +216,7 @@ public:
     void testFirstOpenWindowOutgoing() {
         std::string messageJIDString("testling@test.com");
 
-        ChatWindow* window = new MockChatWindow();//mocks_->InterfaceMock<ChatWindow>();
+        ChatWindow* window = new MockChatWindow();
         mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(JID(messageJIDString), uiEventStream_).Return(window);
 
         uiEventStream_->send(std::make_shared<RequestChatUIEvent>(JID(messageJIDString)));
@@ -224,7 +227,7 @@ public:
         std::string bareJIDString("testling@test.com");
         std::string fullJIDString("testling@test.com/resource1");
 
-        MockChatWindow* window = new MockChatWindow();//mocks_->InterfaceMock<ChatWindow>();
+        MockChatWindow* window = new MockChatWindow();
         mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(JID(bareJIDString), uiEventStream_).Return(window);
         uiEventStream_->send(std::make_shared<RequestChatUIEvent>(JID(bareJIDString)));
 
@@ -238,12 +241,12 @@ public:
 
     void testSecondWindow() {
         std::string messageJIDString1("testling1@test.com");
-        ChatWindow* window1 = new MockChatWindow();//mocks_->InterfaceMock<ChatWindow>();
+        ChatWindow* window1 = new MockChatWindow();
         mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(JID(messageJIDString1), uiEventStream_).Return(window1);
         uiEventStream_->send(std::make_shared<RequestChatUIEvent>(JID(messageJIDString1)));
 
         std::string messageJIDString2("testling2@test.com");
-        ChatWindow* window2 = new MockChatWindow();//mocks_->InterfaceMock<ChatWindow>();
+        ChatWindow* window2 = new MockChatWindow();
         mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(JID(messageJIDString2), uiEventStream_).Return(window2);
 
         uiEventStream_->send(std::make_shared<RequestChatUIEvent>(JID(messageJIDString2)));
@@ -260,7 +263,7 @@ public:
         std::string fullJIDString1("testling@test.com/resource1");
         std::string fullJIDString2("testling@test.com/resource2");
 
-        MockChatWindow* window = new MockChatWindow();//mocks_->InterfaceMock<ChatWindow>();
+        MockChatWindow* window = new MockChatWindow();
         mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(JID(bareJIDString), uiEventStream_).Return(window);
         uiEventStream_->send(std::make_shared<RequestChatUIEvent>(JID(bareJIDString)));
 
@@ -297,18 +300,18 @@ public:
 
 
         std::string messageJIDString1("testling@test.com/1");
-        ChatWindow* window1 = new MockChatWindow();//mocks_->InterfaceMock<ChatWindow>();
+        ChatWindow* window1 = new MockChatWindow();
         mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(JID(messageJIDString1), uiEventStream_).Return(window1);
         uiEventStream_->send(std::make_shared<RequestChatUIEvent>(JID(messageJIDString1)));
 
         std::string messageJIDString2("testling@test.com/2");
-        ChatWindow* window2 = new MockChatWindow();//mocks_->InterfaceMock<ChatWindow>();
+        ChatWindow* window2 = new MockChatWindow();
         mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(JID(messageJIDString2), uiEventStream_).Return(window2);
 
         uiEventStream_->send(std::make_shared<RequestChatUIEvent>(JID(messageJIDString2)));
 
         std::string messageJIDString3("testling@test.com/3");
-        ChatWindow* window3 = new MockChatWindow();//mocks_->InterfaceMock<ChatWindow>();
+        ChatWindow* window3 = new MockChatWindow();
         mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(JID(messageJIDString3), uiEventStream_).Return(window3);
 
         uiEventStream_->send(std::make_shared<RequestChatUIEvent>(JID(messageJIDString3)));
@@ -331,7 +334,7 @@ public:
     void testNoDuplicateUnbind() {
         JID messageJID1("testling@test.com/resource1");
 
-        MockChatWindow* window1 = new MockChatWindow();//mocks_->InterfaceMock<ChatWindow>();
+        MockChatWindow* window1 = new MockChatWindow();
         mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(messageJID1, uiEventStream_).Return(window1);
 
         std::shared_ptr<Message> message1(new Message());
@@ -407,7 +410,7 @@ public:
     void testChatControllerPresenceAccessUpdatedOnAddToRoster() {
         JID messageJID("testling@test.com/resource1");
 
-        MockChatWindow* window = new MockChatWindow();//mocks_->InterfaceMock<ChatWindow>();
+        MockChatWindow* window = new MockChatWindow();
         mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(messageJID, uiEventStream_).Return(window);
         settings_->storeSetting(SettingConstants::REQUEST_DELIVERYRECEIPTS, true);
 
@@ -688,6 +691,50 @@ public:
         CPPUNIT_ASSERT_EQUAL(std::string("participantA has gone offline."), MockChatWindow::bodyFromMessage(window->lastReplacedMessage_));
     }
 
+    void testChatControllerMucPmUnavailableErrorHandling() {
+        auto mucJID = JID("test@rooms.test.com");
+        auto participantA = mucJID.withResource("participantA");
+        auto participantB = mucJID.withResource("participantB");
+
+        auto mucWindow = new MockChatWindow();
+        mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(mucJID, uiEventStream_).Return(mucWindow);
+        uiEventStream_->send(std::make_shared<JoinMUCUIEvent>(mucJID, participantB.getResource()));
+        CPPUNIT_ASSERT_EQUAL(true, mucWindow->mucType_.is_initialized());
+
+        auto window = new MockChatWindow();
+        mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(participantA, uiEventStream_).Return(window);
+        uiEventStream_->send(std::make_shared<RequestChatUIEvent>(participantA));
+        CPPUNIT_ASSERT_EQUAL(false, window->mucType_.is_initialized());
+
+        Presence::ref presence = Presence::create();
+        presence->setFrom(participantA);
+        presence->setShow(StatusShow::Online);
+        stanzaChannel_->onPresenceReceived(presence);
+        CPPUNIT_ASSERT_EQUAL(std::string("participantA has become available."), MockChatWindow::bodyFromMessage(window->lastAddedPresence_));
+
+        // send message to participantA
+        auto messageBody = std::string("message body to send");
+        window->onSendMessageRequest(messageBody, false);
+        auto sendMessageStanza = stanzaChannel_->getStanzaAtIndex<Message>(2);
+        CPPUNIT_ASSERT_EQUAL(messageBody, *sendMessageStanza->getBody());
+
+        // receive reply with error
+        auto messageErrorReply = std::make_shared<Message>();
+        messageErrorReply->setID(stanzaChannel_->getNewIQID());
+        messageErrorReply->setType(Message::Error);
+        messageErrorReply->setFrom(participantA);
+        messageErrorReply->setTo(jid_);
+        messageErrorReply->addPayload(std::make_shared<ErrorPayload>(ErrorPayload::ItemNotFound, ErrorPayload::Cancel, "Recipient not in room"));
+
+        auto lastMUCWindowErrorMessageBeforeError = MockChatWindow::bodyFromMessage(mucWindow->lastAddedErrorMessage_);
+        manager_->handleIncomingMessage(messageErrorReply);
+
+        // assert that error is not routed to MUC window
+        CPPUNIT_ASSERT_EQUAL(lastMUCWindowErrorMessageBeforeError,  MockChatWindow::bodyFromMessage(mucWindow->lastAddedErrorMessage_));
+        // assert that error is routed to PM
+        CPPUNIT_ASSERT_EQUAL(std::string("This user could not be found in the room."), MockChatWindow::bodyFromMessage(window->lastAddedErrorMessage_));
+    }
+
     void testLocalMUCServiceDiscoveryResetOnDisconnect() {
         JID ownJID("test@test.com/resource");
         JID sender("foo@test.com");
diff --git a/Swift/Controllers/UnitTest/MockChatWindow.h b/Swift/Controllers/UnitTest/MockChatWindow.h
index 9b943f0..76e5194 100644
--- a/Swift/Controllers/UnitTest/MockChatWindow.h
+++ b/Swift/Controllers/UnitTest/MockChatWindow.h
@@ -69,7 +69,11 @@ namespace Swift {
             virtual void setAvailableSecurityLabels(const std::vector<SecurityLabelsCatalog::Item>& labels) {labels_ = labels;}
             virtual void setSecurityLabelsEnabled(bool enabled) {labelsEnabled_ = enabled;}
             virtual void setUnreadMessageCount(int /*count*/) {}
-            virtual void convertToMUC(MUCType /*mucType*/) {}
+
+            virtual void convertToMUC(MUCType mucType) {
+                mucType_ = mucType;
+            }
+
             virtual void setSecurityLabelsError() {}
             virtual SecurityLabelsCatalog::Item getSelectedSecurityLabel() {return label_;}
             virtual void setOnline(bool /*online*/) {}
@@ -144,6 +148,7 @@ namespace Swift {
             SecurityLabelsCatalog::Item label_;
             Roster* roster_ = nullptr;
             std::vector<std::pair<std::string, ReceiptState>> receiptChanges_;
+            boost::optional<MUCType> mucType_;
     };
 }
 
diff --git a/Swiften/Elements/Message.h b/Swiften/Elements/Message.h
index f276ef7..c357cd4 100644
--- a/Swiften/Elements/Message.h
+++ b/Swiften/Elements/Message.h
@@ -39,10 +39,8 @@ namespace Swift {
                 updatePayload(std::make_shared<Subject>(subject));
             }
 
-            // Explicitly convert to bool. In C++11, it would be cleaner to
-            // compare to nullptr.
             bool hasSubject() {
-                return static_cast<bool>(getPayload<Subject>());
+                return getPayload<Subject>() != nullptr;
             }
 
             boost::optional<std::string> getBody() const {