From a9f6543e43c53c6cb6759b40876e082a8ae02480 Mon Sep 17 00:00:00 2001 From: Mick Sayson Date: Sun, 5 Apr 2020 20:45:56 -0700 Subject: [PATCH 1/3] feat(notification): Notification string generator for multiple messages --- CMakeLists.txt | 2 + cmake/Testing.cmake | 1 + src/model/notificationdata.h | 11 + src/model/notificationgenerator.cpp | 247 ++++++++++++++ src/model/notificationgenerator.h | 38 +++ src/persistence/igroupsettings.h | 2 - src/persistence/inotificationsettings.h | 50 +++ src/persistence/settings.h | 28 +- test/mock/mockcoreidhandler.h | 26 ++ test/mock/mockgroupquery.h | 63 ++++ test/model/groupmessagedispatcher_test.cpp | 89 +---- test/model/notificationgenerator_test.cpp | 375 +++++++++++++++++++++ 12 files changed, 831 insertions(+), 101 deletions(-) create mode 100644 src/model/notificationdata.h create mode 100644 src/model/notificationgenerator.cpp create mode 100644 src/model/notificationgenerator.h create mode 100644 src/persistence/inotificationsettings.h create mode 100644 test/mock/mockcoreidhandler.h create mode 100644 test/mock/mockgroupquery.h create mode 100644 test/model/notificationgenerator_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 4e991d554..f220c24cc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -369,6 +369,8 @@ set(${PROJECT_NAME}_SOURCES src/model/chathistory.cpp src/model/toxclientstandards.h src/model/ibootstraplistgenerator.h + src/model/notificationgenerator.h + src/model/notificationgenerator.cpp src/net/bootstrapnodeupdater.cpp src/net/bootstrapnodeupdater.h src/net/avatarbroadcaster.cpp diff --git a/cmake/Testing.cmake b/cmake/Testing.cmake index d92674e39..40052b8db 100644 --- a/cmake/Testing.cmake +++ b/cmake/Testing.cmake @@ -48,6 +48,7 @@ auto_test(model groupmessagedispatcher) auto_test(model messageprocessor) auto_test(model sessionchatlog) auto_test(model exiftransform) +auto_test(model notificationgenerator) if (UNIX) auto_test(platform posixsignalnotifier) diff --git a/src/model/notificationdata.h b/src/model/notificationdata.h new file mode 100644 index 000000000..8e0ac30a9 --- /dev/null +++ b/src/model/notificationdata.h @@ -0,0 +1,11 @@ +#pragma once + +#include +#include + +struct NotificationData +{ + QString title; + QString message; + QPixmap pixmap; +}; diff --git a/src/model/notificationgenerator.cpp b/src/model/notificationgenerator.cpp new file mode 100644 index 000000000..8e0948f00 --- /dev/null +++ b/src/model/notificationgenerator.cpp @@ -0,0 +1,247 @@ +#include "notificationgenerator.h" +#include "src/chatlog/content/filetransferwidget.h" + +#include + +namespace +{ + size_t getNumMessages( + const QHash& friendNotifications, + const QHash& groupNotifications) + { + auto numMessages = std::accumulate(friendNotifications.begin(), friendNotifications.end(), 0); + numMessages = std::accumulate(groupNotifications.begin(), groupNotifications.end(), numMessages); + + return numMessages; + } + + size_t getNumChats( + const QHash& friendNotifications, + const QHash& groupNotifications) + { + return friendNotifications.size() + groupNotifications.size(); + } + + QString generateMultiChatTitle(size_t numChats, size_t numMessages) + { + // FIXME: how do I tr this + return QObject::tr("%1 messages from %2 chats") + .arg(numMessages) + .arg(numChats); + } + + template + QString generateSingleChatTitle( + const QHash numNotifications, + T contact) + { + if (numNotifications[contact] > 1) + { + return QObject::tr("%1 messages from %2") + .arg(numNotifications[contact]) + .arg(contact->getDisplayedName()); + } + else + { + return QObject::tr("%1") + .arg(contact->getDisplayedName()); + } + } + + QString generateTitle( + const QHash& friendNotifications, + const QHash& groupNotifications, + const Friend* f) + { + auto numChats = getNumChats(friendNotifications, groupNotifications); + if (numChats > 1) + { + return generateMultiChatTitle(numChats, getNumMessages(friendNotifications, groupNotifications)); + } + else + { + return generateSingleChatTitle(friendNotifications, f); + } + } + + QString generateTitle( + const QHash& friendNotifications, + const QHash& groupNotifications, + const Group* g) + { + auto numChats = getNumChats(friendNotifications, groupNotifications); + if (numChats > 1) + { + return generateMultiChatTitle(numChats, getNumMessages(friendNotifications, groupNotifications)); + } + else + { + return generateSingleChatTitle(groupNotifications, g); + } + } + + QString generateContent( + const QHash& friendNotifications, + const QHash& groupNotifications, + QString lastMessage, + const ToxPk& sender) + { + assert(friendNotifications.size() > 0 || groupNotifications.size() > 0); + + auto numChats = getNumChats(friendNotifications, groupNotifications); + if (numChats > 1) { + // Copy all names into a vector to simplify formatting logic between + // multiple lists + std::vector displayNames; + displayNames.reserve(numChats); + + for (auto it = friendNotifications.begin(); it != friendNotifications.end(); ++it) { + displayNames.push_back(it.key()->getDisplayedName()); + } + + for (auto it = groupNotifications.begin(); it != groupNotifications.end(); ++it) { + displayNames.push_back(it.key()->getDisplayedName()); + } + + assert(displayNames.size() > 0); + + // Lexiographically sort all display names to ensure consistent formatting + QCollator collator; + std::sort(displayNames.begin(), displayNames.end(), [&] (const QString& a, const QString& b) { + return collator.compare(a, b) < 1; + }); + + auto it = displayNames.begin(); + + QString ret = *it; + + while (++it != displayNames.end()) { + ret += ", " + *it; + } + + return ret; + } + else { + if (groupNotifications.size() == 1) { + return groupNotifications.begin().key()->getPeerList()[sender] + ": " + lastMessage; + } + + return lastMessage; + } + } + + QPixmap getSenderAvatar(Profile* profile, const ToxPk& sender) + { + return profile ? profile->loadAvatar(sender) : QPixmap(); + } +} // namespace + +NotificationGenerator::NotificationGenerator( + INotificationSettings const& notificationSettings, + Profile* profile) + : notificationSettings(notificationSettings) + , profile(profile) +{} + +NotificationData NotificationGenerator::friendMessageNotification(const Friend* f, const QString& message) +{ + friendNotifications[f]++; + + NotificationData ret; + + if (notificationSettings.getNotifyHide()) { + ret.title = tr("New message"); + return ret; + } + + ret.title = generateTitle(friendNotifications, groupNotifications, f); + ret.message = generateContent(friendNotifications, groupNotifications, message, f->getPublicKey()); + ret.pixmap = getSenderAvatar(profile, f->getPublicKey()); + + return ret; +} + +NotificationData NotificationGenerator::groupMessageNotification(const Group* g, const ToxPk& sender, const QString& message) +{ + groupNotifications[g]++; + + NotificationData ret; + + if (notificationSettings.getNotifyHide()){ + ret.title = tr("New group message"); + return ret; + } + + ret.title = generateTitle(friendNotifications, groupNotifications, g); + ret.message = generateContent(friendNotifications, groupNotifications, message, sender); + ret.pixmap = getSenderAvatar(profile, sender); + + return ret; +} + +NotificationData NotificationGenerator::fileTransferNotification(const Friend* f, const QString& filename, size_t fileSize) +{ + friendNotifications[f]++; + + NotificationData ret; + + if (notificationSettings.getNotifyHide()) { + ret.title = tr("Incoming file transfer"); + return ret; + } + + auto numChats = getNumChats(friendNotifications, groupNotifications); + auto numMessages = getNumMessages(friendNotifications, groupNotifications); + + if (numChats > 1 || numMessages > 1) + { + ret.title = generateTitle(friendNotifications, groupNotifications, f); + ret.message = generateContent(friendNotifications, groupNotifications, tr("Incoming file transfer"), f->getPublicKey()); + } + else + { + ret.title = f->getDisplayedName() + " - " + tr("File sent"); + ret.message = filename + " (" + FileTransferWidget::getHumanReadableSize(fileSize) + ")"; + } + + ret.pixmap = getSenderAvatar(profile, f->getPublicKey()); + + return ret; +} + +NotificationData NotificationGenerator::groupInvitationNotification(const Friend* from) +{ + NotificationData ret; + + if (notificationSettings.getNotifyHide()) { + ret.title = tr("Group invite received"); + return ret; + } + + ret.title = from->getDisplayedName() + tr(" invites you to join a group."); + ret.message = ""; + ret.pixmap = getSenderAvatar(profile, from->getPublicKey()); + + return ret; +} + +NotificationData NotificationGenerator::friendRequestNotification(const ToxPk& sender, const QString& message) +{ + NotificationData ret; + + if (notificationSettings.getNotifyHide()) { + ret.title = tr("Friend request received"); + return ret; + } + + ret.title = sender.toString() + tr(" sent you a friend request."); + ret.message = message; + + return ret; +} + +void NotificationGenerator::onNotificationActivated() +{ + friendNotifications = {}; + groupNotifications = {}; +} diff --git a/src/model/notificationgenerator.h b/src/model/notificationgenerator.h new file mode 100644 index 000000000..13000e19e --- /dev/null +++ b/src/model/notificationgenerator.h @@ -0,0 +1,38 @@ +#pragma once + + +#include "notificationdata.h" +#include "friend.h" +#include "group.h" + +#include "src/persistence/inotificationsettings.h" +#include "src/persistence/profile.h" + +#include +#include + +class NotificationGenerator : public QObject +{ +public: + NotificationGenerator( + INotificationSettings const& notificationSettings, + // Optional profile input to lookup avatars. Avatar lookup is not + // currently mockable so we allow profile to be nullptr for unit + // testing + Profile* profile); + + NotificationData friendMessageNotification(const Friend* f, const QString& message); + NotificationData groupMessageNotification(const Group* g, const ToxPk& sender, const QString& message); + NotificationData fileTransferNotification(const Friend* f, const QString& filename, size_t fileSize); + NotificationData groupInvitationNotification(const Friend* from); + NotificationData friendRequestNotification(const ToxPk& sender, const QString& message); + +public slots: + void onNotificationActivated(); + +private: + INotificationSettings const& notificationSettings; + Profile* profile; + QHash friendNotifications; + QHash groupNotifications; +}; diff --git a/src/persistence/igroupsettings.h b/src/persistence/igroupsettings.h index 1c283822b..158fc484c 100644 --- a/src/persistence/igroupsettings.h +++ b/src/persistence/igroupsettings.h @@ -27,6 +27,4 @@ public: virtual ~IGroupSettings() = default; virtual QStringList getBlackList() const = 0; virtual void setBlackList(const QStringList& blist) = 0; - virtual bool getGroupAlwaysNotify() const = 0; - virtual void setGroupAlwaysNotify(bool newValue) = 0; }; diff --git a/src/persistence/inotificationsettings.h b/src/persistence/inotificationsettings.h new file mode 100644 index 000000000..666382950 --- /dev/null +++ b/src/persistence/inotificationsettings.h @@ -0,0 +1,50 @@ +/* + Copyright © 2014-2019 by The qTox Project Contributors + + This file is part of qTox, a Qt-based graphical interface for Tox. + + qTox is libre software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + qTox is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with qTox. If not, see . +*/ + +#ifndef INOTIFICATION_SETTINGS_H +#define INOTIFICATION_SETTINGS_H + +#include + +class INotificationSettings +{ +public: + virtual bool getNotify() const = 0; + virtual void setNotify(bool newValue) = 0; + + virtual bool getShowWindow() const = 0; + virtual void setShowWindow(bool newValue) = 0; + + virtual bool getDesktopNotify() const = 0; + virtual void setDesktopNotify(bool enabled) = 0; + + virtual bool getNotifySound() const = 0; + virtual void setNotifySound(bool newValue) = 0; + + virtual bool getNotifyHide() const = 0; + virtual void setNotifyHide(bool newValue) = 0; + + virtual bool getBusySound() const = 0; + virtual void setBusySound(bool newValue) = 0; + + virtual bool getGroupAlwaysNotify() const = 0; + virtual void setGroupAlwaysNotify(bool newValue) = 0; +}; + +#endif /*INOTIFICATION_SETTINGS_H*/ diff --git a/src/persistence/settings.h b/src/persistence/settings.h index 08f833304..17fdfa4fe 100644 --- a/src/persistence/settings.h +++ b/src/persistence/settings.h @@ -27,6 +27,7 @@ #include "src/persistence/paths.h" #include "src/persistence/ifriendsettings.h" #include "src/persistence/igroupsettings.h" +#include "src/persistence/inotificationsettings.h" #include "src/video/ivideosettings.h" #include @@ -50,7 +51,8 @@ class Settings : public QObject, public IFriendSettings, public IGroupSettings, public IAudioSettings, - public IVideoSettings + public IVideoSettings, + public INotificationSettings { Q_OBJECT @@ -311,23 +313,23 @@ public: bool getCheckUpdates() const; void setCheckUpdates(bool newValue); - bool getNotify() const; - void setNotify(bool newValue); + bool getNotify() const override; + void setNotify(bool newValue) override; - bool getShowWindow() const; - void setShowWindow(bool newValue); + bool getShowWindow() const override; + void setShowWindow(bool newValue) override; - bool getDesktopNotify() const; - void setDesktopNotify(bool enabled); + bool getDesktopNotify() const override; + void setDesktopNotify(bool enabled) override; - bool getNotifySound() const; - void setNotifySound(bool newValue); + bool getNotifySound() const override; + void setNotifySound(bool newValue) override; - bool getNotifyHide() const; - void setNotifyHide(bool newValue); + bool getNotifyHide() const override; + void setNotifyHide(bool newValue) override; - bool getBusySound() const; - void setBusySound(bool newValue); + bool getBusySound() const override; + void setBusySound(bool newValue) override; bool getGroupAlwaysNotify() const override; void setGroupAlwaysNotify(bool newValue) override; diff --git a/test/mock/mockcoreidhandler.h b/test/mock/mockcoreidhandler.h new file mode 100644 index 000000000..ab25d7357 --- /dev/null +++ b/test/mock/mockcoreidhandler.h @@ -0,0 +1,26 @@ +#pragma once + +#include "src/core/icoreidhandler.h" + +#include + +class MockCoreIdHandler : public ICoreIdHandler +{ +public: + ToxId getSelfId() const override + { + std::terminate(); + return ToxId(); + } + + ToxPk getSelfPublicKey() const override + { + static uint8_t id[TOX_PUBLIC_KEY_SIZE] = {0}; + return ToxPk(id); + } + + QString getUsername() const override + { + return "me"; + } +}; diff --git a/test/mock/mockgroupquery.h b/test/mock/mockgroupquery.h new file mode 100644 index 000000000..013922a2b --- /dev/null +++ b/test/mock/mockgroupquery.h @@ -0,0 +1,63 @@ +#pragma once + +#include "src/core/icoregroupquery.h" + +#include + +/** + * Mock 1 peer at group number 0 + */ +class MockGroupQuery : public ICoreGroupQuery +{ +public: + GroupId getGroupPersistentId(uint32_t groupNumber) const override + { + return GroupId(0); + } + + uint32_t getGroupNumberPeers(int groupId) const override + { + if (emptyGroup) { + return 1; + } + + return 2; + } + + QString getGroupPeerName(int groupId, int peerId) const override + { + return QString("peer") + peerId; + } + + ToxPk getGroupPeerPk(int groupId, int peerId) const override + { + uint8_t id[TOX_PUBLIC_KEY_SIZE] = {static_cast(peerId)}; + return ToxPk(id); + } + + QStringList getGroupPeerNames(int groupId) const override + { + if (emptyGroup) { + return QStringList({QString("me")}); + } + return QStringList({QString("me"), QString("other")}); + } + + bool getGroupAvEnabled(int groupId) const override + { + return false; + } + + void setAsEmptyGroup() + { + emptyGroup = true; + } + + void setAsFunctionalGroup() + { + emptyGroup = false; + } + +private: + bool emptyGroup = false; +}; diff --git a/test/model/groupmessagedispatcher_test.cpp b/test/model/groupmessagedispatcher_test.cpp index e8d0372d4..cdebc64b1 100644 --- a/test/model/groupmessagedispatcher_test.cpp +++ b/test/model/groupmessagedispatcher_test.cpp @@ -23,6 +23,9 @@ #include "src/model/message.h" #include "src/persistence/settings.h" +#include "test/mock/mockcoreidhandler.h" +#include "test/mock/mockgroupquery.h" + #include #include @@ -47,85 +50,6 @@ public: size_t numSentMessages = 0; }; -/** - * Mock 1 peer at group number 0 - */ -class MockGroupQuery : public ICoreGroupQuery -{ -public: - GroupId getGroupPersistentId(uint32_t groupNumber) const override - { - return GroupId(); - } - - uint32_t getGroupNumberPeers(int groupId) const override - { - if (emptyGroup) { - return 1; - } - - return 2; - } - - QString getGroupPeerName(int groupId, int peerId) const override - { - return QString("peer") + peerId; - } - - ToxPk getGroupPeerPk(int groupId, int peerId) const override - { - uint8_t id[TOX_PUBLIC_KEY_SIZE] = {static_cast(peerId)}; - return ToxPk(id); - } - - QStringList getGroupPeerNames(int groupId) const override - { - if (emptyGroup) { - return QStringList({QString("me")}); - } - return QStringList({QString("me"), QString("other")}); - } - - bool getGroupAvEnabled(int groupId) const override - { - return false; - } - - void setAsEmptyGroup() - { - emptyGroup = true; - } - - void setAsFunctionalGroup() - { - emptyGroup = false; - } - -private: - bool emptyGroup = false; -}; - -class MockCoreIdHandler : public ICoreIdHandler -{ -public: - ToxId getSelfId() const override - { - std::terminate(); - return ToxId(); - } - - ToxPk getSelfPublicKey() const override - { - static uint8_t id[TOX_PUBLIC_KEY_SIZE] = {0}; - return ToxPk(id); - } - - QString getUsername() const override - { - return "me"; - } -}; - class MockGroupSettings : public IGroupSettings { public: @@ -139,13 +63,6 @@ public: blacklist = blist; } - bool getGroupAlwaysNotify() const override - { - return false; - } - - void setGroupAlwaysNotify(bool newValue) override {} - private: QStringList blacklist; }; diff --git a/test/model/notificationgenerator_test.cpp b/test/model/notificationgenerator_test.cpp new file mode 100644 index 000000000..e9d77f984 --- /dev/null +++ b/test/model/notificationgenerator_test.cpp @@ -0,0 +1,375 @@ + +/* + Copyright © 2019 by The qTox Project Contributors + + This file is part of qTox, a Qt-based graphical interface for Tox. + + qTox is libre software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + qTox is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with qTox. If not, see . +*/ + +#include "src/model/notificationgenerator.h" + +#include "test/mock/mockcoreidhandler.h" +#include "test/mock/mockgroupquery.h" + +#include +#include + +namespace +{ + class MockNotificationSettings : public INotificationSettings + { + virtual bool getNotify() const override { return true; } + + virtual void setNotify(bool newValue) override {} + + virtual bool getShowWindow() const override { return true; } + virtual void setShowWindow(bool newValue) override {} + + virtual bool getDesktopNotify() const override { return true; } + virtual void setDesktopNotify(bool enabled) override {} + + virtual bool getNotifySound() const override { return true; } + virtual void setNotifySound(bool newValue) override {} + + virtual bool getNotifyHide() const override { return notifyHide; } + virtual void setNotifyHide(bool newValue) override { notifyHide = newValue; }; + + virtual bool getBusySound() const override { return true; } + virtual void setBusySound(bool newValue) override {} + + virtual bool getGroupAlwaysNotify() const override { return true; } + virtual void setGroupAlwaysNotify(bool newValue) override {} + private: + bool notifyHide = false; + }; + +} // namespace + +class TestNotificationGenerator : public QObject +{ + Q_OBJECT + +private slots: + void init(); + void testSingleFriendMessage(); + void testMultipleFriendMessages(); + void testNotificationClear(); + void testGroupMessage(); + void testMultipleGroupMessages(); + void testMultipleFriendSourceMessages(); + void testMultipleGroupSourceMessages(); + void testMixedSourceMessages(); + void testFileTransfer(); + void testFileTransferAfterMessage(); + void testGroupInvitation(); + void testGroupInviteUncounted(); + void testFriendRequest(); + void testFriendRequestUncounted(); + void testSimpleFriendMessage(); + void testSimpleFileTransfer(); + void testSimpleGroupMessage(); + void testSimpleFriendRequest(); + void testSimpleGroupInvite(); + void testSimpleMessageToggle(); + +private: + std::unique_ptr notificationSettings; + std::unique_ptr notificationGenerator; + std::unique_ptr groupQuery; + std::unique_ptr coreIdHandler; +}; + +void TestNotificationGenerator::init() +{ + notificationSettings.reset(new MockNotificationSettings()); + notificationGenerator.reset(new NotificationGenerator(*notificationSettings, nullptr)); + groupQuery.reset(new MockGroupQuery()); + coreIdHandler.reset(new MockCoreIdHandler()); +} + +void TestNotificationGenerator::testSingleFriendMessage() +{ + Friend f(0, ToxPk()); + f.setName("friendName"); + auto notificationData = notificationGenerator->friendMessageNotification(&f, "test"); + QVERIFY(notificationData.title == "friendName"); + QVERIFY(notificationData.message == "test"); +} + +void TestNotificationGenerator::testMultipleFriendMessages() +{ + Friend f(0, ToxPk()); + f.setName("friendName"); + notificationGenerator->friendMessageNotification(&f, "test"); + auto notificationData = notificationGenerator->friendMessageNotification(&f, "test2"); + QVERIFY(notificationData.title == "2 messages from friendName"); + QVERIFY(notificationData.message == "test2"); + + notificationData = notificationGenerator->friendMessageNotification(&f, "test3"); + QVERIFY(notificationData.title == "3 messages from friendName"); + QVERIFY(notificationData.message == "test3"); +} + +void TestNotificationGenerator::testNotificationClear() +{ + Friend f(0, ToxPk()); + f.setName("friendName"); + + notificationGenerator->friendMessageNotification(&f, "test"); + + // On notification clear we shouldn't see a notification count from the friend + notificationGenerator->onNotificationActivated(); + + auto notificationData = notificationGenerator->friendMessageNotification(&f, "test2"); + QVERIFY(notificationData.title == "friendName"); + QVERIFY(notificationData.message == "test2"); +} + +void TestNotificationGenerator::testGroupMessage() +{ + Group g(0, GroupId(0), "groupName", false, "selfName", *groupQuery, *coreIdHandler); + g.setName("groupName"); + auto sender = groupQuery->getGroupPeerPk(0, 0); + g.updateUsername(sender, "sender1"); + + auto notificationData = notificationGenerator->groupMessageNotification(&g, sender, "test"); + QVERIFY(notificationData.title == "groupName"); + QVERIFY(notificationData.message == "sender1: test"); +} + +void TestNotificationGenerator::testMultipleGroupMessages() +{ + Group g(0, GroupId(0), "groupName", false, "selfName", *groupQuery, *coreIdHandler); + g.setName("groupName"); + auto sender = groupQuery->getGroupPeerPk(0, 0); + g.updateUsername(sender, "sender1"); + + auto sender2 = groupQuery->getGroupPeerPk(0, 1); + g.updateUsername(sender2, "sender2"); + + auto notificationData = notificationGenerator->groupMessageNotification(&g, sender2, "test2"); + QVERIFY(notificationData.title == "groupName"); + QVERIFY(notificationData.message == "sender2: test2"); +} + +void TestNotificationGenerator::testMultipleFriendSourceMessages() +{ + Friend f(0, ToxPk()); + f.setName("friend1"); + + Friend f2(1, ToxPk()); + f2.setName("friend2"); + + notificationGenerator->friendMessageNotification(&f, "test1"); + auto notificationData = notificationGenerator->friendMessageNotification(&f2, "test2"); + + QVERIFY(notificationData.title == "2 messages from 2 chats"); + QVERIFY(notificationData.message == "friend1, friend2"); +} + +void TestNotificationGenerator::testMultipleGroupSourceMessages() +{ + Group g(0, GroupId(QByteArray(32, 0)), "groupName", false, "selfName", *groupQuery, *coreIdHandler); + g.setName("groupName"); + + Group g2(1, GroupId(QByteArray(32, 1)), "groupName", false, "selfName", *groupQuery, *coreIdHandler); + g.setName("groupName2"); + + auto sender = groupQuery->getGroupPeerPk(0, 0); + g.updateUsername(sender, "sender1"); + + notificationGenerator->groupMessageNotification(&g, sender, "test1"); + auto notificationData = notificationGenerator->groupMessageNotification(&g2, sender, "test1"); + + QVERIFY(notificationData.title == "2 messages from 2 chats"); + QVERIFY(notificationData.message == "groupName, groupName2"); +} + +void TestNotificationGenerator::testMixedSourceMessages() +{ + Friend f(0, ToxPk()); + f.setName("friend"); + + Group g(0, GroupId(QByteArray(32, 0)), "groupName", false, "selfName", *groupQuery, *coreIdHandler); + g.setName("group"); + + auto sender = groupQuery->getGroupPeerPk(0, 0); + g.updateUsername(sender, "sender1"); + + notificationGenerator->friendMessageNotification(&f, "test1"); + auto notificationData = notificationGenerator->groupMessageNotification(&g, sender, "test2"); + + QVERIFY(notificationData.title == "2 messages from 2 chats"); + QVERIFY(notificationData.message == "friend, group"); + + notificationData = notificationGenerator->fileTransferNotification(&f, "file", 0); + QVERIFY(notificationData.title == "3 messages from 2 chats"); + QVERIFY(notificationData.message == "friend, group"); +} + +void TestNotificationGenerator::testFileTransfer() +{ + Friend f(0, ToxPk()); + f.setName("friend"); + + auto notificationData = notificationGenerator->fileTransferNotification(&f, "file", 5 * 1024 * 1024 /* 5MB */); + + QVERIFY(notificationData.title == "friend - File sent"); + QVERIFY(notificationData.message == "file (5.00MiB)"); +} + +void TestNotificationGenerator::testFileTransferAfterMessage() +{ + Friend f(0, ToxPk()); + f.setName("friend"); + + notificationGenerator->friendMessageNotification(&f, "test1"); + auto notificationData = notificationGenerator->fileTransferNotification(&f, "file", 5 * 1024 * 1024 /* 5MB */); + + QVERIFY(notificationData.title == "2 messages from friend"); + QVERIFY(notificationData.message == "Incoming file transfer"); +} + +void TestNotificationGenerator::testGroupInvitation() +{ + Friend f(0, ToxPk()); + f.setName("friend"); + + auto notificationData = notificationGenerator->groupInvitationNotification(&f); + + QVERIFY(notificationData.title == "friend invites you to join a group."); + QVERIFY(notificationData.message == ""); +} + +void TestNotificationGenerator::testGroupInviteUncounted() +{ + Friend f(0, ToxPk()); + f.setName("friend"); + + notificationGenerator->friendMessageNotification(&f, "test"); + notificationGenerator->groupInvitationNotification(&f); + auto notificationData = notificationGenerator->friendMessageNotification(&f, "test2"); + + QVERIFY(notificationData.title == "2 messages from friend"); + QVERIFY(notificationData.message == "test2"); +} + +void TestNotificationGenerator::testFriendRequest() +{ + ToxPk sender(QByteArray(32, 0)); + + auto notificationData = notificationGenerator->friendRequestNotification(sender, "request"); + + QVERIFY(notificationData.title == "0000000000000000000000000000000000000000000000000000000000000000 sent you a friend request."); + QVERIFY(notificationData.message == "request"); +} + +void TestNotificationGenerator::testFriendRequestUncounted() +{ + Friend f(0, ToxPk()); + f.setName("friend"); + ToxPk sender(QByteArray(32, 0)); + + notificationGenerator->friendMessageNotification(&f, "test"); + notificationGenerator->friendRequestNotification(sender, "request"); + auto notificationData = notificationGenerator->friendMessageNotification(&f, "test2"); + + QVERIFY(notificationData.title == "2 messages from friend"); + QVERIFY(notificationData.message == "test2"); +} + +void TestNotificationGenerator::testSimpleFriendMessage() +{ + Friend f(0, ToxPk()); + f.setName("friend"); + + notificationSettings->setNotifyHide(true); + + auto notificationData = notificationGenerator->friendMessageNotification(&f, "test"); + + QVERIFY(notificationData.title == "New message"); + QVERIFY(notificationData.message == ""); +} + +void TestNotificationGenerator::testSimpleFileTransfer() +{ + Friend f(0, ToxPk()); + f.setName("friend"); + + notificationSettings->setNotifyHide(true); + + auto notificationData = notificationGenerator->fileTransferNotification(&f, "file", 0); + + QVERIFY(notificationData.title == "Incoming file transfer"); + QVERIFY(notificationData.message == ""); +} + +void TestNotificationGenerator::testSimpleGroupMessage() +{ + Group g(0, GroupId(0), "groupName", false, "selfName", *groupQuery, *coreIdHandler); + g.setName("groupName"); + auto sender = groupQuery->getGroupPeerPk(0, 0); + g.updateUsername(sender, "sender1"); + + notificationSettings->setNotifyHide(true); + + auto notificationData = notificationGenerator->groupMessageNotification(&g, sender, "test"); + QVERIFY(notificationData.title == "New group message"); + QVERIFY(notificationData.message == ""); +} + +void TestNotificationGenerator::testSimpleFriendRequest() +{ + ToxPk sender(QByteArray(32, 0)); + + notificationSettings->setNotifyHide(true); + + auto notificationData = notificationGenerator->friendRequestNotification(sender, "request"); + + QVERIFY(notificationData.title == "Friend request received"); + QVERIFY(notificationData.message == ""); +} + +void TestNotificationGenerator::testSimpleGroupInvite() +{ + Friend f(0, ToxPk()); + f.setName("friend"); + + notificationSettings->setNotifyHide(true); + auto notificationData = notificationGenerator->groupInvitationNotification(&f); + + QVERIFY(notificationData.title == "Group invite received"); + QVERIFY(notificationData.message == ""); +} + +void TestNotificationGenerator::testSimpleMessageToggle() +{ + Friend f(0, ToxPk()); + f.setName("friend"); + + notificationSettings->setNotifyHide(true); + + notificationGenerator->friendMessageNotification(&f, "test"); + + notificationSettings->setNotifyHide(false); + + auto notificationData = notificationGenerator->friendMessageNotification(&f, "test2"); + + QVERIFY(notificationData.title == "2 messages from friend"); + QVERIFY(notificationData.message == "test2"); +} + +QTEST_GUILESS_MAIN(TestNotificationGenerator) +#include "notificationgenerator_test.moc" From 6e163ca5edf3d3ac23ac6fed6521f921a22492a4 Mon Sep 17 00:00:00 2001 From: Mick Sayson Date: Sun, 1 Mar 2020 14:37:57 -0800 Subject: [PATCH 2/3] feat(notification): Notifications now always replace the previous one This is a feature/fix to improve notification behavior when we receive over 3 messages. SnoreNotify prevents over 3 notifications from being displayed before a user clears the notification. This is presumably to avoid infinite notification spam. Unfortunately this results in the notifications just coming in _after_ the user clears them. This means if there are 100s of messages built up the user has to clear their notifications N messages / 3 times. This feature/fix folds all notifications into a single notification representing the current qTox notification state. See notificationgenerator_test.cpp for what the new messages look like. --- src/model/notificationdata.h | 19 ++++++ src/model/notificationgenerator.cpp | 36 ++++++++--- src/model/notificationgenerator.h | 19 ++++++ src/persistence/inotificationsettings.h | 7 +- .../desktop_notifications/desktopnotify.cpp | 64 +++++++++++-------- .../desktop_notifications/desktopnotify.h | 27 ++++---- src/widget/widget.cpp | 55 +++++----------- src/widget/widget.h | 4 +- test/mock/mockcoreidhandler.h | 19 ++++++ test/mock/mockgroupquery.h | 19 ++++++ test/model/notificationgenerator_test.cpp | 40 ++++++------ 11 files changed, 193 insertions(+), 116 deletions(-) diff --git a/src/model/notificationdata.h b/src/model/notificationdata.h index 8e0ac30a9..98d83f5aa 100644 --- a/src/model/notificationdata.h +++ b/src/model/notificationdata.h @@ -1,3 +1,22 @@ +/* + Copyright © 2020 by The qTox Project Contributors + + This file is part of qTox, a Qt-based graphical interface for Tox. + + qTox is libre software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + qTox is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with qTox. If not, see . +*/ + #pragma once #include diff --git a/src/model/notificationgenerator.cpp b/src/model/notificationgenerator.cpp index 8e0948f00..7e0ee49a7 100644 --- a/src/model/notificationgenerator.cpp +++ b/src/model/notificationgenerator.cpp @@ -1,3 +1,22 @@ +/* + Copyright © 2020 by The qTox Project Contributors + + This file is part of qTox, a Qt-based graphical interface for Tox. + + qTox is libre software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + qTox is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with qTox. If not, see . +*/ + #include "notificationgenerator.h" #include "src/chatlog/content/filetransferwidget.h" @@ -24,8 +43,8 @@ namespace QString generateMultiChatTitle(size_t numChats, size_t numMessages) { - // FIXME: how do I tr this - return QObject::tr("%1 messages from %2 chats") + //: e.g. 3 messages from 2 chats + return QObject::tr("%1 message(s) from %2 chats") .arg(numMessages) .arg(numChats); } @@ -37,14 +56,14 @@ namespace { if (numNotifications[contact] > 1) { - return QObject::tr("%1 messages from %2") + //: e.g. 2 messages from Bob + return QObject::tr("%1 message(s) from %2") .arg(numNotifications[contact]) .arg(contact->getDisplayedName()); } else { - return QObject::tr("%1") - .arg(contact->getDisplayedName()); + return contact->getDisplayedName(); } } @@ -200,7 +219,8 @@ NotificationData NotificationGenerator::fileTransferNotification(const Friend* f } else { - ret.title = f->getDisplayedName() + " - " + tr("File sent"); + //: e.g. Bob - file transfer + ret.title = tr("%1 - file transfer").arg(f->getDisplayedName()); ret.message = filename + " (" + FileTransferWidget::getHumanReadableSize(fileSize) + ")"; } @@ -218,7 +238,7 @@ NotificationData NotificationGenerator::groupInvitationNotification(const Friend return ret; } - ret.title = from->getDisplayedName() + tr(" invites you to join a group."); + ret.title = tr("%1 invites you to join a group.").arg(from->getDisplayedName()); ret.message = ""; ret.pixmap = getSenderAvatar(profile, from->getPublicKey()); @@ -234,7 +254,7 @@ NotificationData NotificationGenerator::friendRequestNotification(const ToxPk& s return ret; } - ret.title = sender.toString() + tr(" sent you a friend request."); + ret.title = tr("Friend request received from %1").arg(sender.toString()); ret.message = message; return ret; diff --git a/src/model/notificationgenerator.h b/src/model/notificationgenerator.h index 13000e19e..d21b6f38f 100644 --- a/src/model/notificationgenerator.h +++ b/src/model/notificationgenerator.h @@ -1,3 +1,22 @@ +/* + Copyright © 2020 by The qTox Project Contributors + + This file is part of qTox, a Qt-based graphical interface for Tox. + + qTox is libre software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + qTox is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with qTox. If not, see . +*/ + #pragma once diff --git a/src/persistence/inotificationsettings.h b/src/persistence/inotificationsettings.h index 666382950..cdaa87d14 100644 --- a/src/persistence/inotificationsettings.h +++ b/src/persistence/inotificationsettings.h @@ -1,5 +1,5 @@ /* - Copyright © 2014-2019 by The qTox Project Contributors + Copyright © 2020 by The qTox Project Contributors This file is part of qTox, a Qt-based graphical interface for Tox. @@ -17,8 +17,7 @@ along with qTox. If not, see . */ -#ifndef INOTIFICATION_SETTINGS_H -#define INOTIFICATION_SETTINGS_H +#pragma once #include @@ -46,5 +45,3 @@ public: virtual bool getGroupAlwaysNotify() const = 0; virtual void setGroupAlwaysNotify(bool newValue) = 0; }; - -#endif /*INOTIFICATION_SETTINGS_H*/ diff --git a/src/platform/desktop_notifications/desktopnotify.cpp b/src/platform/desktop_notifications/desktopnotify.cpp index 59b88dd38..1f4f24d1a 100644 --- a/src/platform/desktop_notifications/desktopnotify.cpp +++ b/src/platform/desktop_notifications/desktopnotify.cpp @@ -25,6 +25,7 @@ #include #include +#include DesktopNotify::DesktopNotify() : notifyCore{Snore::SnoreCore::instance()} @@ -37,43 +38,52 @@ DesktopNotify::DesktopNotify() snoreApp = Snore::Application("qTox", snoreIcon); notifyCore.registerApplication(snoreApp); + + connect(¬ifyCore, &Snore::SnoreCore::notificationClosed, this, &DesktopNotify::onNotificationClose); } -void DesktopNotify::createNotification(const QString& title, const QString& text, Snore::Icon& icon) +void DesktopNotify::notifyMessage(const NotificationData& notificationData) { const Settings& s = Settings::getInstance(); if(!(s.getNotify() && s.getDesktopNotify())) { return; } - Snore::Notification notify{snoreApp, Snore::Alert(), title, text, icon}; + auto icon = notificationData.pixmap.isNull() ? snoreIcon : Snore::Icon(notificationData.pixmap); + auto newNotification = Snore::Notification{snoreApp, Snore::Alert(), notificationData.title, notificationData.message, icon, 0}; + latestId = newNotification.id(); - notifyCore.broadcastNotification(notify); -} - -void DesktopNotify::notifyMessage(const QString& title, const QString& message) -{ - createNotification(title, message, snoreIcon); -} - -void DesktopNotify::notifyMessagePixmap(const QString& title, const QString& message, QPixmap avatar) -{ - Snore::Icon new_icon(avatar); - createNotification(title, message, new_icon); -} - -void DesktopNotify::notifyMessageSimple(const MessageType type) -{ - QString message; - switch (type) { - case MessageType::FRIEND: message = tr("New message"); break; - case MessageType::FRIEND_FILE: message = tr("Incoming file transfer"); break; - case MessageType::FRIEND_REQUEST: message = tr("Friend request received"); break; - case MessageType::GROUP: message = tr("New group message"); break; - case MessageType::GROUP_INVITE: message = tr("Group invite received"); break; - default: break; + if (lastNotification.isValid()) { + // Workaround for broken updating behavior in snore. Snore increments + // the message count when a notification is updated. Snore also caps the + // number of outgoing messages at 3. This means that if we update + // notifications more than 3 times we do not get notifications until the + // user activates the notification. + // + // We work around this by closing the existing notification and replacing + // it with a new one. We then only process the notification close if the + // latest notification id is the same as the one we are closing. This allows + // us to continue counting how many unread messages a user has until they + // close the notification themselves. + // + // I've filed a bug on the snorenotify mailing list but the project seems + // pretty dead. I filed a ticket on March 11 2020, and as of April 5 2020 + // the moderators have not even acknowledged the message. A previous message + // got a response starting with "Snorenotify isn't that well maintained any more" + // (see https://mail.kde.org/pipermail/snorenotify/2019-March/000004.html) + // so I don't have hope of this being fixed any time soon + notifyCore.requestCloseNotification(lastNotification, Snore::Notification::CloseReasons::Dismissed); } - createNotification(message, {}, snoreIcon); + notifyCore.broadcastNotification(newNotification); + lastNotification = newNotification; +} + +void DesktopNotify::onNotificationClose(Snore::Notification notification) +{ + if (notification.id() == latestId) { + lastNotification = {}; + emit notificationClosed(); + } } #endif diff --git a/src/platform/desktop_notifications/desktopnotify.h b/src/platform/desktop_notifications/desktopnotify.h index cd6baedad..f162de3c3 100644 --- a/src/platform/desktop_notifications/desktopnotify.h +++ b/src/platform/desktop_notifications/desktopnotify.h @@ -19,11 +19,14 @@ #pragma once -#if DESKTOP_NOTIFICATIONS +#include "src/model/notificationdata.h" + #include #include + #include +#include class DesktopNotify : public QObject { @@ -31,25 +34,19 @@ class DesktopNotify : public QObject public: DesktopNotify(); - enum class MessageType { - FRIEND, - FRIEND_FILE, - FRIEND_REQUEST, - GROUP, - GROUP_INVITE - }; - public slots: - void notifyMessage(const QString& title, const QString& message); - void notifyMessagePixmap(const QString& title, const QString& message, QPixmap avatar); - void notifyMessageSimple(const MessageType type); + void notifyMessage(const NotificationData& notificationData); -private: - void createNotification(const QString& title, const QString& text, Snore::Icon& icon); +signals: + void notificationClosed(); + +private slots: + void onNotificationClose(Snore::Notification notification); private: Snore::SnoreCore& notifyCore; Snore::Application snoreApp; Snore::Icon snoreIcon; + Snore::Notification lastNotification; + uint latestId; }; -#endif // DESKTOP_NOTIFICATIONS diff --git a/src/widget/widget.cpp b/src/widget/widget.cpp index 9972ab833..0bac9f1e6 100644 --- a/src/widget/widget.cpp +++ b/src/widget/widget.cpp @@ -294,6 +294,11 @@ void Widget::init() profileInfo = new ProfileInfo(core, profile); profileForm = new ProfileForm(profileInfo); +#if DESKTOP_NOTIFICATIONS + notificationGenerator.reset(new NotificationGenerator(settings, profile)); + connect(¬ifier, &DesktopNotify::notificationClosed, notificationGenerator.get(), &NotificationGenerator::onNotificationActivated); +#endif + // connect logout tray menu action connect(actionLogout, &QAction::triggered, profileForm, &ProfileForm::onLogoutClicked); @@ -1498,7 +1503,7 @@ void Widget::addGroupDialog(Group* group, ContentDialog* dialog) emit widget->chatroomWidgetClicked(widget); } -bool Widget::newFriendMessageAlert(const ToxPk& friendId, const QString& text, bool sound, bool file) +bool Widget::newFriendMessageAlert(const ToxPk& friendId, const QString& text, bool sound, QString filename, size_t filesize) { bool hasActive; QWidget* currentWindow; @@ -1535,17 +1540,9 @@ bool Widget::newFriendMessageAlert(const ToxPk& friendId, const QString& text, b widget->updateStatusLight(); ui->friendList->trackWidget(widget); #if DESKTOP_NOTIFICATIONS - if (settings.getNotifyHide()) { - notifier.notifyMessageSimple(file ? DesktopNotify::MessageType::FRIEND_FILE - : DesktopNotify::MessageType::FRIEND); - } else { - QString title = f->getDisplayedName(); - if (file) { - title += " - " + tr("File sent"); - } - notifier.notifyMessagePixmap(title, text, - Nexus::getProfile()->loadAvatar(f->getPublicKey())); - } + auto notificationData = filename.isEmpty() ? notificationGenerator->friendMessageNotification(f, text) + : notificationGenerator->fileTransferNotification(f, filename, filesize); + notifier.notifyMessage(notificationData); #endif if (contentDialog == nullptr) { @@ -1586,18 +1583,8 @@ bool Widget::newGroupMessageAlert(const GroupId& groupId, const ToxPk& authorPk, g->setEventFlag(true); widget->updateStatusLight(); #if DESKTOP_NOTIFICATIONS - if (settings.getNotifyHide()) { - notifier.notifyMessageSimple(DesktopNotify::MessageType::GROUP); - } else { - Friend* f = FriendList::findFriend(authorPk); - QString title = g->getPeerList().value(authorPk) + " (" + g->getDisplayedName() + ")"; - if (!f) { - notifier.notifyMessage(title, message); - } else { - notifier.notifyMessagePixmap(title, message, - Nexus::getProfile()->loadAvatar(f->getPublicKey())); - } - } + auto notificationData = notificationGenerator->groupMessageNotification(g, authorPk, message); + notifier.notifyMessage(notificationData); #endif if (contentDialog == nullptr) { @@ -1673,11 +1660,8 @@ void Widget::onFriendRequestReceived(const ToxPk& friendPk, const QString& messa friendRequestsUpdate(); newMessageAlert(window(), isActiveWindow(), true, true); #if DESKTOP_NOTIFICATIONS - if (settings.getNotifyHide()) { - notifier.notifyMessageSimple(DesktopNotify::MessageType::FRIEND_REQUEST); - } else { - notifier.notifyMessage(friendPk.toString() + tr(" sent you a friend request."), message); - } + auto notificationData = notificationGenerator->friendRequestNotification(friendPk, message); + notifier.notifyMessage(notificationData); #endif } } @@ -1686,9 +1670,8 @@ void Widget::onFileReceiveRequested(const ToxFile& file) { const ToxPk& friendPk = FriendList::id2Key(file.friendId); newFriendMessageAlert(friendPk, - file.fileName + " (" - + FileTransferWidget::getHumanReadableSize(file.filesize) + ")", - true, true); + {}, + true, file.fileName, file.filesize); } void Widget::updateFriendActivity(const Friend& frnd) @@ -1934,12 +1917,8 @@ void Widget::onGroupInviteReceived(const GroupInvite& inviteInfo) groupInvitesUpdate(); newMessageAlert(window(), isActiveWindow(), true, true); #if DESKTOP_NOTIFICATIONS - if (settings.getNotifyHide()) { - notifier.notifyMessageSimple(DesktopNotify::MessageType::GROUP_INVITE); - } else { - notifier.notifyMessagePixmap(f->getDisplayedName() + tr(" invites you to join a group."), - {}, Nexus::getProfile()->loadAvatar(f->getPublicKey())); - } + auto notificationData = notificationGenerator->groupInvitationNotification(f); + notifier.notifyMessage(notificationData); #endif } } else { diff --git a/src/widget/widget.h b/src/widget/widget.h index 34170a4ee..482889a3b 100644 --- a/src/widget/widget.h +++ b/src/widget/widget.h @@ -38,6 +38,7 @@ #include "src/model/friendmessagedispatcher.h" #include "src/model/groupmessagedispatcher.h" #if DESKTOP_NOTIFICATIONS +#include "src/model/notificationgenerator.h" #include "src/platform/desktop_notifications/desktopnotify.h" #endif @@ -127,7 +128,7 @@ public: void addFriendDialog(const Friend* frnd, ContentDialog* dialog); void addGroupDialog(Group* group, ContentDialog* dialog); bool newFriendMessageAlert(const ToxPk& friendId, const QString& text, bool sound = true, - bool file = false); + QString filename = QString(), size_t filesize = 0); bool newGroupMessageAlert(const GroupId& groupId, const ToxPk& authorPk, const QString& message, bool notify); bool getIsWindowMinimized(); @@ -360,6 +361,7 @@ private: MessageProcessor::SharedParams sharedMessageProcessorParams; #if DESKTOP_NOTIFICATIONS + std::unique_ptr notificationGenerator; DesktopNotify notifier; #endif diff --git a/test/mock/mockcoreidhandler.h b/test/mock/mockcoreidhandler.h index ab25d7357..8aad8117a 100644 --- a/test/mock/mockcoreidhandler.h +++ b/test/mock/mockcoreidhandler.h @@ -1,3 +1,22 @@ +/* + Copyright © 2020 by The qTox Project Contributors + + This file is part of qTox, a Qt-based graphical interface for Tox. + + qTox is libre software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + qTox is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with qTox. If not, see . +*/ + #pragma once #include "src/core/icoreidhandler.h" diff --git a/test/mock/mockgroupquery.h b/test/mock/mockgroupquery.h index 013922a2b..b039c7cbc 100644 --- a/test/mock/mockgroupquery.h +++ b/test/mock/mockgroupquery.h @@ -1,3 +1,22 @@ +/* + Copyright © 2020 by The qTox Project Contributors + + This file is part of qTox, a Qt-based graphical interface for Tox. + + qTox is libre software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + qTox is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with qTox. If not, see . +*/ + #pragma once #include "src/core/icoregroupquery.h" diff --git a/test/model/notificationgenerator_test.cpp b/test/model/notificationgenerator_test.cpp index e9d77f984..b1135a688 100644 --- a/test/model/notificationgenerator_test.cpp +++ b/test/model/notificationgenerator_test.cpp @@ -114,11 +114,11 @@ void TestNotificationGenerator::testMultipleFriendMessages() f.setName("friendName"); notificationGenerator->friendMessageNotification(&f, "test"); auto notificationData = notificationGenerator->friendMessageNotification(&f, "test2"); - QVERIFY(notificationData.title == "2 messages from friendName"); + QVERIFY(notificationData.title == "2 message(s) from friendName"); QVERIFY(notificationData.message == "test2"); notificationData = notificationGenerator->friendMessageNotification(&f, "test3"); - QVERIFY(notificationData.title == "3 messages from friendName"); + QVERIFY(notificationData.title == "3 message(s) from friendName"); QVERIFY(notificationData.message == "test3"); } @@ -140,7 +140,6 @@ void TestNotificationGenerator::testNotificationClear() void TestNotificationGenerator::testGroupMessage() { Group g(0, GroupId(0), "groupName", false, "selfName", *groupQuery, *coreIdHandler); - g.setName("groupName"); auto sender = groupQuery->getGroupPeerPk(0, 0); g.updateUsername(sender, "sender1"); @@ -152,15 +151,17 @@ void TestNotificationGenerator::testGroupMessage() void TestNotificationGenerator::testMultipleGroupMessages() { Group g(0, GroupId(0), "groupName", false, "selfName", *groupQuery, *coreIdHandler); - g.setName("groupName"); + auto sender = groupQuery->getGroupPeerPk(0, 0); g.updateUsername(sender, "sender1"); auto sender2 = groupQuery->getGroupPeerPk(0, 1); g.updateUsername(sender2, "sender2"); + notificationGenerator->groupMessageNotification(&g, sender, "test1"); + auto notificationData = notificationGenerator->groupMessageNotification(&g, sender2, "test2"); - QVERIFY(notificationData.title == "groupName"); + QVERIFY(notificationData.title == "2 message(s) from groupName"); QVERIFY(notificationData.message == "sender2: test2"); } @@ -175,17 +176,14 @@ void TestNotificationGenerator::testMultipleFriendSourceMessages() notificationGenerator->friendMessageNotification(&f, "test1"); auto notificationData = notificationGenerator->friendMessageNotification(&f2, "test2"); - QVERIFY(notificationData.title == "2 messages from 2 chats"); + QVERIFY(notificationData.title == "2 message(s) from 2 chats"); QVERIFY(notificationData.message == "friend1, friend2"); } void TestNotificationGenerator::testMultipleGroupSourceMessages() { Group g(0, GroupId(QByteArray(32, 0)), "groupName", false, "selfName", *groupQuery, *coreIdHandler); - g.setName("groupName"); - - Group g2(1, GroupId(QByteArray(32, 1)), "groupName", false, "selfName", *groupQuery, *coreIdHandler); - g.setName("groupName2"); + Group g2(1, GroupId(QByteArray(32, 1)), "groupName2", false, "selfName", *groupQuery, *coreIdHandler); auto sender = groupQuery->getGroupPeerPk(0, 0); g.updateUsername(sender, "sender1"); @@ -193,7 +191,7 @@ void TestNotificationGenerator::testMultipleGroupSourceMessages() notificationGenerator->groupMessageNotification(&g, sender, "test1"); auto notificationData = notificationGenerator->groupMessageNotification(&g2, sender, "test1"); - QVERIFY(notificationData.title == "2 messages from 2 chats"); + QVERIFY(notificationData.title == "2 message(s) from 2 chats"); QVERIFY(notificationData.message == "groupName, groupName2"); } @@ -202,8 +200,7 @@ void TestNotificationGenerator::testMixedSourceMessages() Friend f(0, ToxPk()); f.setName("friend"); - Group g(0, GroupId(QByteArray(32, 0)), "groupName", false, "selfName", *groupQuery, *coreIdHandler); - g.setName("group"); + Group g(0, GroupId(QByteArray(32, 0)), "group", false, "selfName", *groupQuery, *coreIdHandler); auto sender = groupQuery->getGroupPeerPk(0, 0); g.updateUsername(sender, "sender1"); @@ -211,11 +208,11 @@ void TestNotificationGenerator::testMixedSourceMessages() notificationGenerator->friendMessageNotification(&f, "test1"); auto notificationData = notificationGenerator->groupMessageNotification(&g, sender, "test2"); - QVERIFY(notificationData.title == "2 messages from 2 chats"); + QVERIFY(notificationData.title == "2 message(s) from 2 chats"); QVERIFY(notificationData.message == "friend, group"); notificationData = notificationGenerator->fileTransferNotification(&f, "file", 0); - QVERIFY(notificationData.title == "3 messages from 2 chats"); + QVERIFY(notificationData.title == "3 message(s) from 2 chats"); QVERIFY(notificationData.message == "friend, group"); } @@ -226,7 +223,7 @@ void TestNotificationGenerator::testFileTransfer() auto notificationData = notificationGenerator->fileTransferNotification(&f, "file", 5 * 1024 * 1024 /* 5MB */); - QVERIFY(notificationData.title == "friend - File sent"); + QVERIFY(notificationData.title == "friend - file transfer"); QVERIFY(notificationData.message == "file (5.00MiB)"); } @@ -238,7 +235,7 @@ void TestNotificationGenerator::testFileTransferAfterMessage() notificationGenerator->friendMessageNotification(&f, "test1"); auto notificationData = notificationGenerator->fileTransferNotification(&f, "file", 5 * 1024 * 1024 /* 5MB */); - QVERIFY(notificationData.title == "2 messages from friend"); + QVERIFY(notificationData.title == "2 message(s) from friend"); QVERIFY(notificationData.message == "Incoming file transfer"); } @@ -262,7 +259,7 @@ void TestNotificationGenerator::testGroupInviteUncounted() notificationGenerator->groupInvitationNotification(&f); auto notificationData = notificationGenerator->friendMessageNotification(&f, "test2"); - QVERIFY(notificationData.title == "2 messages from friend"); + QVERIFY(notificationData.title == "2 message(s) from friend"); QVERIFY(notificationData.message == "test2"); } @@ -272,7 +269,7 @@ void TestNotificationGenerator::testFriendRequest() auto notificationData = notificationGenerator->friendRequestNotification(sender, "request"); - QVERIFY(notificationData.title == "0000000000000000000000000000000000000000000000000000000000000000 sent you a friend request."); + QVERIFY(notificationData.title == "Friend request received from 0000000000000000000000000000000000000000000000000000000000000000"); QVERIFY(notificationData.message == "request"); } @@ -286,7 +283,7 @@ void TestNotificationGenerator::testFriendRequestUncounted() notificationGenerator->friendRequestNotification(sender, "request"); auto notificationData = notificationGenerator->friendMessageNotification(&f, "test2"); - QVERIFY(notificationData.title == "2 messages from friend"); + QVERIFY(notificationData.title == "2 message(s) from friend"); QVERIFY(notificationData.message == "test2"); } @@ -319,7 +316,6 @@ void TestNotificationGenerator::testSimpleFileTransfer() void TestNotificationGenerator::testSimpleGroupMessage() { Group g(0, GroupId(0), "groupName", false, "selfName", *groupQuery, *coreIdHandler); - g.setName("groupName"); auto sender = groupQuery->getGroupPeerPk(0, 0); g.updateUsername(sender, "sender1"); @@ -367,7 +363,7 @@ void TestNotificationGenerator::testSimpleMessageToggle() auto notificationData = notificationGenerator->friendMessageNotification(&f, "test2"); - QVERIFY(notificationData.title == "2 messages from friend"); + QVERIFY(notificationData.title == "2 message(s) from friend"); QVERIFY(notificationData.message == "test2"); } From ca4f9df1eecb43779ec7469ec24908a1a5f53a47 Mon Sep 17 00:00:00 2001 From: Anthony Bilinski Date: Thu, 28 May 2020 20:59:55 -0700 Subject: [PATCH 3/3] fix(notification): hide snore warning log spam snorenotify logs this when we call requestCloseNotification correctly. The behaviour still works, so we can just mask the warning for now. The issue has been reported upstream: https://github.com/qTox/qTox/pull/6073#pullrequestreview-420748519 --- src/main.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main.cpp b/src/main.cpp index e276573f3..0d795abd3 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -98,6 +98,16 @@ void logMessageHandler(QtMsgType type, const QMessageLogContext& ctxt, const QSt && msg == QString("QFSFileEngine::open: No file name specified")) return; + QRegExp snoreFilter{QStringLiteral("Snore::Notification.*was already closed")}; + if (type == QtWarningMsg + && msg.contains(snoreFilter)) + { + // snorenotify logs this when we call requestCloseNotification correctly. The behaviour still works, so we'll + // just mask the warning for now. The issue has been reported upstream: + // https://github.com/qTox/qTox/pull/6073#pullrequestreview-420748519 + return; + } + QString file = ctxt.file; // We're not using QT_MESSAGELOG_FILE here, because that can be 0, NULL, or // nullptr in release builds.