Merge lp:~boiko/telephony-service/do_not_accept_outgoing_channels into lp:telephony-service

[Gustavo Pichorim Boiko]

Make sure outgoing calls are not set as accepted by the approver.

Make sure outgoing calls are not set as accepted by the approver.

== Checklist ==
Are there any related MPs required for this MP to build/function as expected? Please list.
No

Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)
Yes

Did you perform an exploratory manual test run of your code change and any related functionality on device or emulator?
Yes

Did you successfully run all tests found in your component's Test Plan (https://wiki.ubuntu.com/Process/Merges/TestPlan/<package-name>) on device or emulator?
Yes

If you changed the UI, was the change specified/approved by design?
N/A

If you changed UI labels, did you update the pot file?
N/A

If you changed the packaging (debian), did you add a core-dev as a reviewer to this MP?
N/A

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1106
http://jenkins.qa.ubuntu.com/job/telephony-service-ci/505/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-amd64-ci/177
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-armhf-ci/173
        deb: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-armhf-ci/173/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-i386-ci/173

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/telephony-service-ci/505/rebuild

review: Approve (continuous-integration)

[lp:~boiko/telephony-service/do_not_accept_outgoing_channels]

1107. By Gustavo Pichorim Boiko on 2015-07-22

Add a unit test for the bug.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1107
http://jenkins.qa.ubuntu.com/job/telephony-service-ci/506/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-amd64-ci/178/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-armhf-ci/174/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-i386-ci/174/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/telephony-service-ci/506/rebuild

review: Needs Fixing (continuous-integration)

[lp:~boiko/telephony-service/do_not_accept_outgoing_channels]

1108. By Gustavo Pichorim Boiko on 2015-07-23

Fix handler test.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1108
http://jenkins.qa.ubuntu.com/job/telephony-service-ci/508/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-amd64-ci/180
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-armhf-ci/176
        deb: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-armhf-ci/176/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-i386-ci/176

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/telephony-service-ci/508/rebuild

review: Approve (continuous-integration)

[Tiago Salem Herrmann (tiagosh)]

Just one small remark.
Looks good otherwise.

review: Needs Fixing

[lp:~boiko/telephony-service/do_not_accept_outgoing_channels]

1109. By Gustavo Pichorim Boiko on 2015-07-23

Use initialised instead of initialized to be consistent with telepathy.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1109
http://jenkins.qa.ubuntu.com/job/telephony-service-ci/510/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-amd64-ci/182/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-armhf-ci/178/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-i386-ci/178/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/telephony-service-ci/510/rebuild

review: Needs Fixing (continuous-integration)

[lp:~boiko/telephony-service/do_not_accept_outgoing_channels]

1110. By Gustavo Pichorim Boiko on 2015-07-23

Update tests.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1110
http://jenkins.qa.ubuntu.com/job/telephony-service-ci/511/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-amd64-ci/183
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-armhf-ci/179
        deb: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-armhf-ci/179/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-i386-ci/179

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/telephony-service-ci/511/rebuild

review: Approve (continuous-integration)

[lp:~boiko/telephony-service/do_not_accept_outgoing_channels]

1111. By Gustavo Pichorim Boiko on 2015-07-27

Make sure the approver don't call accept() on call channels that are already active.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1111
http://jenkins.qa.ubuntu.com/job/telephony-service-ci/512/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-amd64-ci/184
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-armhf-ci/180
        deb: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-armhf-ci/180/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telephony-service-vivid-i386-ci/180

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/telephony-service-ci/512/rebuild

review: Approve (continuous-integration)

[Tiago Salem Herrmann (tiagosh)]

Looks good now. thanks.

--Checklist--
Did you perform an exploratory manual test run of the code change and any related functionality on device or emulator?
Yes

Did CI run pass? If not, please explain why.
Yes

Have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?
Yes

review: Approve

=== modified file 'approver/approver.cpp'
--- approver/approver.cpp	2015-07-02 03:35:20 +0000
+++ approver/approver.cpp	2015-07-27 19:38:20 +0000
@@ -302,9 +302,7 @@
         return;
     }
 
