Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Remove NotificationPreviewPresenter::notificationPresented()
Noone was interested in the signal except unit test and internal
connection to feedback player. Intention of code emission is clearer
if the feedback is made explicitly, and following that in unit test
assumes slightly less of the internal implementation.
  • Loading branch information
pvuorela committed Jan 17, 2018
1 parent 7629987 commit d60d9e8
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 53 deletions.
11 changes: 4 additions & 7 deletions src/notifications/notificationpreviewpresenter.cpp
Expand Up @@ -69,8 +69,6 @@ NotificationPreviewPresenter::NotificationPreviewPresenter(
this, [=](uint id) {
removeNotification(id);
});
connect(this, &NotificationPreviewPresenter::notificationPresented,
m_notificationFeedbackPlayer, &NotificationFeedbackPlayer::addNotification);
QTimer::singleShot(0, this, SLOT(createWindowIfNecessary()));
}

Expand Down Expand Up @@ -115,8 +113,7 @@ void NotificationPreviewPresenter::showNextNotification()

if (!show) {
if (m_deviceLock->state() != NemoDeviceLock::DeviceLock::ManagerLockout) { // Suppress feedback if locked out.
// Don't show the notification but just remove it from the queue
emit notificationPresented(notification->id());
m_notificationFeedbackPlayer->addNotification(notification->id());
}

setCurrentNotification(0);
Expand All @@ -128,7 +125,7 @@ void NotificationPreviewPresenter::showNextNotification()
m_window->show();
}

emit notificationPresented(notification->id());
m_notificationFeedbackPlayer->addNotification(notification->id());

