862742 - Import contacts from memory card in VCF (vCard) format

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Contacts, defect)

Description

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

The user should be able to import vcards from the memory card into the Contacts app. Vcard is the standard in contact format and most feature phones export to it. Firefox OS should be able to import these contacts easily from the Contacts app and in the FTU.

Comment 1

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Attachment: Github PR URL

Comment 2

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Attachment: VCF 2.1 example

Comment 3

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Attachment: VCF 3.0 example

Comment 4

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Attachment: VCF 4.0 example

Comment 5

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Attachment: VCF Multiple contacts example

Comment 6

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

You can test easily with real-world examples of multiple VCF by exporting from http://contacts.google.com in case you have a gmail account.

Comment 7

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 738439 [details] [review]
Github PR URL

r+, check the functionality and the look and feel and looks awesome.

Please merge once the rebased and squashed :)

Comment 8

[Francisco Jordano [:arcturus] [:francisco]]

Nominating as leo? since the functionality closes the story of importing contacts.

Comment 9

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Comment on attachment 738439 [details] [review]
Github PR URL

The new pull request rebased and squashed here: https://github.com/mozilla-b2g/gaia/pull/9338

Comment 10

[Jose Manuel Cantera]

I think before landing this code we would need some review by the UX team. Particularly I'm not convinced we should add a new option to the Contacts Settings screen. Perhaps import from vCArd and SDCard is something could be implemented by an independent app. 

In any case I think this needs a review from UX both from an interaction and visual perspective.

Comment 11

[Francisco Jordano [:arcturus] [:francisco]]

(In reply to Jose M. Cantera from comment #10)
> I think before landing this code we would need some review by the UX team.
> Particularly I'm not convinced we should add a new option to the Contacts
> Settings screen. Perhaps import from vCArd and SDCard is something could be
> implemented by an independent app. 
> 
> In any case I think this needs a review from UX both from an interaction and
> visual perspective.

We requested UX feedback to Sergi, as I couldn't find Ayman past friday. Anyway sounds good to me if Ayman takes a look to it.

Thanks,
F.

Comment 12

[Sergi]

I think it looks good as it's currently implemented and makes sense to do it from the contact settings screen. I'm not sure about creating a new app to import contacts, away from the place you manage them, basically the contacts app. Anyway, that's my opinion. You can check it with Ayman.

Comment 13

[Josh Carpenter [:jcarpenter]]

(In reply to Sergi from comment #12)
> I think it looks good as it's currently implemented and makes sense to do it
> from the contact settings screen. I'm not sure about creating a new app to
> import contacts, away from the place you manage them, basically the contacts
> app. Anyway, that's my opinion. You can check it with Ayman.

Agreed. An independent app for a transaction like this is a non-starter. Ayman should review and chime in on this.

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

This sounds more like a nice-to-have, very valuable new feature so we won't block here but will track and consider an uplift nomination to v1-train if low risk and very independent of other code.

Comment 15

[ayman maat :maat]

Hey guys

Ok I have checked this and it looks all good to me.

Ayman

Comment 16

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Comment on attachment 738439 [details] [review]
Github PR URL

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky):
String or UUID changes made by this patch:

Final Github Pull request: https://github.com/mozilla-b2g/gaia/pull/9338

Comment 17

[Julien Wajsberg [:julienw]]

this broke the linter, please fix the errors.

see https://travis-ci.org/mozilla-b2g/gaia/builds/6634027 for example

Next week we'll start backing out patches because of such errors, so take care :)

Also, for your next commits, please put the bug number in the commit log, this is easier to find bugs from the commit hash.

And also put the commit hash in the comments next time, like that :

master: 957057ef791be7c1a30cb127e90915bb0ef98f3a

Comment 18

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Thanks Julien, will do. I am still learning bugzilla bureacracy.

I am working on fixing linter error and will commit asap.

Comment 19

[Julien Wajsberg [:julienw]]

And you should fill in the approval form too ;) otherwise the triagers will reject the approval.

Comment 20

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Attachment: Github PR fixing linter and unit test errors

Comment 21

