[Merge] lp:~boiko/dialer-app/clear_call_notifications into lp:dialer-app

Tiago Salem Herrmann tiago.herrmann at canonical.com
Thu May 21 21:38:20 UTC 2015


Thanks. I left some comments below.

Diff comments:

> === modified file 'src/qml/HistoryPage/HistoryDelegate.qml'
> --- src/qml/HistoryPage/HistoryDelegate.qml	2015-04-09 19:49:26 +0000
> +++ src/qml/HistoryPage/HistoryDelegate.qml	2015-05-21 18:56:31 +0000
> @@ -45,6 +45,9 @@
>      property bool active: false
>  
>      function activate() {
> +        // clear any notification related to this call
> +        callNotification.clearCallNotification(model.remoteParticipant, model.accountId)

Should we only remove the notification when the item is activated? Was this agreed with design? I thought all missed calls should be removed when the call log is displayed.

> +
>          // ignore private and unknown numbers
>          if (!interactive) {
>              return;
> 
> === modified file 'tests/qml/CMakeLists.txt'
> --- tests/qml/CMakeLists.txt	2015-04-07 19:56:35 +0000
> +++ tests/qml/CMakeLists.txt	2015-05-21 18:56:31 +0000
> @@ -27,6 +27,7 @@
>      declare_qml_test("keypad_button" tst_KeypadButton.qml)
>      declare_qml_test("main_view" tst_MainView.qml)
>      declare_qml_test("stopwatch" tst_StopWatch.qml)
> +    declare_qml_test("historydelegate" tst_HistoryDelegate.qml)
>  # FIXME currently disabled. There is a bug when registering the telepathy observer
>  #    declare_qml_dbus_test("dialer_page" tst_DialerPage.qml)
>  else()
> 
> === added file 'tests/qml/tst_HistoryDelegate.qml'
> --- tests/qml/tst_HistoryDelegate.qml	1970-01-01 00:00:00 +0000
> +++ tests/qml/tst_HistoryDelegate.qml	2015-05-21 18:56:31 +0000
> @@ -0,0 +1,60 @@
> +import QtQuick 2.2
> +import QtTest 1.0
> +import Ubuntu.Test 0.1
> +import "../../src/qml/HistoryPage"
> +
> +Item {
> +    id: root
> +    width: units.gu(40)
> +    height: units.gu(10)
> +
> +    signal clearCallNotificationCalled(string remoteParticipant, string accountId)
> +
> +    Item {
> +        id: callNotification
> +
> +        function clearCallNotification(remoteParticipant, accountId) {
> +            root.clearCallNotificationCalled(remoteParticipant, accountId);
> +        }
> +    }
> +
> +    Item {
> +        id: mainView
> +
> +        function populateDialpad(remoteParticipant, accountId) {
> +            // FIXME: implement
> +        }
> +    }
> +
> +    Item {
> +        id: model
> +        property variant participants: [ {identifier: "12345"} ]
> +        property string senderId: "12345"
> +        property string remoteParticipant: "12345"
> +        property string accountId: "theAccountId"
> +        property bool callMissed: true
> +        property int eventCount: 1
> +        property variant timestamp: 0
> +    }
> +
> +    HistoryDelegate {
> +        id: delegate
> +    }
> +
> +    UbuntuTestCase {
> +        id: historyDelegateTestCase
> +        name: 'historyDelegateTestCase'
> +
> +        SignalSpy {
> +            id: clearNotificationSpy
> +            target: root
> +            signalName: 'clearCallNotificationCalled'
> +        }
> +
> +        function test_notificationClearedAfterItemPressed() {
> +            delegate.activate()
> +            compare(clearNotificationSpy.count, 1, 'clear notification not called')
> +        }
> +    }
> +
> +    }

Can you fix the indentation here?

> 


-- 
https://code.launchpad.net/~boiko/dialer-app/clear_call_notifications/+merge/259831
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~boiko/dialer-app/clear_call_notifications into lp:dialer-app.



More information about the Ubuntu-reviews mailing list