[Merge] lp:~renatofilho/address-book-app/fix-1371243 into lp:~phablet-team/address-book-app/staging

[Michał Karnicki]

Review: Needs Information

Left some in-line questions. Marking as "needs information" only because I have questions, since I'm not familiar with the code base.

Diff comments:

> === modified file 'src/imports/ContactList/ContactExporter.qml'
> --- src/imports/ContactList/ContactExporter.qml	2014-05-06 13:18:07 +0000
> +++ src/imports/ContactList/ContactExporter.qml	2014-09-30 20:22:25 +0000
> @@ -20,15 +20,15 @@
>  Item {
>      id: root
>  
> -    property var contacts: []
>      property var contactModel
>      property var outputFile
>  
> -    signal completed(int error)
> +    signal contactExported(int error, string url)
> +    signal contactFetched(var contacts)

If this may be more than one, I would name the signal contactsFetched. (possibly, the one earlier, contactsExported, too).

>  
> -    function start() {
> +    function start(contacts) {
>          if (!contactModel) {
> -            console.log("No contact model defined")
> +            console.error("No contact model defined")
>              return

This is JS. No ; at the end of lines?

>          }
>  
> @@ -48,6 +48,7 @@
>              priv.currentQueryId = contactModel.fetchContacts(ids)
>          }
>      }
> +
>      Item {
>          id: priv
>  
> @@ -58,15 +59,18 @@
>  
>              onExportCompleted: {
>                  priv.currentQueryId = -1
> -                root.completed(error)
> +                root.contactExported(error, root.outputFile)
>              }
>  
>              onContactsFetched: {
>                  // currentQueryId == -2 is used during a fetch using "memory" manager
>                  if ((priv.currentQueryId == -2) || (requestId == priv.currentQueryId)) {
> -                    root.contactModel.exportContacts(root.outputFile,
> -                                                     [],
> -                                                     fetchedContacts)
> +                    if (root.outputFile !== "") {
> +                        root.contactModel.exportContacts(root.outputFile,
> +                                                         [],
> +                                                         fetchedContacts)
> +                    }
> +                    root.contactFetched(fetchedContacts)
>                  }
>              }
>          }
> 
> === modified file 'src/imports/ContactList/ContactListPage.qml'
> --- src/imports/ContactList/ContactListPage.qml	2014-09-18 23:08:25 +0000
> +++ src/imports/ContactList/ContactListPage.qml	2014-09-30 20:22:25 +0000
> @@ -463,7 +463,7 @@
>                              contactList.selectAll()
>                          }
>                      }
> -                    visible: contactList.isInSelectionMode
> +                    visible: contactList.multipleSelection
>                  },
>                  Action {
>                      objectName: "share"
> @@ -478,13 +478,7 @@
>                              contacts.push(items.get(i).model.contact)
>                          }
>  
> -                        if (mainPage.pickMode) {
> -                            contactExporter.exportContacts(contacts)
> -                            mainPage.pickMode = false
> -                        } else {
> -                            pageStack.push(Qt.resolvedUrl("../ContactShare/ContactSharePage.qml"),
> -                                           { contactModel: contactList.listModel, contacts: contacts })
> -                        }
> +                        contactExporter.start(contacts)
>                          contactList.endSelection()
>                      }
>                  },
> @@ -628,39 +622,41 @@
>          }
>      }
>  
> -    QtObject {
> +    ContactExporter {
>          id: contactExporter
>  
>          property var activeTransfer: null
>          readonly property bool active: activeTransfer && (activeTransfer.state === ContentHub.ContentTransfer.InProgress && activeTransfer.direction === ContentHub.ContentTransfer.Import)
>          readonly property bool isMultiple: activeTransfer && (activeTransfer.selectionType === ContentHub.ContentTransfer.Multiple)
>  
> -        function exportContacts(contacts)
> -        {
> -            if (activeTransfer) {
> -                var exportUrl = "file:///tmp/address_book_app_export.vcf"
> -                mainPage.contactModel.exportCompleted.connect(contactExporter.onExportCompleted)
> -                mainPage.contactModel.exportContacts(exportUrl, [], contacts)
> -            } else {
> -                console.error("Export requested with noo active transfer")
> -            }
> -        }
> -
> -        function onExportCompleted(error, url)
> -        {
> -            mainPage.contactModel.exportCompleted.disconnect(contactExporter.onExportCompleted)
> +        contactModel: contactList.listModel
> +        outputFile: mainPage.pickMode ? "file:///tmp/address_book_app_export.vcf" : ""
> +        onContactExported: {
> +            // send contacts back to source app (pick mode)
>              if (error === ContactModel.ExportNoError) {
>                  var obj = Qt.createQmlObject("import Ubuntu.Content 0.1;  ContentItem { url: '" + url + "' }", contactExporter)
> -                activeTransfer.items = [obj]
> -                activeTransfer.state = ContentHub.ContentTransfer.Charged
> +                if (activeTransfer) {
> +                    activeTransfer.items = [obj]
> +                    activeTransfer.state = ContentHub.ContentTransfer.Charged
> +                }  else {
> +                    console.error("No active transfer")
> +                }
>              } else {
>                  console.error("Fail to export contacts:" + error)
>              }
>              activeTransfer = null
> -            pickMode = false
> +            mainPage.pickMode = false
>              mainPage.state = "defautl"
>              application.returnVcard(url)
>          }
> +
> +        onContactFetched: {
> +            // Share contacts to an application chosen by the user
> +            if (!mainPage.pickMode) {
> +                pageStack.push(Qt.resolvedUrl("../ContactShare/ContactSharePage.qml"),
> +                               { contactModel: contactExporter.contactModel, contacts: contacts })
> +            }
> +        }
>      }
>  
>      Component.onCompleted: {
> 
> === modified file 'src/imports/ContactShare/ContactSharePage.qml'
> --- src/imports/ContactShare/ContactSharePage.qml	2014-07-24 12:22:13 +0000
> +++ src/imports/ContactShare/ContactSharePage.qml	2014-09-30 20:22:25 +0000
> @@ -35,7 +35,7 @@
>          onPeerSelected: {
>              picker.curTransfer = peer.request();
>              if (picker.curTransfer.state === ContentHub.ContentTransfer.InProgress) {
> -                var vCardUrl = "file:///tmp/vcard_" + encodeURIComponent(contact.contactId) + ".vcf"
> +                var vCardUrl = "file:///tmp/vcard_" + encodeURIComponent(picker.contacts[0].contactId) + ".vcf"

This looks like passing a single contact id here, is that the case?

>                  picker.contactModel.exportContacts(vCardUrl, [], picker.contacts)
>              }
>          }
> 


-- 
https://code.launchpad.net/~renatofilho/address-book-app/fix-1371243/+merge/235708
Your team Ubuntu Phablet Team is subscribed to branch lp:~phablet-team/address-book-app/staging.