From 901ca8337c983f098394c7d4889f74aad452b1d0 Mon Sep 17 00:00:00 2001 From: Tobias Markmann Date: Wed, 1 Mar 2017 14:21:53 +0000 Subject: Fix keyword highlights not issuing system notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adjusted the MUCController tests accordingly, as self-mentions in group chats have been case insensitive and are now case sensitive. Test-Information: Tested on macOS 10.12.3 with Qt 5.7.1, that with a highlight rule for the ‘Swift’ keyword, a system notification is generated when the app is inactive. Change-Id: I325b682c5afa81e05eec8cf3a8a15b2ff0303e5c diff --git a/Swift/Controllers/Chat/MUCController.cpp b/Swift/Controllers/Chat/MUCController.cpp index c476cf3..8fa98f6 100644 --- a/Swift/Controllers/Chat/MUCController.cpp +++ b/Swift/Controllers/Chat/MUCController.cpp @@ -592,8 +592,8 @@ void MUCController::addMessageHandleIncomingMessage(const JID& from, const ChatW void MUCController::postHandleIncomingMessage(std::shared_ptr messageEvent, const ChatWindow::ChatMessage& chatMessage) { std::shared_ptr message = messageEvent->getStanza(); if (joined_ && messageEvent->getStanza()->getFrom().getResource() != nick_ && !message->getPayload()) { - if (messageTargetsMe(message) || isImpromptu_) { - highlighter_->handleSystemNotifications(chatMessage, messageEvent); + highlighter_->handleSystemNotifications(chatMessage, messageEvent); + if (!messageEvent->getNotifications().empty()) { eventController_->handleIncomingEvent(messageEvent); } if (!messageEvent->getConcluded()) { diff --git a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp index 80f8346..00df3da 100644 --- a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp +++ b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp @@ -46,6 +46,7 @@ #include #include #include +#include #include #include #include @@ -63,8 +64,40 @@ #include #include +#include + using namespace Swift; +class DummyNotifier : public Notifier { + public: + virtual void showMessage( + Type type, + const std::string& subject, + const std::string& description, + const boost::filesystem::path& picture, + boost::function callback) { + notifications.push_back({type, subject, description, picture, callback}); + } + + /** Remove any pending callbacks. */ + virtual void purgeCallbacks() { + + } + + public: + struct Notification { + Type type; + std::string subject; + std::string description; + boost::filesystem::path picture; + boost::function callback; + }; + + public: + std::vector notifications; +}; + + class ChatsManagerTest : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(ChatsManagerTest); CPPUNIT_TEST(testFirstOpenWindowIncoming); @@ -91,6 +124,8 @@ class ChatsManagerTest : public CppUnit::TestFixture { // Highlighting tests CPPUNIT_TEST(testChatControllerHighlightingNotificationTesting); CPPUNIT_TEST(testChatControllerHighlightingNotificationDeduplicateSounds); + CPPUNIT_TEST(testChatControllerHighlightingNotificationKeyword); + CPPUNIT_TEST(testChatControllerMeMessageHandling); CPPUNIT_TEST(testRestartingMUCComponentCrash); CPPUNIT_TEST(testChatControllerMeMessageHandlingInMUC); @@ -104,6 +139,7 @@ class ChatsManagerTest : public CppUnit::TestFixture { public: void setUp() { mocks_ = new MockRepository(); + notifier_ = std::unique_ptr(new DummyNotifier()); jid_ = JID("test@test.com/resource"); stanzaChannel_ = new DummyStanzaChannel(); iqRouter_ = new IQRouter(stanzaChannel_); @@ -128,6 +164,7 @@ public: ftManager_ = new DummyFileTransferManager(); ftOverview_ = new FileTransferOverview(ftManager_); avatarManager_ = new NullAvatarManager(); + eventNotifier_ = new EventNotifier(eventController_, notifier_.get(), avatarManager_, nickResolver_); wbSessionManager_ = new WhiteboardSessionManager(iqRouter_, stanzaChannel_, presenceOracle_, entityCapsProvider_); wbManager_ = new WhiteboardManager(whiteboardWindowFactory_, uiEventStream_, nickResolver_, wbSessionManager_); highlightManager_ = new HighlightManager(settings_); @@ -149,6 +186,7 @@ public: void tearDown() { delete highlightManager_; delete profileSettings_; + delete eventNotifier_; delete avatarManager_; delete manager_; delete clientBlockListManager_; @@ -812,6 +850,7 @@ public: CPPUNIT_ASSERT_EQUAL(2, handledHighlightActions_); CPPUNIT_ASSERT(soundsPlayed_.find(keywordRuleA.action.getSoundFilePath().get_value_or("")) != soundsPlayed_.end()); CPPUNIT_ASSERT(soundsPlayed_.find(keywordRuleB.action.getSoundFilePath().get_value_or("")) != soundsPlayed_.end()); + CPPUNIT_ASSERT_EQUAL(size_t(1), notifier_->notifications.size()); } void testChatControllerHighlightingNotificationDeduplicateSounds() { @@ -841,6 +880,29 @@ public: CPPUNIT_ASSERT_EQUAL(1, handledHighlightActions_); CPPUNIT_ASSERT(soundsPlayed_.find(keywordRuleA.action.getSoundFilePath().get_value_or("")) != soundsPlayed_.end()); CPPUNIT_ASSERT(soundsPlayed_.find(keywordRuleB.action.getSoundFilePath().get_value_or("")) != soundsPlayed_.end()); + CPPUNIT_ASSERT_EQUAL(size_t(1), notifier_->notifications.size()); + } + + void testChatControllerHighlightingNotificationKeyword() { + auto keywordRuleA = HighlightConfiguration::KeywordHightlight(); + keywordRuleA.keyword = "Swift"; + keywordRuleA.action.setFrontColor(boost::optional("yellow")); + keywordRuleA.action.setSoundFilePath(boost::optional("")); + keywordRuleA.action.setSystemNotificationEnabled(true); + highlightManager_->getConfiguration()->keywordHighlights.push_back(keywordRuleA); + + JID messageJID = JID("testling@test.com"); + + MockChatWindow* window = new MockChatWindow(); + mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(messageJID, uiEventStream_).Return(window); + + std::shared_ptr message(new Message()); + message->setFrom(messageJID); + std::string body("Let's see if the Swift highlight kicks off a system notification."); + message->setBody(body); + manager_->handleIncomingMessage(message); + + CPPUNIT_ASSERT_EQUAL(size_t(2), notifier_->notifications.size()); } void testChatControllerMeMessageHandling() { @@ -917,6 +979,12 @@ public: keywordHighlight.action.setBackColor(boost::optional("green")); highlightManager_->getConfiguration()->keywordHighlights.push_back(keywordHighlight); + HighlightConfiguration::KeywordHightlight keywordHighlightNotification; + keywordHighlightNotification.keyword = "Swift"; + keywordHighlightNotification.action.setBackColor(boost::optional("green")); + keywordHighlightNotification.action.setSystemNotificationEnabled(true); + highlightManager_->getConfiguration()->keywordHighlights.push_back(keywordHighlightNotification); + MockChatWindow* window = new MockChatWindow(); mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(mucJID, uiEventStream_).Return(window); @@ -970,6 +1038,18 @@ public: manager_->handleIncomingMessage(mucMirrored); } CPPUNIT_ASSERT_EQUAL(std::string("says hello with a test message with foo and foo"), window->bodyFromMessage(window->lastAddedAction_)); + + window->resetLastMessages(); + { + Message::ref keywordMessage = std::make_shared(); + keywordMessage->setFrom(mucJID.withResource("someOtherDifferentNickname")); + keywordMessage->setTo(jid_); + keywordMessage->setType(Message::Groupchat); + keywordMessage->setBody("Let's mention Swift and see what happens."); + manager_->handleIncomingMessage(keywordMessage); + } + CPPUNIT_ASSERT_EQUAL(std::string("Let's mention Swift and see what happens."), window->bodyFromMessage(window->lastAddedMessage_)); + CPPUNIT_ASSERT_EQUAL(size_t(1), notifier_->notifications.size()); } void testPresenceChangeDoesNotReplaceMUCInvite() { @@ -1136,6 +1216,7 @@ private: private: JID jid_; + std::unique_ptr notifier_; ChatsManager* manager_; DummyStanzaChannel* stanzaChannel_; IQRouter* iqRouter_; @@ -1145,6 +1226,7 @@ private: NickResolver* nickResolver_; PresenceOracle* presenceOracle_; AvatarManager* avatarManager_; + EventNotifier* eventNotifier_; std::shared_ptr serverDiscoInfo_; XMPPRosterImpl* xmppRoster_; PresenceSender* presenceSender_; diff --git a/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp b/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp index eabf4c5..59c3a87 100644 --- a/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp +++ b/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp @@ -92,6 +92,7 @@ public: entityCapsProvider_ = new DummyEntityCapsProvider(); settings_ = new DummySettingsProvider(); highlightManager_ = new HighlightManager(settings_); + highlightManager_->resetToDefaultConfiguration(); muc_ = std::make_shared(mucJID_); mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(muc_->getJID(), uiEventStream_).Return(window_); chatMessageParser_ = std::make_shared(std::map(), highlightManager_->getConfiguration(), ChatMessageParser::Mode::GroupChat); @@ -188,21 +189,24 @@ public: message->setBody("Hi " + boost::to_lower_copy(nick_) + "."); message->setType(Message::Groupchat); controller_->handleIncomingMessage(MessageEvent::ref(new MessageEvent(message))); - CPPUNIT_ASSERT_EQUAL((size_t)4, eventController_->getEvents().size()); + + // The last message is ignored because self-mention highlights are matched case + // sensitive against the nickname. + CPPUNIT_ASSERT_EQUAL((size_t)3, eventController_->getEvents().size()); message = Message::ref(new Message()); message->setFrom(JID(muc_->getJID().toString() + "/other3")); message->setBody("Hi bert."); message->setType(Message::Groupchat); controller_->handleIncomingMessage(MessageEvent::ref(new MessageEvent(message))); - CPPUNIT_ASSERT_EQUAL((size_t)4, eventController_->getEvents().size()); + CPPUNIT_ASSERT_EQUAL((size_t)3, eventController_->getEvents().size()); message = Message::ref(new Message()); message->setFrom(JID(muc_->getJID().toString() + "/other2")); message->setBody("Hi " + boost::to_lower_copy(nick_) + "ie."); message->setType(Message::Groupchat); controller_->handleIncomingMessage(MessageEvent::ref(new MessageEvent(message))); - CPPUNIT_ASSERT_EQUAL((size_t)4, eventController_->getEvents().size()); + CPPUNIT_ASSERT_EQUAL((size_t)3, eventController_->getEvents().size()); } void testNotAddressedToSelf() { -- cgit v0.10.2-6-g49f6