From 73c1a3dc286af9ffe25428276c6e94ae02a517cc Mon Sep 17 00:00:00 2001 From: Eetu Lehmusvuo Date: Wed, 18 Jan 2012 11:29:53 +0200 Subject: [PATCH] Fixes: NB#297027 - Notification group does not show the real latest timestamp RevBy: Vesa Halttunen --- debian/changelog | 1 + .../notifications/notificationareasink.cpp | 4 +- .../notifications/notificationmanager.cpp | 41 +++- .../notifications/notificationmanager.h | 6 + .../ut_notificationareasink.cpp | 13 +- .../ut_notificationmanager.cpp | 200 +++++++++++++++++- .../ut_notificationmanager.h | 12 +- 7 files changed, 252 insertions(+), 25 deletions(-) diff --git a/debian/changelog b/debian/changelog index f9e2015a..b8fb04e6 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,6 +2,7 @@ system-ui (1.3.1~1) unstable; urgency=low * [UNRELEASED] * Fixes: NB#295751 - Device shows OS update message after SSU from PR1 40-4 to PR2 50-5 + * Fixes: NB#297027 - Notification group does not show the real latest timestamp -- Vesa Halttunen Mon, 16 Jan 2012 14:07:50 +0200 diff --git a/src/systemui/notifications/notificationareasink.cpp b/src/systemui/notifications/notificationareasink.cpp index 20e17afc..809af7d9 100644 --- a/src/systemui/notifications/notificationareasink.cpp +++ b/src/systemui/notifications/notificationareasink.cpp @@ -16,6 +16,7 @@ ** of this file. ** ****************************************************************************/ + #include "notificationareasink.h" #include "notificationwidgetparameterfactory.h" #include "notificationmanagerinterface.h" @@ -77,6 +78,7 @@ void NotificationAreaSink::updateNotification(MBanner *infoBanner, const Notific infoBanner->setProperty(SUBTITLE_TEXT_PROPERTY, infoBannerTitleText(parameters)); infoBanner->setProperty(GENERIC_TEXT_PROPERTY, infoBannerGenericText(parameters)); infoBanner->setProperty(USER_REMOVABLE_PROPERTY, determineUserRemovability(parameters)); + infoBanner->setBannerTimeStamp(QDateTime::fromTime_t(parameters.value("timestamp").toUInt())); updatePrefixForNotificationGroupBannerTimestamp(infoBanner, parameters.value("count").toUInt()); @@ -179,7 +181,6 @@ void NotificationAreaSink::addNotificationToGroup(const Notification ¬ificati // Seems like the infoBanner is NULL. So it means that the group banner was removed, but group is alive. Recreate the banner. infoBanner = createGroupBanner(groupId, notificationGroupParameters.value(groupId)); } - infoBanner->setBannerTimeStamp(QDateTime::fromTime_t(notification.parameters().value("timestamp").toUInt())); if (infoBanner != NULL && infoBanner->parentItem() == NULL) { // Add the group to the notification area if this is the first notification to the group @@ -198,7 +199,6 @@ void NotificationAreaSink::addStandAloneNotification(const Notification ¬ific if (infoBanner != NULL) { // If the notification is already in the map, only update it updateNotification(infoBanner, notification.parameters()); - infoBanner->setBannerTimeStamp(QDateTime::fromTime_t(notification.parameters().value("timestamp").toUInt())); } else { infoBanner = createInfoBanner(notification); setupInfoBanner(infoBanner, notification.parameters()); diff --git a/src/systemui/notifications/notificationmanager.cpp b/src/systemui/notifications/notificationmanager.cpp index d559cfa2..55d104a1 100644 --- a/src/systemui/notifications/notificationmanager.cpp +++ b/src/systemui/notifications/notificationmanager.cpp @@ -285,6 +285,8 @@ uint NotificationManager::addNotification(uint notificationUserId, const Notific submitNotification(notification); + updateGroupTimestampFromNotifications(groupId); + return notificationId; } return 0; @@ -312,6 +314,8 @@ bool NotificationManager::updateNotification(uint notificationUserId, uint notif emit notificationUpdated(notificationContainer.value(notificationId)); } + updateGroupTimestampFromNotifications((*ni).groupId()); + return true; } else { return false; @@ -333,7 +337,7 @@ bool NotificationManager::removeNotification(uint notificationId) { if (notificationContainer.contains(notificationId)) { // Mark the notification unused - notificationContainer.take(notificationId); + const Notification removedNotification = notificationContainer.take(notificationId); saveNotifications(); @@ -353,6 +357,8 @@ bool NotificationManager::removeNotification(uint notificationId) } } + updateGroupTimestampFromNotifications(removedNotification.groupId()); + return true; } else { return false; @@ -373,13 +379,13 @@ bool NotificationManager::removeNotificationsInGroup(uint groupId) foreach(uint notificationId, notificationIds) { result &= removeNotification(notificationId); } + return result; } uint NotificationManager::addGroup(uint notificationUserId, const NotificationParameters ¶meters) { NotificationParameters fullParameters(appendEventTypeParameters(parameters)); - fullParameters.add(GenericNotificationParameterFactory::timestampKey(), timestamp(parameters)); uint groupID = nextAvailableGroupID(); NotificationGroup group(groupID, notificationUserId, fullParameters); @@ -399,9 +405,7 @@ bool NotificationManager::updateGroup(uint notificationUserId, uint groupId, con QHash::iterator gi = groupContainer.find(groupId); if (gi != groupContainer.end()) { - NotificationParameters fullParameters(parameters); - fullParameters.add(GenericNotificationParameterFactory::timestampKey(), timestamp(parameters)); - gi->updateParameters(fullParameters); + gi->updateParameters(parameters); saveStateData(); @@ -673,3 +677,30 @@ uint NotificationManager::timestamp(const NotificationParameters ¶meters) return timestamp == 0 ? QDateTime::currentDateTimeUtc().toTime_t() : timestamp; } +void NotificationManager::updateGroupTimestampFromNotifications(uint groupId) +{ + if (groupId != 0 && groupContainer.contains(groupId)) { + NotificationGroup group = groupContainer.value(groupId); + NotificationParameters groupParameters = group.parameters(); + uint oldGroupTimestamp = groupParameters.value(GenericNotificationParameterFactory::timestampKey()).toUInt(); + + // Check the latest notification timestamp of the group's notifications + uint newGroupTimestamp = 0; + QHash::const_iterator notificationIterator; + for (notificationIterator = notificationContainer.begin(); notificationIterator != notificationContainer.end(); notificationIterator++) { + if ((*notificationIterator).groupId() == groupId) { + uint notificationTimestamp = (*notificationIterator).parameters().value(GenericNotificationParameterFactory::timestampKey()).toUInt(); + if (newGroupTimestamp < notificationTimestamp) { + newGroupTimestamp = notificationTimestamp; + } + } + } + + if (oldGroupTimestamp != newGroupTimestamp) { + // Update the group timestamp + groupParameters.add(GenericNotificationParameterFactory::timestampKey(), newGroupTimestamp); + group.updateParameters(groupParameters); + updateGroup(group.userId(), group.groupId(), group.parameters()); + } + } +} diff --git a/src/systemui/notifications/notificationmanager.h b/src/systemui/notifications/notificationmanager.h index 3c5e6309..a3870682 100644 --- a/src/systemui/notifications/notificationmanager.h +++ b/src/systemui/notifications/notificationmanager.h @@ -305,6 +305,12 @@ private slots: */ uint timestamp(const NotificationParameters ¶meters); + /*! Updates the given group's timestamp according to the latest timestamp of group's notifications. + * + * \param groupId The id of the group of which timestamp should be updated + */ + void updateGroupTimestampFromNotifications(uint groupId); + //! Hash of all notifications keyed by notification IDs QHash notificationContainer; diff --git a/tests/ut_notificationareasink/ut_notificationareasink.cpp b/tests/ut_notificationareasink/ut_notificationareasink.cpp index 6983b0b0..0e3cfd68 100644 --- a/tests/ut_notificationareasink/ut_notificationareasink.cpp +++ b/tests/ut_notificationareasink/ut_notificationareasink.cpp @@ -340,9 +340,10 @@ void Ut_NotificationAreaSink::testAddNotificationToGroup() TestNotificationParameters parameters1("title1", "subtitle1", "icon1", "content1", 12345); emit addNotification(Notification(0, 1, 2, parameters1, Notification::ApplicationEvent, 1000)); - QCOMPARE(timestamps[1].toTime_t(), (uint)12345); QCOMPARE(addSpy.count(), 1); QCOMPARE(notifications.count(), 1); + // Group timestamp shouldn't change in sink when notification added (that is handled in notification manager) + QCOMPARE(timestamps.count(), 1); } void Ut_NotificationAreaSink::testAddNewNotificationToGroupUpdatesNotificationArea() @@ -370,7 +371,7 @@ void Ut_NotificationAreaSink::testWhenAddingNewNotificationToGroupThatHasBeenPre const QString NOTIFICATION_ICON("notificationIcon"); const QString NOTIFICATION_ACTION("notificationAction"); - TestNotificationParameters groupParameters(GROUP_SUMMARY, GROUP_BODY, GROUP_ICON, GROUP_ACTION); + TestNotificationParameters groupParameters(GROUP_SUMMARY, GROUP_BODY, GROUP_ICON, GROUP_ACTION, 123); emit addGroup(GROUP_ID, groupParameters); TestNotificationParameters notificationParameters(NOTIFICATION_SUMMARY, NOTIFICATION_BODY, NOTIFICATION_ICON, NOTIFICATION_ACTION, 12345); emit addNotification(Notification(NOTIFICATION_ID, GROUP_ID, 2, notificationParameters, Notification::ApplicationEvent, 1000)); @@ -386,8 +387,8 @@ void Ut_NotificationAreaSink::testWhenAddingNewNotificationToGroupThatHasBeenPre MBanner *banner = bannerCatcher.banners.at(0); // The banner should have the notification group's data QCOMPARE(banner->title(), GROUP_BODY); - // The banner should have the timestamp of the previous notification - QCOMPARE(timestamps[0].toTime_t(), (uint)12345); + // The banner should have the timestamp of the group + QCOMPARE(timestamps[0].toTime_t(), (uint)123); } void Ut_NotificationAreaSink::testUpdateGroup() @@ -410,14 +411,14 @@ void Ut_NotificationAreaSink::testUpdateGroup() emit addGroup(1, parameters1); - // The stub is now aware of the banner so updates go to the first occurrence of the banner. Timestamp should not get set. + // The stub is now aware of the banner so updates go to the first occurrence of the banner. Timestamp should get updated. QCOMPARE(titles.length(), 2); QCOMPARE(titles[0], QString("subtitle1")); QCOMPARE(subtitles.length(), 2); QCOMPARE(subtitles[0], QString("title1")); QCOMPARE(buttonIcons.length(), 1); QCOMPARE(buttonIcons[0], QString("buttonicon1")); - QCOMPARE(timestamps[0].toTime_t(), (uint)12345); + QCOMPARE(timestamps[0].toTime_t(), (uint)123456); // TODO: even though contents.length is 2, there's only 1 action in the mnotification // clearing of the actions should be stubbed somehow... QCOMPARE(contents.length(), 2); diff --git a/tests/ut_notificationmanager/ut_notificationmanager.cpp b/tests/ut_notificationmanager/ut_notificationmanager.cpp index d9b57e67..01bbe628 100644 --- a/tests/ut_notificationmanager/ut_notificationmanager.cpp +++ b/tests/ut_notificationmanager/ut_notificationmanager.cpp @@ -1467,7 +1467,7 @@ void Ut_NotificationManager::testGroupInfoRestoration() QCOMPARE(parameters.value(ICON).toString(), QString("buttonicon2")); } -void Ut_NotificationManager::tesNotificationStorage() +void Ut_NotificationManager::testNotificationStorage() { gEventTypeSettings["persistent"][PERSISTENT] = "true"; @@ -1851,7 +1851,7 @@ void Ut_NotificationManager::testThatTimestampOfNotificationIsUpdatedWhenNotific QCOMPARE(notification.notificationId(), notificationId); } -void Ut_NotificationManager::testAddGroupWithAndWithoutTimestamp() +void Ut_NotificationManager::testAddGroupWithoutAndWithTimestamp() { QSignalSpy spy(manager, SIGNAL(groupUpdated(uint, const NotificationParameters&))); @@ -1860,44 +1860,224 @@ void Ut_NotificationManager::testAddGroupWithAndWithoutTimestamp() qDateTimeToTime_t = 123; NotificationParameters params; params.add(TIMESTAMP, timestamp); - manager->addGroup(userId, params); manager->addGroup(userId, NotificationParameters()); + manager->addGroup(userId, params); QCOMPARE(spy.count(), 2); - // First notification has timestamp in NotificationParameters + // First notification has no timestamp in NotificationParameters, + // so no timestamp is set for the group QList arguments = spy.takeFirst(); NotificationParameters notificationParams = qvariant_cast(arguments.at(1)); - QCOMPARE(notificationParams.value(TIMESTAMP).toUInt(), timestamp); + QCOMPARE(notificationParams.value(TIMESTAMP).toUInt(), (uint)0); - // Second notification has a zero timestamp in NotificationParameters, - // so current time is used as a timestamp + // Second notification has a timestamp in NotificationParameters arguments = spy.takeFirst(); notificationParams = qvariant_cast(arguments.at(1)); - QCOMPARE(notificationParams.value(TIMESTAMP).toUInt(), qDateTimeToTime_t); + QCOMPARE(notificationParams.value(TIMESTAMP).toUInt(), timestamp); } void Ut_NotificationManager::testThatTimestampOfGroupIsUpdatedWhenGroupIsUpdated() { - QSignalSpy spy(manager, SIGNAL(groupUpdated(uint, const NotificationParameters&))); - uint timestamp = 12345678; qDateTimeToTime_t = 123; uint groupId = manager->addGroup(0, NotificationParameters()); + QSignalSpy spy(manager, SIGNAL(groupUpdated(uint, const NotificationParameters&))); + + manager->updateGroup(0, groupId, NotificationParameters()); + NotificationParameters params; params.add(TIMESTAMP, timestamp); manager->updateGroup(0, groupId, params); QCOMPARE(spy.count(), 2); + // First update has no timestamp in NotificationParameters, + // so no timestamp is updated for the group QList arguments = spy.takeFirst(); + QCOMPARE(qvariant_cast(arguments.at(1)).value(TIMESTAMP).toUInt(), (uint)0); QCOMPARE(arguments.at(0).toUInt(), groupId); + // Second update has a timestamp in NotificationParameters arguments = spy.takeFirst(); QCOMPARE(qvariant_cast(arguments.at(1)).value(TIMESTAMP).toUInt(), timestamp); QCOMPARE(arguments.at(0).toUInt(), groupId); } +void Ut_NotificationManager::testAddNotificationToGroupWithTimestamp() +{ + uint timestamp = 12345678; + + uint groupId = manager->addGroup(0, NotificationParameters()); + + QSignalSpy groupSpy(manager, SIGNAL(groupUpdated(uint, const NotificationParameters &))); + + NotificationParameters parameters0; + parameters0.add(TIMESTAMP, timestamp); + manager->addNotification(0, parameters0, groupId); + + QCOMPARE(groupSpy.count(), 1); + QList arguments = groupSpy.takeFirst(); + QCOMPARE(qvariant_cast(arguments.at(1)).value(TIMESTAMP).toUInt(), timestamp); + QCOMPARE(arguments.at(0).toUInt(), groupId); +} + +void Ut_NotificationManager::testUpdateGroupTimestampWhenNewerNotificationAddedToGroup() +{ + uint groupId = manager->addGroup(0, NotificationParameters()); + + NotificationParameters parameters0; + uint olderTimestamp = 123; + parameters0.add(TIMESTAMP, olderTimestamp); + manager->addNotification(0, parameters0, groupId); + + QSignalSpy groupSpy(manager, SIGNAL(groupUpdated(uint, const NotificationParameters &))); + + NotificationParameters parameters1; + uint newerTimestamp = 123456; + parameters1.add(TIMESTAMP, newerTimestamp); + manager->addNotification(0, parameters1, groupId); + + QCOMPARE(groupSpy.count(), 1); + QList arguments = groupSpy.takeFirst(); + QCOMPARE(arguments.at(0).toUInt(), groupId); + QCOMPARE(qvariant_cast(arguments.at(1)).value(TIMESTAMP).toUInt(), newerTimestamp); +} + +void Ut_NotificationManager::testNotUpdatingGroupTimestampWhenAddingNotificationsWithOlderTimestamp() +{ + uint groupId = manager->addGroup(0, NotificationParameters()); + + QSignalSpy groupSpy(manager, SIGNAL(groupUpdated(uint, const NotificationParameters &))); + + NotificationParameters parameters0; + const uint mostLatestNotificationTimestamp = 123456; + parameters0.add(TIMESTAMP, mostLatestNotificationTimestamp); + manager->addNotification(0, parameters0, groupId); + + // Add a notification with an older timestamp + NotificationParameters parameters1; + const uint olderTimestamp = 123; + parameters1.add(TIMESTAMP, olderTimestamp); + manager->addNotification(0, parameters1, groupId); + + QCOMPARE(groupSpy.count(), 1); + QList arguments = groupSpy.takeFirst(); + QCOMPARE(arguments.at(0).toUInt(), groupId); + // Verify that the latest timestamp is still the only one updated to the group + QCOMPARE(qvariant_cast(arguments.at(1)).value(TIMESTAMP).toUInt(), mostLatestNotificationTimestamp); +} + +void Ut_NotificationManager::testNotUpdatingGroupTimestampWhenUpdatingNotificationsWithOlderTimestamp() +{ + uint groupId = manager->addGroup(0, NotificationParameters()); + + QSignalSpy groupSpy(manager, SIGNAL(groupUpdated(uint, const NotificationParameters &))); + + NotificationParameters parameters0; + const uint mostLatestNotificationTimestamp = 123456; + parameters0.add(TIMESTAMP, mostLatestNotificationTimestamp); + manager->addNotification(0, parameters0, groupId); + + uint updatedNotificationId = manager->addNotification(0, NotificationParameters(), groupId); + + // Update notification with an older timestamp + NotificationParameters parameters1; + const uint olderTimestamp = 123; + parameters1.add(TIMESTAMP, olderTimestamp); + manager->updateNotification(0, updatedNotificationId, parameters1); + + QCOMPARE(groupSpy.count(), 1); + QList arguments = groupSpy.takeFirst(); + QCOMPARE(arguments.at(0).toUInt(), groupId); + // Verify that the latest timestamp is still the only one updated to the group + QCOMPARE(qvariant_cast(arguments.at(1)).value(TIMESTAMP).toUInt(), mostLatestNotificationTimestamp); +} + +void Ut_NotificationManager::testUpdateNotificationInGroupWithTimestamp_data() +{ + QTest::addColumn("oldTimestamp"); + QTest::addColumn("updatedTimestamp"); + QTest::addColumn("expectedTimestamp"); + + const uint olderTimestamp = 123; + const uint laterTimestamp = 123456; + + QTest::newRow("Timestamp updated to newer value") << olderTimestamp << laterTimestamp << laterTimestamp; + QTest::newRow("Timestamp updated to older value") << laterTimestamp << olderTimestamp << olderTimestamp; +} + +void Ut_NotificationManager::testUpdateNotificationInGroupWithTimestamp() +{ + QFETCH(uint, oldTimestamp); + QFETCH(uint, updatedTimestamp); + QFETCH(uint, expectedTimestamp); + + uint groupId = manager->addGroup(0, NotificationParameters()); + + NotificationParameters parameters0; + parameters0.add(TIMESTAMP, oldTimestamp); + uint notificationId = manager->addNotification(0, parameters0, groupId); + + QSignalSpy groupSpy(manager, SIGNAL(groupUpdated(uint, const NotificationParameters &))); + + parameters0.add(TIMESTAMP, updatedTimestamp); + manager->updateNotification(0, notificationId, parameters0); + + QCOMPARE(groupSpy.count(), 1); + QList arguments = groupSpy.takeFirst(); + QCOMPARE(arguments.at(0).toUInt(), groupId); + QCOMPARE(qvariant_cast(arguments.at(1)).value(TIMESTAMP).toUInt(), expectedTimestamp); +} + +void Ut_NotificationManager::testUpdatingGroupTimestampWhenRemovingNotifications() +{ + uint groupId = manager->addGroup(0, NotificationParameters()); + + NotificationParameters parameters0; + uint timestampNotification0 = 123; + parameters0.add(TIMESTAMP, timestampNotification0); + manager->addNotification(0, parameters0, groupId); + + NotificationParameters parameters1; + uint timestampNotification1 = 123456; + parameters1.add(TIMESTAMP, timestampNotification1); + uint notificationId1 = manager->addNotification(0, parameters1, groupId); + + QSignalSpy groupSpy(manager, SIGNAL(groupUpdated(uint, const NotificationParameters &))); + + manager->removeNotification(notificationId1); + + QCOMPARE(groupSpy.count(), 1); + QList arguments = groupSpy.takeFirst(); + QCOMPARE(arguments.at(0).toUInt(), groupId); + QCOMPARE(qvariant_cast(arguments.at(1)).value(TIMESTAMP).toUInt(), timestampNotification0); +} + +void Ut_NotificationManager:: testUpdatingGroupTimestampWhenGroupIsCleared() +{ + uint groupId = manager->addGroup(0, NotificationParameters()); + + NotificationParameters parameters0; + uint timestampNotification0 = 123; + parameters0.add(TIMESTAMP, timestampNotification0); + manager->addNotification(0, parameters0, groupId); + + NotificationParameters parameters1; + uint timestampNotification1 = 123456; + parameters1.add(TIMESTAMP, timestampNotification1); + manager->addNotification(0, parameters1, groupId); + + QSignalSpy groupSpy(manager, SIGNAL(groupUpdated(uint, const NotificationParameters &))); + + manager->removeNotificationsInGroup(groupId); + + QCOMPARE(groupSpy.count(), 1); + QList arguments = groupSpy.takeLast(); + QCOMPARE(qvariant_cast(arguments.at(1)).value(TIMESTAMP).toUInt(), (uint)0); + QCOMPARE(arguments.at(0).toUInt(), groupId); +} + QTEST_MAIN(Ut_NotificationManager) diff --git a/tests/ut_notificationmanager/ut_notificationmanager.h b/tests/ut_notificationmanager/ut_notificationmanager.h index e3173bc8..cded2052 100644 --- a/tests/ut_notificationmanager/ut_notificationmanager.h +++ b/tests/ut_notificationmanager/ut_notificationmanager.h @@ -133,7 +133,7 @@ private slots: // Test that the group info is restored from the persistent storage void testGroupInfoRestoration(); // Test that the persistent notifications are saved to the persistent storage - void tesNotificationStorage(); + void testNotificationStorage(); // Test that the persistent notifications are restored from the persistent storage void testNotificationRestoration(); // Test the removal of notifications based on event type @@ -162,9 +162,17 @@ private slots: // Test that timestamp of notification is updated void testThatTimestampOfNotificationIsUpdatedWhenNotificationUpdates(); // Test adding group with and without user specified timestamp - void testAddGroupWithAndWithoutTimestamp(); + void testAddGroupWithoutAndWithTimestamp(); // Test that timestamp of group is updated when group is updated void testThatTimestampOfGroupIsUpdatedWhenGroupIsUpdated(); + void testAddNotificationToGroupWithTimestamp(); + void testUpdateGroupTimestampWhenNewerNotificationAddedToGroup(); + void testNotUpdatingGroupTimestampWhenAddingNotificationsWithOlderTimestamp(); + void testNotUpdatingGroupTimestampWhenUpdatingNotificationsWithOlderTimestamp(); + void testUpdateNotificationInGroupWithTimestamp_data(); + void testUpdateNotificationInGroupWithTimestamp(); + void testUpdatingGroupTimestampWhenRemovingNotifications(); + void testUpdatingGroupTimestampWhenGroupIsCleared(); }; #endif // UT_NOTIFICATIONMANAGER_H