messaging-app

Merge lp:~tiagosh/messaging-app/dual-sim2 into lp:messaging-app

dual-sim2
Merge into trunk

Proposed by Tiago Salem Herrmann on 2014-07-15

Status:	Merged
Approved by:	Gustavo Pichorim Boiko on 2014-07-16
Approved revision:	147
Merged at revision:	150
Proposed branch:	lp:~tiagosh/messaging-app/dual-sim2
Merge into:	lp:messaging-app
Diff against target:	465 lines (+154/-54) 6 files modified src/qml/MainPage.qml (+6/-4) src/qml/MessageDelegate.qml (+15/-1) src/qml/Messages.qml (+128/-46) src/qml/NewRecipientPage.qml (+1/-1) src/qml/ThreadDelegate.qml (+3/-1) tests/autopilot/messaging_app/emulators.py (+1/-1)
To merge this branch:	bzr merge lp:~tiagosh/messaging-app/dual-sim2
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Gustavo Pichorim Boiko (community)		2014-07-15	Approve on 2014-07-16
PS Jenkins bot	continuous-integration		Needs Fixing on 2014-07-15
Review via email: mp+226906@code.launchpad.net

Commit message

- Implement sim card switcher when there is more than one account available (label is hardcoded for now)
- Update some visuals to match the new spec
- Merge messages from different accounts

Description of the change

- Implement sim card switcher when there is more than one account available (label is hardcoded for now)
- Update some visuals to match the new spec
- Merge messages from different accounts

--Checklist--
Are there any related MPs required for this MP to build/function as expected? Please list.
https://code.launchpad.net/~tiagosh/telephony-service/dual-sim2/+merge/226904
https://code.launchpad.net/~boiko/history-service/thread_proxy_model/+merge/226758

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/messaging-app) on device or emulator?
Yes

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

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

Revision history for this message

Gustavo Pichorim Boiko (boiko) wrote on 2014-07-15:

The code is looking good, there are just a few minor things that need to be fixed. Please check the inline diff comments.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-15:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/messaging-app-ci/291/rebuild

review: Needs Fixing (continuous-integration)

lp:~tiagosh/messaging-app/dual-sim2 updated on 2014-07-15

147. By Tiago Salem Herrmann on 2014-07-15: fix translation
update method name to reflect its real job
remove unused code

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-15:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/messaging-app-ci/292/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Gustavo Pichorim Boiko (boiko) wrote on 2014-07-16:

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

Code looks good and works as expected!

review: Approve

lp:~tiagosh/messaging-app/dual-sim2 updated on 2014-07-17

148. By Tiago Salem Herrmann on 2014-07-17: merge trunk
149. By Tiago Salem Herrmann on 2014-07-17: only show accountLabel if there is more than 1 account
150. By Tiago Salem Herrmann on 2014-07-17: increase touch area
151. By Tiago Salem Herrmann on 2014-07-17: increase font size
fix bottom edge swipe

Revision history for this message

Brendan Donegan (brendan-donegan) wrote on 2014-07-21:

The CI run did not pass here - I guess the failures have been ignored because of false positives? That should really be fixed, and until then the tests should be run manually on the device. If you had done this you would have noticed you broke a test by changing the 'name' property of the Add contact icon from 'new-contact' to 'contact'.

Revision history for this message

Brendan Donegan (brendan-donegan) wrote on 2014-07-21:

Also, are we planning to have any tests for the dual SIM feature?

Revision history for this message

Tiago Salem Herrmann (tiagosh) wrote on 2014-07-21:

The tests failed because this MR needs changes in the history-service and telephony-service, as explained in the MR description. If you check the logs you will see that the app is not even running correctly because the new proxy model is inexistent. Also, I probably forgot to run the tests manually (only tested the functionalities on the device) and ended up breaking the test. My bad! I will fix this still today. Thank for the report.

About tests for dual sim, I believe we need to add some, but the current infrastructure (with-ofono-phonesim script) does not support this setup yet.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Sam Short