[Julien Wajsberg [:julienw]]

I've put some comments on the PR already

Comment 22

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

(In reply to Julien Wajsberg [:julienw] from comment #21)
> I've put some comments on the PR already

Thanks, I went through them and updated the PR.

Comment 23

[Julien Wajsberg [:julienw]]

I've put some more comments, all only test-related, Francisco should review what the code is doing because I have no clue ;)

Comment 24

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Comment on attachment 738439 [details] [review]
Github PR URL

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 862742
User impact if declined: Users won't be able to import contacts from SDCard in vCard format. This affects mainly users of feature phones with systems such as Nokia S40, where the main way to extract contacts is through vCard exporting using bluetooth or SDCard. In both cases, importing vCard files from SDCard solves the issue. Countries like Serbia depend highly on vCard-based systems.
Testing completed: Basic UI and import testing included 
Risk to taking this patch (and alternatives if risky): This is a low-risk patch. UI flows are tested and verified.
String or UUID changes made by this patch: Yes. Only in  apps/communications/contacts/locales/contacts.en-US.properties. See https://github.com/mozilla-b2g/gaia/pull/9338/files#diff-6.

Comment 25

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 741921 [details] [review]
Github PR fixing linter and unit test errors

r+

agree with comments by Julien, but ok to do it on a follow up.

Comment 26

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 738439 [details] [review]
Github PR URL

Adding qawanted to get some additional testing around this beyond the basic UI flows - please stress-test this so that we know this (very much needed) additional functionality works well for the user.

Comment 27

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted cbea03490518734515b7e6c1508759768342b18c to:
v1-train: a5fed32b0eddcd1354477c7da66e99f183463fa9

Comment 28

[Julien Wajsberg [:julienw]]

Sergi, please put the master commit for the linter+test fixes here, ask approval because the new patch touches some application code (I still didn't really understand why btw), and add needinfo in john ford to uplift when it gets the approval.

Comment 29

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Comment on attachment 741921 [details] [review]
Github PR fixing linter and unit test errors

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

Request approval for this commit, as per Julien's advice, and because it touches application code

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 862742
User impact if declined: None, but tests will break the build.
Testing completed: Yes.
Risk to taking this patch (and alternatives if risky): No risk.
String or UUID changes made by this patch: None.

Comment 30

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Comment on attachment 741921 [details] [review]
Github PR fixing linter and unit test errors

master for the fix: https://github.com/mozilla-b2g/gaia/commit/a75b3731696570c2851522758dd1653fcaebcb96

Comment 31

[Julien Wajsberg [:julienw]]

need info John for uplifting the test fix

Comment 32

[John Ford [:jhford] CET/CEST Berlin Time]

There is a small merge conflict when I try to uplift this, which looks easy to resolve, but I'd like to make sure that it's OK:

++<<<<<<< HEAD
 +MockFb.getWorksAt = function(fbData) {
 +  return 'Telef<97>nica';
++=======
+ Mockfb.getWorksAt = function(fbData) {
+   return 'Telefónica';
++>>>>>>> a75b373... Merge pull request #9413 from comoyo/fix_sdcard_tests

Comment 33

[Julien Wajsberg [:julienw]]

John, I think that's beause Bug 851476 was not uplifted. Could you do it as it's npotb ?
(and I sure hope it will be uplifted without conflicts)

Comment 34

[John Ford [:jhford] CET/CEST Berlin Time]

Follow up v1-train: 0d624b6c55094eaaa26318e1a3424cb8137f1580

Comment 35

[Tony Chung [:tchung]]

Sreenidhi to add a few moztrap testcases for this feature.

Comment 36

[John Hammink]

I actually tested this case in leorun4, with all of the VCF contact versions.  

In my tests, All contact types are importing OK.

Comment 37

[Salem Elkabule]

This bug no longer reproduces on buri 1.1 
Environmental Variables 
Device: Buri 1.1 MOZ
BuildID: 20140102041202
Gaia: 6ff3a607f873320d00cb036fa76117f6fadd010f
Gecko: bdac595a4e46
Version: 18.0