Swift
summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTobias Markmann <tm@ayena.de>2016-11-22 07:51:50 (GMT)
committerKevin Smith <kevin.smith@isode.com>2016-11-30 10:29:11 (GMT)
commitc65a90e2a73814d09ad8c60adc4a259e90006db7 (patch)
treec6b0f90fba9aa2c64e94e35e99c9b30557d5e4ad
parent2039930eadd4756068a8a60c8340d9908a7136d3 (diff)
downloadswift-c65a90e2a73814d09ad8c60adc4a259e90006db7.zip
swift-c65a90e2a73814d09ad8c60adc4a259e90006db7.tar.bz2
Show MUC PM item-not-found error in MUC PM window
Previously, if one would send a MUC PM to a user that has left a room the error response from the MUC would be shown in the MUC room. Now the error will show in the MUC PM window, if the MUC PM full JID has a ChatController. Test-Information: Added unit test verifying new behaviour. Manually verified that the error is shown in the MUC PM window instead of the MUC room window. Change-Id: I1b259d5eee9e22217bbe7e5c09294d2166a77895
Diffstat
-rw-r--r--Swift/Controllers/Chat/ChatControllerBase.cpp8
-rw-r--r--Swift/Controllers/Chat/ChatsManager.cpp14
-rw-r--r--Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp71
-rw-r--r--Swift/Controllers/UnitTest/MockChatWindow.h7
-rw-r--r--Swiften/Elements/Message.h4
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 {