Tiago Salem Herrmann

Ubuntu Phablet Team

 === modified file 'src/qml/MainPage.qml'
 --- src/qml/MainPage.qml	2014-06-17 13:18:57 +0000
 +++ src/qml/MainPage.qml	2014-07-17 21:29:41 +0000
@@ -131,11 +131,12 @@
+         }
+     }
--    SortProxyModel {
++    HistoryThreadGroupingProxyModel {
          id: sortProxy
          sortRole: HistoryThreadModel.LastEventTimestampRole
          sourceModel: threadModel
          ascending: false
++        groupingProperty: "participants"
+     }
      HistoryThreadModel {
@@ -201,7 +202,6 @@
+                     }
                  } else {
                      var properties = {}
--                    properties["threadId"] = threadId
                      properties["accountId"] = accountId
                      properties["participants"] = participants
                      properties["keyboardFocus"] = false
@@ -215,8 +215,10 @@
+         }
          onSelectionDone: {
              for (var i=0; i < items.count; i++) {
--                var thread = items.get(i).model
--                threadModel.removeThread(thread.accountId, thread.threadId, thread.type)
++                var threads = items.get(i).model.threads
++                for (var j in threads) {
++                    threadModel.removeThread(threads[j].accountId, threads[j].threadId, threads[j].type)
++                }
+             }
+         }
 === modified file 'src/qml/MessageDelegate.qml'
 --- src/qml/MessageDelegate.qml	2014-07-07 13:11:03 +0000
 +++ src/qml/MessageDelegate.qml	2014-07-17 21:29:41 +0000
@@ -38,6 +38,7 @@
      property alias selected: internalDelegate.selected
      property variant activeAttachment
      property string mmsText: ""
++    property string accountLabel: ""
      anchors.left: parent ? parent.left : undefined
      anchors.right: parent ? parent.right: undefined
@@ -262,13 +263,26 @@
                        textMessageStatus == HistoryThreadModel.MessageStatusTemporarilyFailed) && !incoming && mmsText === ""
