[Merge] lp:~boiko/telephony-service/remove_messages_from_menu into lp:telephony-service

[Tiago Salem Herrmann]

Review: Needs Fixing

Just one small remark and one question.

Diff comments:

> 
> === modified file 'libtelephonyservice/chatmanager.cpp'
> --- libtelephonyservice/chatmanager.cpp	2015-03-10 21:41:45 +0000
> +++ libtelephonyservice/chatmanager.cpp	2015-09-03 22:20:54 +0000
> @@ -204,6 +204,12 @@
>      mMessagesToAck[account->accountId()][recipients].append(messageId);
>  }
>  
> +void ChatManager::acknowledgeAllMessages(const QStringList &recipients, const QString &accountId)
> +{
> +    QDBusInterface *phoneAppHandler = TelepathyHelper::instance()->handlerInterface();
> +    phoneAppHandler->call("AcknowledgeAllMessages", recipients, accountId);

Should we make this call async just to avoid clients getting stuck waiting on the dbus call to finish?
I see the method is already marked as Q_NOREPLY in the server side, just not sure if there is any improvement on making it async on the client side too.

> +}
> +
>  void ChatManager::onAckTimerTriggered()
>  {
>      // ack all pending messages
> 
> === modified file 'tests/handler/HandlerTest.cpp'
> --- tests/handler/HandlerTest.cpp	2015-08-18 13:19:43 +0000
> +++ tests/handler/HandlerTest.cpp	2015-09-03 22:20:54 +0000
> @@ -319,6 +320,40 @@
>      QCOMPARE(messageReadSpy.first()[0].toString(), receivedMessageId);
>  }
>  
> +void HandlerTest::testAcknowledgeAllMessages()
> +{
> +    // FIXME: we assume the observer is already registered from the test above
> +    QString recipient("98437666");
> +    QString recipient2("+554198437666");
> +    QString message("Hello, world! %1");
> +    int messageCount = 10;
> +    QSignalSpy messageSentSpy(mMockController, SIGNAL(MessageSent(QString,QVariantMap)));
> +
> +    // first send a message to a certain number so the handler request one channel
> +    HandlerController::instance()->sendMessage(recipient, message, mTpAccount->uniqueIdentifier());
> +    TRY_COMPARE(messageSentSpy.count(), 1);
> +
> +    QSignalSpy messageReceivedSpy(ChatManager::instance(), SIGNAL(messageReceived(QString,QString,QDateTime,QString,bool)));
> +
> +    // now receive some messages from a very similar number so CM creates another
> +    // channel and the handler needs to deal with both
> +    QVariantMap properties;
> +    properties["Sender"] = recipient2;
> +    properties["Recipients"] = (QStringList() << recipient2);
> +    for (int i = 0; i < messageCount; ++i) {
> +        mMockController->PlaceIncomingMessage(message.arg(QString::number(i)), properties);
> +    }
> +
> +    TRY_COMPARE(messageReceivedSpy.count(), messageCount);
> +
> +    // then acknoledge the messages that arrived in the second channel and make sure handler

small typo.

> +    // does the right thing
> +    QSignalSpy messageReadSpy(mMockController, SIGNAL(MessageRead(QString)));
> +    ChatManager::instance()->acknowledgeAllMessages(properties["Recipients"].toStringList(), mTpAccount->uniqueIdentifier());
> +
> +    TRY_COMPARE(messageReadSpy.count(), messageCount);
> +}
> +
>  void HandlerTest::testActiveCallIndicator()
>  {
>      // start by making sure the property is false by default


-- 
https://code.launchpad.net/~boiko/telephony-service/remove_messages_from_menu/+merge/270119
Your team Ubuntu Phablet Team is subscribed to branch lp:telephony-service.