Merge lp:~renatofilho/address-book-app/fix-1268731 into lp:address-book-app

[Renato Araujo Oliveira Filho]

Fixed phone and email context save.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:117
http://jenkins.qa.ubuntu.com/job/address-book-app-ci/353/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-trusty-amd64-ci/63
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-trusty-armhf-ci/63
        deb: http://jenkins.qa.ubuntu.com/job/address-book-app-trusty-armhf-ci/63/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-trusty-i386-ci/63
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2313
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2193
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2031
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2315
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2315/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2193
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2193/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4652
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3067

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-app-ci/353/rebuild

review: Approve (continuous-integration)

[lp:~renatofilho/address-book-app/fix-1268731]

118. By Renato Araujo Oliveira Filho on 2014-01-14

Fixed field label with memory manager.

119. By Renato Araujo Oliveira Filho on 2014-01-14

Created autopilot test for phone and email field label.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:119
http://jenkins.qa.ubuntu.com/job/address-book-app-ci/355/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-trusty-amd64-ci/65
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-trusty-armhf-ci/65
        deb: http://jenkins.qa.ubuntu.com/job/address-book-app-trusty-armhf-ci/65/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-trusty-i386-ci/65
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2343
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2219
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2055
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2345
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2345/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2219
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2219/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4676
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3101

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-app-ci/355/rebuild

review: Approve (continuous-integration)

[Bill Filler (bfiller)]

Does not work for me. Here is what I tested

0) first I synced my google contacts
1) Add new contact
2) fill in first name and last name
3) add 3 email addresses (home, work, other)
4) press save

I either get "Failed to load contacts" dialog, or if I try again I just get spinning cursor on the black screen with "Cancel" and "Save" buttons showing on the bottom.

This is in the ~/.cache/upstart/application-legacy-address-book-app-.log:

void QtContacts::QDeclarativeContactModel::onContactsAddedFetchRequestStateChanged(QtContacts::QContactAbstractRequest::State) contact to be added already exists in the model

review: Needs Fixing

[Bill Filler (bfiller)]

Also with this branch this is broken:

1) create new contact with just first name/last name and nothing else
2) edit the contact and add 3 email addresses
3) save the contact, this is saved correctly
4) try to delete the contact

the contact is not deleted and cannot be deleted from toolbar or swipe to delete

review: Needs Fixing

[Bill Filler (bfiller)]

the reported problems are not due to this branch, they are reported here:
https://bugs.launchpad.net/ubuntu/+source/address-book-app/+bug/1269561

approving this branch

review: Approve

=== modified file 'src/imports/Common/ContactDetailGroupBase.qml'
--- src/imports/Common/ContactDetailGroupBase.qml	2013-11-14 13:25:28 +0000
+++ src/imports/Common/ContactDetailGroupBase.qml	2014-01-14 20:44:40 +0000
@@ -112,7 +112,7 @@
                 Binding {
                     target: detailItem.item
                     property: "detail"
-                    value: root.details[index]
+                    value: root.contact && root.details ? root.details[index] : null
                 }
 
                 Binding {

=== modified file 'src/imports/Common/ContactDetailGroupWithTypeBase.qml'
--- src/imports/Common/ContactDetailGroupWithTypeBase.qml	2013-07-15 22:43:13 +0000
+++ src/imports/Common/ContactDetailGroupWithTypeBase.qml	2014-01-14 20:44:40 +0000
@@ -40,7 +40,7 @@
     }
 
     typeModel: ListModel {
-
+        property bool ready: false
         signal loaded()
 
         function getTypeIndex(detail) {
@@ -61,7 +61,7 @@
                 return 0
             } else if (context === QtContacts.ContactDetail.ContextWork) {
                 return 1
-            } else if (subType === QtContacts.ContactDetail.ContextOther) {
+            } else if (context === QtContacts.ContactDetail.ContextOther) {
                 return 2
             } else {
                 return 0 // default value is "Home"
@@ -103,12 +103,6 @@
                 return false
             }
 
-            // WORKAROUND: in EDS empty context is equal to QtContacts.ContactDetail.ContextOther
-            // this will avoid call contact update if the context has not changed
-            if ((detail.contexts.length === 0) && (modelData.value === QtContacts.ContactDetail.ContextOther)) {
-                return false
-            }
-
             var newContext = detail.contexts.filter(isNotAModelValue)
             newContext.push(modelData.value)
             if (!compareList(newContext, detail.contexts)) {
@@ -122,6 +116,7 @@
             append({"value": QtContacts.ContactDetail.ContextHome, "label": i18n.tr("Home"), "icon": null})
             append({"value": QtContacts.ContactDetail.ContextWork, "label": i18n.tr("Work"), "icon": null})
             append({"value": QtContacts.ContactDetail.ContextOther, "label": i18n.tr("Other"), "icon": null})
+            ready = true
             loaded()
         }
     }