setCurrentNotification(notification);
}
Expand Down Expand Up @@ -156,9 +153,9 @@ void NotificationPreviewPresenter::updateNotification(uint id)
}
}
} else {
// Remove updated notification only from the queue so that a currently visible notification won't suddenly disappear
emit notificationPresented(id);
m_notificationFeedbackPlayer->addNotification(id);

// Remove updated notification only from the queue so that a currently visible notification won't suddenly disappear
removeNotification(id, true);

if (m_currentNotification != notification) {
Expand Down
16 changes: 2 additions & 14 deletions src/notifications/notificationpreviewpresenter.h
Expand Up @@ -42,17 +42,8 @@ class LIPSTICK_EXPORT NotificationPreviewPresenter : public QObject
Q_PROPERTY(LipstickNotification *notification READ notification NOTIFY notificationChanged)

public:
/*!
* Creates a notification preview presenter.
*
* \param parent the parent object
*/
explicit NotificationPreviewPresenter(
ScreenLock *screenLock, NemoDeviceLock::DeviceLock *deviceLock, QObject *parent = 0);

/*!
* Destroys the notification preview presenter.
*/
explicit NotificationPreviewPresenter(ScreenLock *screenLock, NemoDeviceLock::DeviceLock *deviceLock,
QObject *parent = 0);
virtual ~NotificationPreviewPresenter();

/*!
Expand All @@ -67,9 +58,6 @@ class LIPSTICK_EXPORT NotificationPreviewPresenter : public QObject
//! Sent when the notification to be shown has changed.
void notificationChanged();

//! Sent when a notification is considered presented by the presenter
void notificationPresented(uint id);

public slots:
/*!
* Shows the next notification to be shown, if any. If the notification
Expand Down
Expand Up @@ -166,6 +166,16 @@ void NotificationManager::reportModifications()
{
}

static int playedFeedbacks()
{
return gNotificationFeedbackPlayerStub->stubCallsTo("addNotification").count();
}

static uint lastFeedbackId()
{
return gNotificationFeedbackPlayerStub->stubLastCallTo("addNotification").parameter<uint>(0);
}

enum Urgency { Low = 0, Normal = 1, Critical = 2 };

LipstickNotification *createNotification(uint id, Urgency urgency = Normal)
Expand Down Expand Up @@ -197,6 +207,7 @@ void Ut_NotificationPreviewPresenter::init()
touchScreen = new TouchScreen;
screenLock = new ScreenLock(touchScreen);
deviceLock = new NemoDeviceLock::DeviceLock;
gNotificationFeedbackPlayerStub->stubReset();
}

void Ut_NotificationPreviewPresenter::cleanup()
Expand All @@ -218,7 +229,6 @@ void Ut_NotificationPreviewPresenter::testAddNotificationWhenWindowNotOpen()
{
NotificationPreviewPresenter presenter(screenLock, deviceLock);
QSignalSpy changedSpy(&presenter, SIGNAL(notificationChanged()));
QSignalSpy presentedSpy(&presenter, SIGNAL(notificationPresented(uint)));

// Check that the window is not automatically created
QCOMPARE(homeWindows.isEmpty(), true);
Expand All @@ -244,15 +254,14 @@ void Ut_NotificationPreviewPresenter::testAddNotificationWhenWindowNotOpen()
// Check that the expected notification is signaled onwards
QCOMPARE(changedSpy.count(), 1);
QCOMPARE(presenter.notification(), notification);
QCOMPARE(presentedSpy.count(), 1);
QCOMPARE(presentedSpy.last().at(0).toUInt(), (uint)1);
QCOMPARE(playedFeedbacks(), 1);
QCOMPARE(lastFeedbackId(), (uint)1);
}

void Ut_NotificationPreviewPresenter::testAddNotificationWhenWindowAlreadyOpen()
{
NotificationPreviewPresenter presenter(screenLock, deviceLock);
QSignalSpy changedSpy(&presenter, SIGNAL(notificationChanged()));
QSignalSpy presentedSpy(&presenter, SIGNAL(notificationPresented(uint)));

// Create a notification: this will create a window
createNotification(1);
Expand All @@ -267,8 +276,8 @@ void Ut_NotificationPreviewPresenter::testAddNotificationWhenWindowAlreadyOpen()

// The second notification should not be signaled onwards yet since the first one is being presented
QCOMPARE(changedSpy.count(), 1);
QCOMPARE(presentedSpy.count(), 1);
QCOMPARE(presentedSpy.last().at(0).toUInt(), (uint)1);
QCOMPARE(playedFeedbacks(), 1);
QCOMPARE(lastFeedbackId(), (uint)1);

// Show the next notification
presenter.showNextNotification();
Expand All @@ -279,8 +288,8 @@ void Ut_NotificationPreviewPresenter::testAddNotificationWhenWindowAlreadyOpen()
// Check that the expected notification is signaled onwards
QCOMPARE(changedSpy.count(), 2);
QCOMPARE(presenter.notification(), notification);
QCOMPARE(presentedSpy.count(), 2);
QCOMPARE(presentedSpy.last().at(0).toUInt(), (uint)2);
QCOMPARE(playedFeedbacks(), 2);
QCOMPARE(lastFeedbackId(), (uint)2);
}

void Ut_NotificationPreviewPresenter::testUpdateNotification()
Expand All @@ -295,13 +304,13 @@ void Ut_NotificationPreviewPresenter::testUpdateNotification()

// Update both notifications
QSignalSpy changedSpy(&presenter, SIGNAL(notificationChanged()));
QSignalSpy presentedSpy(&presenter, SIGNAL(notificationPresented(uint)));
gNotificationFeedbackPlayerStub->stubReset();
presenter.updateNotification(1);
presenter.updateNotification(2);

// Check that no signals were sent
QCOMPARE(changedSpy.count(), 0);
QCOMPARE(presentedSpy.count(), 0);
QCOMPARE(playedFeedbacks(), 0);
}

void Ut_NotificationPreviewPresenter::testRemoveNotification()
Expand Down Expand Up @@ -344,7 +353,7 @@ void Ut_NotificationPreviewPresenter::testNotificationNotShownIfNoSummaryOrBody_
QTest::addColumn<QString>("previewSummary");
QTest::addColumn<QString>("previewBody");
QTest::addColumn<int>("changedSignalCount");
QTest::addColumn<int>("presentedSignalCount");
QTest::addColumn<int>("playedFeedbackCount");
QTest::addColumn<bool>("windowVisible");
QTest::newRow("No summary, no body") << "" << "" << 0 << 1 << false;
QTest::newRow("Summary, no body") << "summary" << "" << 1 << 1 << true;
Expand All @@ -357,12 +366,11 @@ void Ut_NotificationPreviewPresenter::testNotificationNotShownIfNoSummaryOrBody(
QFETCH(QString, previewSummary);
QFETCH(QString, previewBody);
QFETCH(int, changedSignalCount);
QFETCH(int, presentedSignalCount);
QFETCH(int, playedFeedbackCount);
QFETCH(bool, windowVisible);

NotificationPreviewPresenter presenter(screenLock, deviceLock);
QSignalSpy changedSpy(&presenter, SIGNAL(notificationChanged()));
QSignalSpy presentedSpy(&presenter, SIGNAL(notificationPresented(uint)));

// Create notification
QVariantHash hints;
Expand All @@ -375,7 +383,7 @@ void Ut_NotificationPreviewPresenter::testNotificationNotShownIfNoSummaryOrBody(

// Check whether the expected notification is signaled onwards
QCOMPARE(changedSpy.count(), changedSignalCount);
QCOMPARE(presentedSpy.count(), presentedSignalCount);
QCOMPARE(playedFeedbacks(), playedFeedbackCount);

QCOMPARE(homeWindowVisible.isEmpty(), !windowVisible);
if (windowVisible) {
Expand All @@ -388,7 +396,6 @@ void Ut_NotificationPreviewPresenter::testNotificationNotShownIfHidden()
{
NotificationPreviewPresenter presenter(screenLock, deviceLock);
QSignalSpy changedSpy(&presenter, SIGNAL(notificationChanged()));
QSignalSpy presentedSpy(&presenter, SIGNAL(notificationPresented(uint)));

// Create notification
QVariantHash hints;
Expand All @@ -403,15 +410,14 @@ void Ut_NotificationPreviewPresenter::testNotificationNotShownIfHidden()
QCOMPARE(homeWindowVisible.isEmpty(), true);

// The notification should be considered presented
QCOMPARE(presentedSpy.count(), 1);
QCOMPARE(presentedSpy.last().at(0).toUInt(), (uint)1);
QCOMPARE(playedFeedbacks(), 1);
QCOMPARE(lastFeedbackId(), (uint)1);
}

void Ut_NotificationPreviewPresenter::testNotificationNotShownIfRestored()
{
NotificationPreviewPresenter presenter(screenLock, deviceLock);
QSignalSpy changedSpy(&presenter, SIGNAL(notificationChanged()));
QSignalSpy presentedSpy(&presenter, SIGNAL(notificationPresented(uint)));

// Create notification
QVariantHash hints;
Expand All @@ -426,15 +432,14 @@ void Ut_NotificationPreviewPresenter::testNotificationNotShownIfRestored()
QCOMPARE(homeWindowVisible.isEmpty(), true);

// The notification should be considered presented
QCOMPARE(presentedSpy.count(), 1);
QCOMPARE(presentedSpy.last().at(0).toUInt(), (uint)1);
QCOMPARE(playedFeedbacks(), 1);
QCOMPARE(lastFeedbackId(), (uint)1);
}

void Ut_NotificationPreviewPresenter::testShowingOnlyCriticalNotifications()
{
NotificationPreviewPresenter presenter(screenLock, deviceLock);
QSignalSpy changedSpy(&presenter, SIGNAL(notificationChanged()));
QSignalSpy presentedSpy(&presenter, SIGNAL(notificationPresented(uint)));

// Create normal urgency notification
QVariantHash hints;
Expand All @@ -453,8 +458,8 @@ void Ut_NotificationPreviewPresenter::testShowingOnlyCriticalNotifications()
QCOMPARE(homeWindowVisible.isEmpty(), true);

// The notification should be considered presented
QCOMPARE(presentedSpy.count(), 1);
QCOMPARE(presentedSpy.last().at(0).toUInt(), (uint)1);
QCOMPARE(playedFeedbacks(), 1);
QCOMPARE(lastFeedbackId(), (uint)1);

// Urgency set to critical, so the notification should be shown
hints.insert(LipstickNotification::HINT_URGENCY, 2);
Expand All @@ -463,8 +468,8 @@ void Ut_NotificationPreviewPresenter::testShowingOnlyCriticalNotifications()
QCOMPARE(changedSpy.count(), 1);
QCOMPARE(homeWindowVisible.isEmpty(), false);
QCOMPARE(homeWindowVisible[homeWindows.first()], true);
QCOMPARE(presentedSpy.count(), 2);
QCOMPARE(presentedSpy.last().at(0).toUInt(), (uint)1);
QCOMPARE(playedFeedbacks(), 2);
QCOMPARE(lastFeedbackId(), (uint)1);
}

void Ut_NotificationPreviewPresenter::testUpdateNotificationRemovesNotificationFromQueueIfNotShowable()
Expand All @@ -480,7 +485,7 @@ void Ut_NotificationPreviewPresenter::testUpdateNotificationRemovesNotificationF

// Update the notifications to have no summary or body
QSignalSpy changedSpy(&presenter, SIGNAL(notificationChanged()));
QSignalSpy presentedSpy(&presenter, SIGNAL(notificationPresented(uint)));
gNotificationFeedbackPlayerStub->stubReset();
notification1->setHints(QVariantHash());
notification2->setHints(QVariantHash());
presenter.updateNotification(1);
Expand All @@ -490,13 +495,13 @@ void Ut_NotificationPreviewPresenter::testUpdateNotificationRemovesNotificationF
QCOMPARE(changedSpy.count(), 0);

// The notifications should be considered presented
QCOMPARE(presentedSpy.count(), 2);
QCOMPARE(playedFeedbacks(), 2);

// Check that the other notification is removed from the queue
presenter.showNextNotification();
QCOMPARE(changedSpy.count(), 1);
QCOMPARE(presenter.notification(), (LipstickNotification *)0);
QCOMPARE(presentedSpy.count(), 2);
QCOMPARE(playedFeedbacks(), 2);
}

Q_DECLARE_METATYPE(MeeGo::QmDisplayState::DisplayState)
Expand All @@ -507,7 +512,7 @@ void Ut_NotificationPreviewPresenter::testNotificationNotShownIfTouchScreenIsLoc
QTest::addColumn<NemoDeviceLock::DeviceLock::LockState>("lockState");
QTest::addColumn<int>("urgency");
QTest::addColumn<int>("notifications");
QTest::addColumn<int>("presentedCount");
QTest::addColumn<int>("playedFeedbackCount");
QTest::newRow("Display on, touch screen not locked") << MeeGo::QmDisplayState::On << NemoDeviceLock::DeviceLock::Unlocked << static_cast<int>(Normal) << 1 << 1;
QTest::newRow("Display on, touch screen locked") << MeeGo::QmDisplayState::On << NemoDeviceLock::DeviceLock::Locked << static_cast<int>(Normal) << 0 << 1;
QTest::newRow("Display off, touch screen not locked") << MeeGo::QmDisplayState::Off << NemoDeviceLock::DeviceLock::Unlocked << static_cast<int>(Normal) << 1 << 1;
Expand All @@ -524,21 +529,20 @@ void Ut_NotificationPreviewPresenter::testNotificationNotShownIfTouchScreenIsLoc
QFETCH(NemoDeviceLock::DeviceLock::LockState, lockState);
QFETCH(int, urgency);
QFETCH(int, notifications);
QFETCH(int, presentedCount);
QFETCH(int, playedFeedbackCount);

gQmDisplayStateStub->stubSetReturnValue("get", displayState);
deviceLock->setState(lockState);

NotificationPreviewPresenter presenter(screenLock, deviceLock);
QSignalSpy changedSpy(&presenter, SIGNAL(notificationChanged()));
QSignalSpy presentedSpy(&presenter, SIGNAL(notificationPresented(uint)));

createNotification(1, static_cast<Urgency>(urgency));
QTest::qWait(0);
presenter.updateNotification(1);
QCOMPARE(homeWindowVisible.count(), notifications);
QCOMPARE(changedSpy.count(), notifications);
QCOMPARE(presentedSpy.count(), presentedCount);
QCOMPARE(playedFeedbacks(), playedFeedbackCount);
}

void Ut_NotificationPreviewPresenter::testCriticalNotificationIsMarkedAfterShowing()
Expand Down

0 comments on commit d60d9e8

Please sign in to comment.