-    bool isIncoming = channel->initiatorContact() != dispatchOp->connection()->selfContact();
-
-    if (isIncoming && !callChannel->isRequested() && callChannel->callState() == Tp::CallStateInitialised) {
+    if (isIncoming(channel) && !callChannel->isRequested() && callChannel->callState() == Tp::CallStateInitialised) {
         callChannel->setRinging();
     } else {
         onApproved(dispatchOp);
@@ -562,7 +560,7 @@
     // accept all channels
     Q_FOREACH(Tp::ChannelPtr channel, dispatchOp->channels()) {
         Tp::CallChannelPtr callChannel = Tp::CallChannelPtr::dynamicCast(channel);
-        if (callChannel) {
+        if (callChannel && isIncoming(callChannel) && callChannel->callState() != Tp::CallStateActive) {
             callChannel->accept();
         }
     }
@@ -591,6 +589,11 @@
     return callDispatchOp;
 }
 
+bool Approver::isIncoming(const Tp::ChannelPtr &channel)
+{
+   return channel->initiatorContact() != channel->connection()->selfContact();
+}
+
 void Approver::processChannels()
 {
     Q_FOREACH (Tp::ChannelDispatchOperationPtr dispatchOperation, mDispatchOps) {

=== modified file 'approver/approver.h'
--- approver/approver.h	2015-07-01 21:29:58 +0000
+++ approver/approver.h	2015-07-27 19:38:20 +0000
@@ -61,6 +61,7 @@
 
 protected:
     Tp::ChannelDispatchOperationPtr dispatchOperationForIncomingCall();
+    bool isIncoming(const Tp::ChannelPtr &channel);
 
 private Q_SLOTS:
     void processChannels();

=== modified file 'tests/approver/ApproverTest.cpp'
--- tests/approver/ApproverTest.cpp	2015-07-08 21:11:14 +0000
+++ tests/approver/ApproverTest.cpp	2015-07-27 19:38:20 +0000
@@ -35,6 +35,8 @@
     void cleanup();
     void testSnapDecisionTimeout();
     void testAcceptCall();
+    void testCarKitOutgoingCall();
+    void testCarKitIncomingCall();
 
 private:
     void waitForCallActive(const QString &callerId);
@@ -104,5 +106,64 @@
     mMockController->HangupCall(callerId);
 }
 
+void ApproverTest::testCarKitOutgoingCall()
+{
+    // make sure that an outgoing call placed outside of telepathy is handle correctly
+    QString callerId("2345678");
+
+    QVariantMap properties;
+    properties["Caller"] = callerId;
+    properties["State"] = "outgoing";
+
+    QSignalSpy callStateSpy(mMockController, SIGNAL(CallStateChanged(QString,QString,QString)));
+
+    QDBusInterface notificationsMock("org.freedesktop.Notifications", "/org/freedesktop/Notifications", "org.freedesktop.Notifications");
+    QSignalSpy notificationSpy(&notificationsMock, SIGNAL(MockNotificationReceived(QString, uint, QString, QString, QString, QStringList, QVariantMap, int)));
+    QString objectPath = mMockController->placeCall(properties);
+
+    // wait for a few seconds and check that no notification was displayed
+    QTest::qWait(3000);
+    QCOMPARE(notificationSpy.count(), 0);
+
+    // check that the call state is not "accepted"
+    TRY_VERIFY(callStateSpy.count() > 0);
+    QCOMPARE(callStateSpy.last()[0].toString(), callerId);
+    QCOMPARE(callStateSpy.last()[1].toString(), objectPath);
+    QCOMPARE(callStateSpy.last()[2].toString(), QString("initialised"));
+    mMockController->HangupCall(callerId);
+}
+
+void ApproverTest::testCarKitIncomingCall()
+{
+    // make sure that an outgoing call placed outside of telepathy is handle correctly
+    QString callerId("3456789");
+
+    QVariantMap properties;
+    properties["Caller"] = callerId;
+    properties["State"] = "incoming";
+
+    QDBusInterface notificationsMock("org.freedesktop.Notifications", "/org/freedesktop/Notifications", "org.freedesktop.Notifications");
+    QSignalSpy notificationSpy(&notificationsMock, SIGNAL(MockNotificationReceived(QString, uint, QString, QString, QString, QStringList, QVariantMap, int)));
+    QString objectPath = mMockController->placeCall(properties);
+
+    TRY_COMPARE(notificationSpy.count(), 1);
+
+    // at this point we are already sure the approver has the call, as the notification was placed
+    QSignalSpy callStateSpy(mMockController, SIGNAL(CallStateChanged(QString,QString,QString)));
+
+    // now set the call state as active to simulate the call being answered on the car kit
+    mMockController->SetCallState(callerId, "active");
+
+    // wait for a few seconds while the approver approves the channel and give it to the handler
+    QTest::qWait(3000);
+    QCOMPARE(callStateSpy.count(), 1);
+    QCOMPARE(callStateSpy.first()[0].toString(), callerId);
+    QCOMPARE(callStateSpy.first()[1].toString(), objectPath);
+
+    // the call state needs to be active. If it is in any other state, it is because the approver wrongly called Accept() on the channel
+    QCOMPARE(callStateSpy.first()[2].toString(), QString("active"));
+    mMockController->HangupCall(callerId);
+}
+
 QTEST_MAIN(ApproverTest)
 #include "ApproverTest.moc"

=== modified file 'tests/common/mock/callchannel.cpp'
--- tests/common/mock/callchannel.cpp	2015-07-17 18:18:57 +0000
+++ tests/common/mock/callchannel.cpp	2015-07-27 19:38:20 +0000
@@ -89,17 +89,19 @@
     mIncoming = mState == "incoming" || mState == "waiting";
 
     identifiers[mTargetHandle] = mPhoneNumber;
+
     reason.actor =  0;
     reason.reason = Tp::CallStateChangeReasonProgressMade;
     reason.message = "";
     reason.DBusReason = "";
+
     if (mIncoming) {
         memberFlags[mTargetHandle] = 0;
     } else {
         memberFlags[mTargetHandle] = Tp::CallMemberFlagRinging;
     }
 
-    mCallChannel->setCallState(Tp::CallStateInitialising, 0, reason, stateDetails);
+    setCallState("initialising");
 
     mCallContent = Tp::BaseCallContent::create(baseChannel()->dbusConnection(), baseChannel().data(), "audio", Tp::MediaStreamTypeAudio, Tp::MediaStreamDirectionNone);
 
@@ -112,7 +114,7 @@
 
     mCallChannel->setMembersFlags(memberFlags, identifiers, Tp::UIntList(), reason);
 
-    mCallChannel->setCallState(Tp::CallStateInitialised, 0, reason, stateDetails);
+    setCallState("initialised");
     QObject::connect(mBaseChannel.data(), SIGNAL(closed()), this, SLOT(deleteLater()));
 }
 
@@ -223,6 +225,12 @@
     } else if (state == "waiting") {
         mIncoming = true;
         qDebug() << "waiting";
+    } else if (state == "initialising") {
+        reason.reason = Tp::CallStateChangeReasonProgressMade;
+        mCallChannel->setCallState(Tp::CallStateInitialising, 0, reason, stateDetails);
+    } else if (state == "initialised") {
+        reason.reason = Tp::CallStateChangeReasonProgressMade;
+        mCallChannel->setCallState(Tp::CallStateInitialised, 0, reason, stateDetails);
     }
 
     mState = state;

=== modified file 'tests/handler/HandlerTest.cpp'
--- tests/handler/HandlerTest.cpp	2015-07-02 00:07:05 +0000
+++ tests/handler/HandlerTest.cpp	2015-07-27 19:38:20 +0000
@@ -306,7 +306,8 @@
 void HandlerTest::testNotApprovedChannels()
 {
     QVariantMap properties;
-    properties["Caller"] = "123456";
+    QString callerId = "123456";
+    properties["Caller"] = callerId;
     properties["State"] = "incoming";
 
     QSignalSpy approverCallSpy(mApprover, SIGNAL(newCall()));
@@ -324,8 +325,11 @@
     // wait for a few seconds
     QTest::qWait(3000);
 
-    // no state changes should happen, as the channel was not accepted
-    QVERIFY(callStateSpy.isEmpty());
+    // the last state received should be initialised
+    TRY_VERIFY(callStateSpy.count() > 0);
+    QCOMPARE(callStateSpy.last()[0].toString(), callerId);
+    QCOMPARE(callStateSpy.last()[1].toString(), objectPath);
+    QCOMPARE(callStateSpy.last()[2].toString(), QString("initialised"));
 }
 
 void HandlerTest::registerApprover()