=== modified file 'src/imports/ContactEdit/ContactDetailGroupWithTypeEditor.qml'
--- src/imports/ContactEdit/ContactDetailGroupWithTypeEditor.qml	2013-11-18 19:17:01 +0000
+++ src/imports/ContactEdit/ContactDetailGroupWithTypeEditor.qml	2014-01-14 20:44:40 +0000
@@ -160,7 +160,7 @@
         Item {
             Connections {
                 target: root.typeModel
-                onLoaded: updateCombo(true)
+                onReadyChanged: updateCombo(true)
             }
         }
     }

=== modified file 'src/imports/ContactView/ContactDetailGroupWithTypeView.qml'
--- src/imports/ContactView/ContactDetailGroupWithTypeView.qml	2013-10-21 19:42:00 +0000
+++ src/imports/ContactView/ContactDetailGroupWithTypeView.qml	2014-01-14 20:44:40 +0000
@@ -51,7 +51,7 @@
     }
 
     detailDelegate: ContactDetailWithTypeView {
-        property variant detailType: detail && root.contact && root.typeModel ? root.getType(detail) : null
+        property variant detailType: detail && root.contact && root.typeModel.ready ? root.getType(detail) : null
 
         action: root.defaultAction
         contact: root.contact

=== modified file 'src/imports/ContactView/ContactDetailPhoneNumbersView.qml'
--- src/imports/ContactView/ContactDetailPhoneNumbersView.qml	2013-11-14 13:25:28 +0000
+++ src/imports/ContactView/ContactDetailPhoneNumbersView.qml	2014-01-14 20:44:40 +0000
@@ -35,7 +35,7 @@
     }
 
     detailDelegate: ContactDetailPhoneNumberView {
-        property variant detailType: detail && root.contact && root.typeModel ? root.getType(detail) : null
+        property variant detailType: detail && root.contact && root.typeModel.ready ? root.getType(detail) : null
         property bool isPreferred: root.contact && root.contact.preferredDetails && detail && root.contact.isPreferredDetail("TEL", detail)
 
         action: Action {

=== modified file 'src/imports/Ubuntu/Contacts/ContactDetailOnlineAccountTypeModel.qml'
--- src/imports/Ubuntu/Contacts/ContactDetailOnlineAccountTypeModel.qml	2013-07-31 19:36:55 +0000
+++ src/imports/Ubuntu/Contacts/ContactDetailOnlineAccountTypeModel.qml	2014-01-14 20:44:40 +0000
@@ -20,6 +20,8 @@
 ListModel {
     id: typeModel
 
+    property bool ready: false
+
     signal loaded()
 
     function getTypeIndex(detail) {
@@ -67,5 +69,6 @@
         /*4*/   append({"value": 7, "label": i18n.tr("Skype"), "icon": "artwork:/protocol-skype.svg"})
         /*5*/   append({"value": 8, "label": i18n.tr("Yahoo"), "icon": "artwork:/protocol-yahoo.svg"})
         loaded()
+        ready = true
     }
 }

=== modified file 'src/imports/Ubuntu/Contacts/ContactDetailPhoneNumberTypeModel.qml'
--- src/imports/Ubuntu/Contacts/ContactDetailPhoneNumberTypeModel.qml	2013-07-31 19:36:55 +0000
+++ src/imports/Ubuntu/Contacts/ContactDetailPhoneNumberTypeModel.qml	2014-01-14 20:44:40 +0000
@@ -20,6 +20,7 @@
 ListModel {
     id: typeModel
 
+    property bool ready: false
     signal loaded()
 
     function getTypeIndex(detail) {
@@ -38,7 +39,9 @@
             } else {
                 return 1
             }
-        } else{
+        } else if (contexts.indexOf(QtContacts.ContactDetail.ContextOther) > -1) {
+            return 4
+        } else {
             return 2 // Default value is "Mobile"
         }
     }
@@ -69,21 +72,11 @@
             return false
         }
 
-//        // WORKAROUND: in EDS empty context is equal to QtContacts.ContactDetail.ContextOther
-//        // this will avoid call contact update if the context has not changed
-//        if ((detail.contexts.length === 0) && (modelData.value === "Other")) {
-//            return
-//        }
-
         var newSubTypes = []
         var newContext = []
 
         newContext.push(modelData.context)
-        if (modelData.subType !== -1) {
-            newSubTypes.push(modelData.subType)
-        }
-        // All current labels is voice type
-        newSubTypes.push(QtContacts.PhoneNumber.Voice)
+        newSubTypes.push(modelData.subType)
 
         var changed  = false
         if (!compareList(newContext, detail.contexts)) {
@@ -108,7 +101,8 @@
         append({"value": "Mobile-Work", "label": i18n.tr("Work Mobile"), "icon": null,
                 "context": QtContacts.ContactDetail.ContextWork, "subType": QtContacts.PhoneNumber.Mobile })
         append({"value": "Other", "label": i18n.tr("Other"), "icon": null,
-                "context": QtContacts.ContactDetail.ContextOther, "subType": -1 })
+                "context": QtContacts.ContactDetail.ContextOther, "subType": QtContacts.PhoneNumber.Landline })
         loaded()