+         }
++        Label {
++            id: accountIndicator
++            anchors {
++                right: bubble.left
++                rightMargin: units.gu(0.5)
++                bottom: bubble.bottom
++            }
++            text: accountLabel
++            visible: !incoming
++            font.pixelSize: FontUtils.sizeToPixels("small")
++            color: "green"
++        }
++
          // FIXME: this is just a temporary workaround while we dont have the final design
          UbuntuShape {
              id: warningButton
              color: "yellow"
              height: units.gu(3)
              width: units.gu(3)
--            anchors.right: bubble.left
++            anchors.right: accountIndicator.left
              anchors.left: undefined
              anchors.verticalCenter: bubble.verticalCenter
              anchors.leftMargin: 0
 === modified file 'src/qml/Messages.qml'
 --- src/qml/Messages.qml	2014-07-16 15:11:35 +0000
 +++ src/qml/Messages.qml	2014-07-17 21:29:41 +0000
@@ -31,9 +31,8 @@
  Page {
      id: messages
      objectName: "messagesPage"
--    property string threadId: ""
--    property bool newMessage: threadId === ""
--    // FIXME: we should get the account ID properly when dealing with multiple accounts
++    // FIXME this info must come from system settings or telephony-service
++    property var accounts: {"ofono/ofono/account0": "SIM 1", "ofono/ofono/account1": "SIM 2"}
      property string accountId: telepathyHelper.accountIds[0]
      property variant participants: []
      property bool groupChat: participants.length > 1
@@ -46,6 +45,7 @@
      property var activeTransfer: null
      property int activeAttachmentIndex: -1
      property var sharedAttachmentsTransfer: []
++    property string lastFilter: ""
      property string text: ""
      function addAttachmentsToModel(transfer) {
@@ -64,6 +64,10 @@
+         }
+     }
++    function checkNetwork() {
++        return telepathyHelper.isAccountConnected(messages.accountId)
++    }
++
      ListModel {
          id: attachments
+     }
@@ -85,7 +89,7 @@
      flickable: null
      // we need to use isReady here to know if this is a bottom edge page or not.
--    __customHeaderContents: newMessage && isReady ? newMessageHeader : null
++    __customHeaderContents: participants.length === 0 && isReady ? newMessageHeader : null
      property bool isReady: false
      signal ready
      onReady: {
@@ -112,7 +116,7 @@
              if (participants.length == 1) {
                  return firstRecipient
              } else {
--                return i18n.tr("Group")
++                return i18n.tr("Group (%1 members)").arg(participants.length)
+             }
+         }
          return i18n.tr("New Message")
@@ -136,17 +140,37 @@
+     }
      Component.onCompleted: {
--        threadId = getCurrentThreadId()
++        updateFilters()
          addAttachmentsToModel(sharedAttachmentsTransfer)
+     }
--    function getCurrentThreadId() {
--        if (participants.length == 0)
--            return ""
--        return eventModel.threadIdForParticipants(accountId,
--                                                              HistoryThreadModel.EventTypeText,
--                                                              participants,
--                                                              HistoryThreadModel.MatchPhoneNumber)
++    function updateFilters() {
++        if (participants.length == 0) {
++            eventModel.filter = null
++            return
++        }
++        var componentUnion = "import Ubuntu.History 0.1; HistoryUnionFilter { %1 }"
++        var componentFilters = ""
++        for (var i = 0; i < telepathyHelper.accountIds.length; i++) {
++            var filterValue = eventModel.threadIdForParticipants(telepathyHelper.accountIds[i],
++                                                                 HistoryThreadModel.EventTypeText,
++                                                                 participants,
++                                                                 HistoryThreadModel.MatchPhoneNumber)
++            if (filterValue === "") {
++                continue
++            }
++            componentFilters += 'HistoryFilter { filterProperty: "threadId"; filterValue: "%1" } '.arg(filterValue)
++        }
++        if (componentFilters === "") {
++            eventModel.filter = null
++            lastFilter = ""
++            return
++        }
++        if (componentFilters != lastFilter) {
++            var finalString = componentUnion.arg(componentFilters)
++            eventModel.filter = Qt.createQmlObject(finalString, eventModel)
++            lastFilter = componentFilters
++        }
+     }
      function markMessageAsRead(accountId, threadId, eventId, type) {
@@ -216,6 +240,65 @@
+         }
+     }
++    Component {
++        id: noNetworkDialog
++        Dialog {
++            id: dialogue
++            title: i18n.tr("No network")
++            text: telepathyHelper.accountIds.length >= 2 ? i18n.tr("There is currently no network on %1").arg(messages.accounts[messages.accountId]) : i18n.tr("There is currently no network.")
++            Button {
++                objectName: "closeNoNetworkDialog"
++                text: i18n.tr("Close")
++                color: UbuntuColors.orange
++                onClicked: {
++                    PopupUtils.close(dialogue)
++                    Qt.inputMethod.hide()
++                }
++            }
++        }
++    }
++
++    Rectangle {
++        id: accountList
++        z: 1
++        anchors {
++            left: parent.left
++            right: parent.right
++            top: parent.top
++        }
++        height: telepathyHelper.accountIds.length > 1 ? childrenRect.height : 0
++        color: "white"
++        Row {
++            anchors {
++                top: parent.top
++                horizontalCenter: parent.horizontalCenter
++            }
++            height: childrenRect.height
++            width: childrenRect.width
++            spacing: units.gu(2)
++            Repeater {
++                model: telepathyHelper.accountIds
++                delegate: Label {
++                    width: paintedWidth
++                    height: paintedHeight
++                    text: messages.accounts[modelData]
++                    font.pixelSize: FontUtils.sizeToPixels("small")
++                    color: messages.accountId == modelData ? "red" : "#5d5d5d"
++                    MouseArea {
++                        anchors {
++                            fill: parent
++                            // increase touch area
++                            leftMargin: units.gu(-1)
++                            rightMargin: units.gu(-1)
++                            bottomMargin: units.gu(-1)
++                        }
++                        onClicked: messages.accountId = modelData
++                    }
++                }
++            }
++        }
++    }
++
      Item {
          id: newMessageHeader
          anchors {
@@ -223,7 +306,6 @@
              rightMargin: units.gu(1)
              right: parent.right
              bottom: parent.bottom
--            top: parent.top
+         }
          visible: participants.length == 0 && isReady && messages.active
          MultiRecipientInput {
@@ -248,7 +330,7 @@
                  verticalCenter: parent.verticalCenter
+             }
--            name: "new-contact"
++            name: "contact"
              color: "gray"
              MouseArea {
                  anchors.fill: parent
@@ -285,7 +367,7 @@
+         ]
          anchors {
--            top: parent.top
++            top: accountList.bottom
              topMargin: units.gu(1)
              left: parent.left
              right: parent.right
@@ -403,7 +485,7 @@
+     }
      onParticipantsChanged: {
--        threadId = getCurrentThreadId()
++        updateFilters()
+     }
      ToolbarItems {
@@ -447,7 +529,7 @@
              objectName: "groupChatButton"
              action: Action {
                  objectName: "groupChatAction"
--                iconSource: "image://theme/navigation-menu"
++                iconSource: "image://theme/contact-group"
                  onTriggered: {
                      PopupUtils.open(participantsPopover, messages.header)
+                 }
@@ -533,16 +615,7 @@
      HistoryEventModel {
          id: eventModel
          type: HistoryThreadModel.EventTypeText
--        filter: HistoryIntersectionFilter {
--            HistoryFilter {
--                filterProperty: "threadId"
--                filterValue: threadId
--            }
--            HistoryFilter {
--                filterProperty: "accountId"
--                filterValue: accountId
--            }
--        }
++        filter: null
          sort: HistorySort {
             sortField: "timestamp"
             sortOrder: HistorySort.DescendingOrder
@@ -551,7 +624,7 @@
      SortProxyModel {
          id: sortProxy
--        sourceModel: eventModel
++        sourceModel: eventModel.filter ? eventModel : null
          sortRole: HistoryEventModel.TimestampRole
          ascending: false
+     }
@@ -561,7 +634,7 @@
          objectName: "messageList"
          clip: true
          anchors {
--            top: parent.top
++            top: accountList.bottom
              left: parent.left
              right: parent.right
              bottom: bottomPanel.top
@@ -573,7 +646,7 @@
          footer: Item {
              height: units.gu(2)
+         }
--        listModel: !newMessage ? sortProxy : null
++        listModel: participants.length > 0 ? sortProxy : null
          verticalLayoutDirection: ListView.BottomToTop
          spacing: units.gu(2)
          highlightFollowsCurrentItem: false
@@ -586,6 +659,7 @@
              removable: !messages.selectionMode
              selectionMode: messages.selectionMode
              confirmRemoval: true
++            accountLabel: telepathyHelper.accountIds.length > 1 ? messages.accounts[accountId] : ""
              onClicked: {
                  if (messageList.isInSelectionMode) {
                      if (!messageList.selectItem(messageDelegate)) {
@@ -624,7 +698,7 @@
                      return
+                 }
                  eventModel.removeEvent(accountId, threadId, eventId, type)
--                chatManager.sendMessage(messages.participants, textMessage, accountId)
++                chatManager.sendMessage(messages.participants, textMessage, messages.accountId)
+             }
+         }
          onSelectionDone: {
@@ -666,7 +740,7 @@
              height: units.gu(3)
              width: units.gu(3)
              color: "gray"
--            name: "camera"
++            name: "camera-app-symbolic"
              MouseArea {
                  anchors.fill: parent
                  onClicked: {
@@ -825,6 +899,8 @@
                  placeholderText: i18n.tr("Write a message...")
                  focus: textEntry.focus
                  font.family: "Ubuntu"
++                font.pixelSize: FontUtils.sizeToPixels("medium")
++                color: "#5d5d5d"
                  text: messages.text
+             }
@@ -852,10 +928,10 @@
              text: "Send"
              color: "green"
              width: units.gu(7)
++            height: units.gu(4)
++            font.pixelSize: FontUtils.sizeToPixels("small")
              enabled: {
--                if (!telepathyHelper.connected)
--                    return false
--                if (participants.length > 0 || multiRecipient.recipientCount > 0) {
++               if (participants.length > 0 || multiRecipient.recipientCount > 0) {
                      if (textEntry.text != "" || textEntry.inputMethodComposing || attachments.count > 0) {
                          return true
+                     }
@@ -863,26 +939,32 @@
                  return false
+             }
              onClicked: {
++                if (!checkNetwork()) {
++                    Qt.inputMethod.hide()
++                    PopupUtils.open(noNetworkDialog)
++                    return
++                }
                  // make sure we flush everything we have prepared in the OSK preedit
                  Qt.inputMethod.commit();
                  if (textEntry.text == "" && attachments.count == 0) {
                      return
+                 }
--                if (participants.length == 0 && multiRecipient.recipientCount > 0) {
--                    participants = multiRecipient.recipients
--                }
                  if (messages.accountId == "") {
                      // FIXME: handle dual sim
                      messages.accountId = telepathyHelper.accountIds[0]
+                 }
--                if (messages.newMessage) {
--                    // create the new thread and get the threadId
--                    messages.threadId = eventModel.threadIdForParticipants(messages.accountId,
--                                                                            HistoryThreadModel.EventTypeText,
--                                                                            participants,
--                                                                            HistoryThreadModel.MatchPhoneNumber,
--                                                                            true)
++                // dont change the participants list
++                if (participants.length == 0) {
++                    participants = multiRecipient.recipients
+                 }
++                // create the new thread and update the threadId list
++                eventModel.threadIdForParticipants(messages.accountId,
++                                                   HistoryThreadModel.EventTypeText,
++                                                   participants,
++                                                   HistoryThreadModel.MatchPhoneNumber,
++                                                   true)
++
++                updateFilters()
                  messages.pendingMessage = true
                  if (attachments.count > 0) {
                      var newAttachments = []
 === modified file 'src/qml/NewRecipientPage.qml'
 --- src/qml/NewRecipientPage.qml	2014-07-16 15:11:35 +0000
 +++ src/qml/NewRecipientPage.qml	2014-07-17 21:29:41 +0000
@@ -42,7 +42,7 @@
+         }
          onTextChanged: contactList.currentIndex = -1
          inputMethodHints: Qt.ImhNoPredictiveText
--        placeholderText: i18n.tr("Type a name or phone to search")
++        placeholderText: i18n.tr("Type to search by name or number")
+     }
      ContactListView {
 === modified file 'src/qml/ThreadDelegate.qml'
 --- src/qml/ThreadDelegate.qml	2014-07-06 22:06:24 +0000
 +++ src/qml/ThreadDelegate.qml	2014-07-17 21:29:41 +0000
@@ -180,7 +180,9 @@
          font.weight: Font.Light
+     }
      onItemRemoved: {
--        threadModel.removeThread(accountId, threadId, type)
++        for (var i in threads) {
++            threadModel.removeThread(threads[i].accountId, threads[i].threadId, threads[i].type)
++        }
+     }
      Item {
 === modified file 'tests/autopilot/messaging_app/emulators.py'
 --- tests/autopilot/messaging_app/emulators.py	2014-07-15 16:32:01 +0000
 +++ tests/autopilot/messaging_app/emulators.py	2014-07-17 21:29:41 +0000
@@ -449,7 +449,7 @@
              ThreadDelegate, objectName='thread{}'.format(participants))
          self.pointing_device.click_object(thread)
          root = self.get_root_instance()
--        return root.wait_select_single(Messages, threadId=thread.threadId)
++        return root.wait_select_single(Messages, participants=participants)
  class Messages(toolkit_emulators.UbuntuUIToolkitEmulatorBase):