782136 - add carrier field and ContactField interface to Contacts API

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Andreas Gal :gal]

The product teams agreed that they need a carrier field for each phone number of a contact. Jonas has signed off on the change. Gregor will try to finish it by EOD August 13 along with a few other minor changes since this involves a schema change.

Comment 1

[Gregor Wagner [:gwagner]]

I will also add multivalue interface ContactField in this bug.

Comment 2

[Gregor Wagner [:gwagner]]

Attachment: IDL

Comment 3

[Mounir Lamouri (:mounir)]

(In reply to Andreas Gal :gal from comment #0)
> The product teams agreed that they need a carrier field for each phone
> number of a contact. Jonas has signed off on the change. Gregor will try to
> finish it by EOD August 13 along with a few other minor changes since this
> involves a schema change.

Where did that discussion happen? Any link?
I wonder how that would be used? Would that be pre-filled by the phone or filed by the user?

Comment 4

[Daniel Coloma:dcoloma]

All the discussion was held at: https://groups.google.com/forum/?fromgroups#!topic/mozilla.dev.gaia/T-dsKQt3zYs[1-25]

Comment 5

[Gregor Wagner [:gwagner]]

Attachment: Impl patch

Comment 6

[Gregor Wagner [:gwagner]]

Attachment: IDL

Comment 7

[Andreas Gal :gal]

Review ping. This is urgently needed. Other work is blocked on it.

Comment 8

[[:fabrice] Fabrice Desré]

Comment on attachment 651522 [details] [diff] [review]
Impl patch

Review of attachment 651522 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/contacts/fallback/ContactDB.jsm
@@ +127,5 @@
>          // Create new searchable indexes.
>          objectStore.createIndex("email", "search.email", { unique: false, multiEntry: true });
> +      } else if (currVersion == 3) {
> +        debug("upgrade 3");
> + 

nit: whitespace

@@ +216,5 @@
>  
>                // Chop off the first characters
> +              let number = aContact.properties[field][i].value;
> +              if (number) {
> +                for(let i = 0; i < number.length; i++) {

nit: for (...)

Comment 9

[Andreas Gal :gal]

Comment on attachment 651524 [details] [diff] [review]
IDL

stealing, looks good to me, we can always tweak after landing

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 651524 [details] [diff] [review]
IDL

Review of attachment 651524 [details] [diff] [review]:
-----------------------------------------------------------------

r+=me if you change the type to be just a string. Otherwise I'd like to hear why we think there are more cases that will be dealing with importing/exporting vcard data than that will be displaying the type in UI. Anytime we are displaying the type in UI we need a single string. The array is only needed when importing/exporting to vcard.

r=me on adding the carrier field either way.

Comment 11

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/82dba0cf6958
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab0c6dd14478

Comment 12

[Gregor Wagner [:gwagner]]

Forgot the review comments:
https://hg.mozilla.org/integration/mozilla-inbound/rev/01a3cbde459c

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ab0c6dd14478
https://hg.mozilla.org/mozilla-central/rev/82dba0cf6958
https://hg.mozilla.org/mozilla-central/rev/01a3cbde459c