+        ready = true
     }
 }

=== modified file 'tests/autopilot/address_book_app/tests/__init__.py'
--- tests/autopilot/address_book_app/tests/__init__.py	2014-01-10 15:07:11 +0000
+++ tests/autopilot/address_book_app/tests/__init__.py	2014-01-14 20:44:40 +0000
@@ -78,8 +78,6 @@
     def launch_test_installed(self):
         df = "/usr/share/applications/address-book-app.desktop"
         self.ARGS.append("--desktop_file_hint=" + df)
-        print "ARGS:", AddressBookAppTestCase.ARGS
-        print "ENV:", os.environ["ADDRESS_BOOK_TEST_DATA"]
         self.app = self.launch_test_application(
             "address-book-app",
             *AddressBookAppTestCase.ARGS,
@@ -159,6 +157,50 @@
 
         return edit_page
 
+    def set_phone_number(self, idx, phone_number, phone_type=-1):
+        phoneGroup = self.main_window.select_single(
+            "ContactDetailGroupWithTypeEditor",
+            objectName="phones")
+
+        if (idx > 0):
+            self.create_new_detail(phoneGroup)
+
+        phone_number_input = self.main_window.select_single(
+            "TextInputDetail",
+            objectName="phoneNumber_" + str(idx))
+        self.type_on_field(phone_number_input, phone_number)
+
+        if (phone_type != -1):
+            phone_value_selector = self.main_window.select_single(
+                "ValueSelector",
+                objectName="type_phoneNumber_" + str(idx))
+            self.pointing_device.click_object(phone_value_selector)
+            self.select_a_value(phone_number_input,
+                                phone_value_selector,
+                                phone_type)
+
+    def set_email_address(self, idx, email_address, email_type=-1):
+        emailGroup = self.main_window.select_single(
+            "ContactDetailGroupWithTypeEditor",
+            objectName="emails")
+
+        if (idx > 0):
+            self.create_new_detail(emailGroup)
+
+        email_address_input = self.main_window.select_single(
+            "TextInputDetail",
+            objectName="emailAddress_" + str(idx))
+        self.type_on_field(email_address_input, email_address)
+
+        if (email_type != -1):
+            email_value_selector = self.main_window.select_single(
+                "ValueSelector",
+                objectName="type_email_" + str(idx))
+            self.pointing_device.click_object(email_value_selector)
+            self.select_a_value(email_address_input,
+                                email_value_selector,
+                                email_type)
+
     def add_contact(self,
                     first_name,
                     last_name,
@@ -187,26 +229,14 @@
                 "ContactDetailGroupWithTypeEditor",
                 objectName="phones")
             for idx, number in enumerate(phone_numbers):
-                if (idx > 0):
-                    self.create_new_detail(phoneGroup)
-
-                phone_number_input = self.main_window.select_single(
-                    "TextInputDetail",
-                    objectName="phoneNumber_" + str(idx))
-                self.type_on_field(phone_number_input, number)
+                self.set_phone_number(idx, number)
 
         if (email_address):
             emailGroup = self.main_window.select_single(
                 "ContactDetailGroupWithTypeEditor",
                 objectName="emails")
             for idx, address in enumerate(email_address):
-                if (idx > 0):
-                    self.create_new_detail(emailGroup)
-
-                email_address_input = self.main_window.select_single(
-                    "TextInputDetail",
-                    objectName="emailAddress_" + str(idx))
-                self.type_on_field(email_address_input, address)
+                self.set_email_address(idx, address)
 
         if (im_address):
             imGroup = self.main_window.select_single(

=== modified file 'tests/autopilot/address_book_app/tests/test_add_contact.py'
--- tests/autopilot/address_book_app/tests/test_add_contact.py	2013-11-21 12:14:04 +0000
+++ tests/autopilot/address_book_app/tests/test_add_contact.py	2014-01-14 20:44:40 +0000
@@ -279,3 +279,127 @@
             "ContactListView",
             objectName="contactListView")
         self.assertThat(list_view.count, Eventually(Equals(1)))
+
+    def test_email_label_save(self):
+        # execute add new contact
+        self.main_window.open_toolbar().click_button("Add")
+
+        # fill name
+        firstNameField = self.main_window.wait_select_single(
+            "TextInputDetail",
+            objectName="firstName")
+        lastNameField = self.main_window.wait_select_single(
+            "TextInputDetail",
+            objectName="lastName")
+        self.type_on_field(firstNameField, "Sherlock")
+        self.type_on_field(lastNameField, "Holmes")
+
+        # Home
+        self.set_email_address(0, "[e-mail]", 0) 
+        # Work
+        self.set_email_address(1, "[e-mail]", 1)
+        # Other
+        self.set_email_address(2, "[e-mail]", 2)
+
+        # Save contact
+        acceptButton = self.main_window.wait_select_single(
+            "Button",
+            objectName="accept")
+        self.pointing_device.click_object(acceptButton)
+
+        contacts = self.main_window.select_many("ContactDelegate")
+        self.pointing_device.click_object(contacts[0])
+
+        # check if contacts was saved with the correct labels
+        view_page = self.main_window.get_contact_view_page()
+        self.assertThat(view_page.visible, Eventually(Equals(True)))
+
+        # check if we have 3 emails"""
+        email_group = view_page.select_single(
+            "ContactDetailGroupWithTypeView",
+            objectName="emails")
+        self.assertThat(email_group.detailsCount, Eventually(Equals(3)))
+
+        emails = {"[e-mail]" : "Home",
+                  "[e-mail]" : "Work",
+                  "[e-mail]" : "Other"}
+
+        # Check if they have the correct label
+        for idx in range(3):
+            email_type = view_page.select_single(
+                "Label",
+                objectName="type_email_" + str(idx))
+
+            email_label = view_page.select_single(
+                "Label",
+                objectName="label_emailAddress_" + str(idx) + ".0")
+
+            self.assertThat(emails[email_label.text], Equals(email_type.text))
+            del emails[email_label.text]
+
+        self.assertThat(len(emails), Equals(0))
+
+    def test_phone_label_save(self):
+        # execute add new contact
+        self.main_window.open_toolbar().click_button("Add")
+
+        # fill name
+        firstNameField = self.main_window.wait_select_single(
+            "TextInputDetail",
+            objectName="firstName")
+        lastNameField = self.main_window.wait_select_single(
+            "TextInputDetail",
+            objectName="lastName")
+        self.type_on_field(firstNameField, "Sherlock")
+        self.type_on_field(lastNameField, "Holmes")
+
+        # Home
+        self.set_phone_number(0, "00 0000 0000", 0) 
+        # Work
+        self.set_phone_number(1, "11 1111 1111", 1)
+        # Mobile
+        self.set_phone_number(2, "22 2222 2222", 2)
+        # Work Mobile
+        self.set_phone_number(3, "33 3333 3333", 3)
+        # Other
+        self.set_phone_number(4, "44 4444 4444", 4)
+
+        # Save contact
+        acceptButton = self.main_window.wait_select_single(
+            "Button",
+            objectName="accept")
+        self.pointing_device.click_object(acceptButton)
+
+        contacts = self.main_window.select_many("ContactDelegate")
+        self.pointing_device.click_object(contacts[0])
+
+        # check if contacts was saved with the correct labels
+        view_page = self.main_window.get_contact_view_page()
+        self.assertThat(view_page.visible, Eventually(Equals(True)))
+
+        # check if we have five phones"""
+        phone_group = view_page.select_single(
+            "ContactDetailGroupWithTypeView",
+            objectName="phones")
+        self.assertThat(phone_group.detailsCount, Eventually(Equals(5)))
+
+        phones = {"00 0000 0000" : "Home",
+                  "11 1111 1111" : "Work",
+                  "22 2222 2222" : "Mobile", 
+                  "33 3333 3333" : "Work Mobile",
+                  "44 4444 4444" : "Other"}
+
+        # Check if they have the correct label
+        for idx in range(5):
+            phone_type = view_page.select_single(
+                "Label",
+                objectName="type_phoneNumber_" + str(idx))
+
+            phone_label = view_page.select_single(
+                "Label",
+                objectName="label_phoneNumber_" + str(idx) + ".0")
+
+            self.assertThat(phones[phone_label.text], Equals(phone_type.text))
+            del phones[phone_label.text]
+
+        self.assertThat(len(phones), Equals(0))

=== modified file 'tests/autopilot/address_book_app/tests/test_edit_contact.py'
--- tests/autopilot/address_book_app/tests/test_edit_contact.py	2013-11-22 01:05:52 +0000
+++ tests/autopilot/address_book_app/tests/test_edit_contact.py	2014-01-14 20:44:40 +0000
@@ -109,10 +109,10 @@
         self.assertThat(email_group.detailsCount, Eventually(Equals(1)))
 
         # check if the new value is correct
-        phone_label_1 = view_page.select_single(
+        email_label_1 = view_page.select_single(
             "Label",
             objectName="label_emailAddress_0.0")
-        self.assertThat(phone_label_1.text,
+        self.assertThat(email_label_1.text,
                         Eventually(Equals("[e-mail]")))
 
     def test_remove_email(self):