891723 - [User Story] Preloaded Contacts Runtime Customization by SIM

Status: VERIFIED FIXED

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

Description

[Peter Dolanjski [:pdol]]

User Story:

As an OEM, I want to be able to specify which contacts should be preloaded on the device based on the MNC/MCC setting from the SIM card inserted during the First Run Experience in order to target customizations to locales without needing to use separate builds.


Acceptance Criteria:

1. If a certain preloaded contacts are specified to be included for an MNC/MCC combination, and a SIM card with that MNC/MCC combination is in the device during the First Run Experience, the Contacts app shows the specified preloaded contacts.
2. If no SIM card is inserted during the First Run Experience, the specified default preloaded contacts are displayed in the Contacts app.
3. If no SIM card is inserted during the First Run Experience, and no default preloaded contacts are specified, no preloaded contacts are displayed in the Contacts app.

Comment 1

[Aus Lacroix [:aus]]

Here's a little preview of the helper object for operator variants.

https://github.com/nullaus/gaia/compare/mozilla-b2g:master...market-customizations

Comment 2

[Peter Dolanjski [:pdol]]

After further partner discussion, the scenario where no SIM is used during First Run, but inserted after the fact needs to be considered:

As an OEM, I want to be able to specify which contacts should be preloaded on the device based on the MNC/MCC setting from the SIM card inserted during the First Run Experience or when the SIM card is inserted for the first time in order to target customizations to locales without needing to use separate builds.

Also, a clarification on the Acceptance Criteria: the display of the preloaded contacts is not just within the Contacts app, but anywhere device contacts are shown (Call log, adding a contact to SMS, etc.).

Lastly, one additional acceptance criteria:
4. If no SIM card is inserted during the First Run Experience, the first time a SIM card is inserted, the contacts, as per acceptance criteria 1, are now added to any existing contacts.

Comment 3

[Alex Keybl [:akeybl]]

koi+ per https://wiki.mozilla.org/B2G/Roadmap

Comment 4

[Al Tsai [:atsai]]

Change whiteboard for aligning the uid on the User story backlog.

Comment 5

[Al Tsai [:atsai]]

MozTrap #9550, #9551, #9552, #9553, #9554

Comment 6

[Candice Serran (:cserran)]

Peter Dolanjski changed story state to started in Pivotal Tracker

Comment 7

[Reuben Morais [:reuben]]

Attachment: Remove default contacts logic from ContactDB

Comment 8

[Reuben Morais [:reuben]]

Attachment: Gaia PR, v2

Comment 9

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

Hi,

this bug is intimate related to bug 891729, what I'm suggesting in both is having a common way of load any default information per mcc/mnc ... 

In that bug I suggest to create a single file by mcc/mnc and share the common things over there.

For that we can create the infrastructure to read from the correct file (with a single format) and then used in different places, also will put all the specific configuration on a single file, but separated or at least well modularized from the reading of those settings.

@Marina, you and @Reuben coordinate to have a common approach?

Comment 10

[Reuben Morais [:reuben]]

Francisco, having a shared customization data loading code would be great, but how strongly do you feel about having a separate file per mcc/mnc pair? I don't see what the advantages are compared to what Marina and I have, and it's different from what's done in our existing customization support code (one file/folder per type of customization).

Comment 11

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

(In reply to Reuben Morais [:reuben] from comment #10)
> Francisco, having a shared customization data loading code would be great,
> but how strongly do you feel about having a separate file per mcc/mnc pair?
> I don't see what the advantages are compared to what Marina and I have, and
> it's different from what's done in our existing customization support code
> (one file/folder per type of customization).

Hi Reuben, my main concern is we start using this file adding base64 binaries or seeing this file grow like crazy and loading a lot of information that we won't need at all.

In the case of same file for different configurations, pointing to the same file will solve the problem, but if we think in Latam, where we could have up to 10 different countries we will be loading extra information that is not needed.

Cheers.

Comment 12

[Candice Serran (:cserran)]

Reuben is this story blocked? or does the needinfo status need to be updated (as I see Fernando's response) ?

Comment 13

[Gregor Wagner [:gwagner]]

(In reply to Francisco Jordano [:arcturus] from comment #11)
> (In reply to Reuben Morais [:reuben] from comment #10)
> > Francisco, having a shared customization data loading code would be great,
> > but how strongly do you feel about having a separate file per mcc/mnc pair?
> > I don't see what the advantages are compared to what Marina and I have, and
> > it's different from what's done in our existing customization support code
> > (one file/folder per type of customization).
> 
> Hi Reuben, my main concern is we start using this file adding base64
> binaries or seeing this file grow like crazy and loading a lot of
> information that we won't need at all.

Where do you see the base64 strings coming from? Pics for contacts?

Comment 14

[Reuben Morais [:reuben]]

This is not blocked, no.

Comment 15

[Gregor Wagner [:gwagner]]

This doesn't work if we have a SIM lock. We have to wait until we unlock the sim.

Comment 16

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

(In reply to Gregor Wagner [:gwagner] from comment #13)
> (In reply to Francisco Jordano [:arcturus] from comment #11)
> > (In reply to Reuben Morais [:reuben] from comment #10)
> > > Francisco, having a shared customization data loading code would be great,
> > > but how strongly do you feel about having a separate file per mcc/mnc pair?
> > > I don't see what the advantages are compared to what Marina and I have, and
> > > it's different from what's done in our existing customization support code
> > > (one file/folder per type of customization).
> > 
> > Hi Reuben, my main concern is we start using this file adding base64
> > binaries or seeing this file grow like crazy and loading a lot of
> > information that we won't need at all.
> 
> Where do you see the base64 strings coming from? Pics for contacts?

Pics for contacts is an example, also (despite that we can use a file perfectly) wallpapers maybe?

My point here is, let's load just what we need.

Thanks Gregor!

Comment 17

[Gregor Wagner [:gwagner]]

(In reply to Francisco Jordano [:arcturus] from comment #16)
> (In reply to Gregor Wagner [:gwagner] from comment #13)
> > (In reply to Francisco Jordano [:arcturus] from comment #11)
> > > (In reply to Reuben Morais [:reuben] from comment #10)
> > > > Francisco, having a shared customization data loading code would be great,
> > > > but how strongly do you feel about having a separate file per mcc/mnc pair?
> > > > I don't see what the advantages are compared to what Marina and I have, and
> > > > it's different from what's done in our existing customization support code
> > > > (one file/folder per type of customization).
> > > 
> > > Hi Reuben, my main concern is we start using this file adding base64
> > > binaries or seeing this file grow like crazy and loading a lot of
> > > information that we won't need at all.
> > 
> > Where do you see the base64 strings coming from? Pics for contacts?
> 
> Pics for contacts is an example, also (despite that we can use a file
> perfectly) wallpapers maybe?
> 
> My point here is, let's load just what we need.
> 
> Thanks Gregor!

Wallpapers will be handled in a different way. We don't change the way we do it currently. The wallpapers/ringtones live in one folder and the customization just picks one by the file name.
There are no custom wallpapers for one country.

Comment 18

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

(In reply to Gregor Wagner [:gwagner] from comment #17)
> (In reply to Francisco Jordano [:arcturus] from comment #16)
> > (In reply to Gregor Wagner [:gwagner] from comment #13)
> > > (In reply to Francisco Jordano [:arcturus] from comment #11)
> > > > (In reply to Reuben Morais [:reuben] from comment #10)
> > > > > Francisco, having a shared customization data loading code would be great,
> > > > > but how strongly do you feel about having a separate file per mcc/mnc pair?
> > > > > I don't see what the advantages are compared to what Marina and I have, and
> > > > > it's different from what's done in our existing customization support code
> > > > > (one file/folder per type of customization).
> > > > 
> > > > Hi Reuben, my main concern is we start using this file adding base64
> > > > binaries or seeing this file grow like crazy and loading a lot of
> > > > information that we won't need at all.
> > > 
> > > Where do you see the base64 strings coming from? Pics for contacts?
> > 
> > Pics for contacts is an example, also (despite that we can use a file
> > perfectly) wallpapers maybe?
> > 
> > My point here is, let's load just what we need.
> > 
> > Thanks Gregor!
> 
> Wallpapers will be handled in a different way. We don't change the way we do
> it currently. The wallpapers/ringtones live in one folder and the
> customization just picks one by the file name.
> There are no custom wallpapers for one country.

We have bug 891729, that, IMHO, shares too many things, like loading from a dictionary based on mnc/mcc configurable things, contacts, wallpaper, ringtones and so on. That's the part that I would like to be sync, the specific implementation for adding those contacts is not what I'm commenting.

Does that make sense for you?

Comment 19

[Albert [:albert]]

(In reply to Francisco Jordano [:arcturus] from comment #18)
> (In reply to Gregor Wagner [:gwagner] from comment #17)
> > (In reply to Francisco Jordano [:arcturus] from comment #16)
> > > (In reply to Gregor Wagner [:gwagner] from comment #13)
> > > > (In reply to Francisco Jordano [:arcturus] from comment #11)
> > > > > (In reply to Reuben Morais [:reuben] from comment #10)
> > > > > > Francisco, having a shared customization data loading code would be great,
> > > > > > but how strongly do you feel about having a separate file per mcc/mnc pair?
> > > > > > I don't see what the advantages are compared to what Marina and I have, and
> > > > > > it's different from what's done in our existing customization support code
> > > > > > (one file/folder per type of customization).
> > > > > 
> > > > > Hi Reuben, my main concern is we start using this file adding base64
> > > > > binaries or seeing this file grow like crazy and loading a lot of
> > > > > information that we won't need at all.
> > > > 
> > > > Where do you see the base64 strings coming from? Pics for contacts?
> > > 
> > > Pics for contacts is an example, also (despite that we can use a file
> > > perfectly) wallpapers maybe?
> > > 
> > > My point here is, let's load just what we need.
> > > 
> > > Thanks Gregor!
> > 
> > Wallpapers will be handled in a different way. We don't change the way we do
> > it currently. The wallpapers/ringtones live in one folder and the
> > customization just picks one by the file name.
> > There are no custom wallpapers for one country.
> 
> We have bug 891729, that, IMHO, shares too many things, like loading from a
> dictionary based on mnc/mcc configurable things, contacts, wallpaper,
> ringtones and so on. That's the part that I would like to be sync, the
> specific implementation for adding those contacts is not what I'm commenting.
> 
> Does that make sense for you?

I'm agree with Reuben, I think the json should be only a configuration file, it shouldn't contain the resources (ringtones, wallpapers, etc.) in base64. The approach proposed by Francisco, currently implemented in bug 891729, means to have one JSON for each MCC/MNC and a one global JSON to know the groups of MCC/MNC that should be configured. Having only one JSON with links to the resources seems a better option.

Comment 20

[Reuben Morais [:reuben]]

Comment on attachment 799138 [details]
Gaia PR, v2

By the powers of Bugzilla I morph this patch into v2, just pushed to Github.

Comment 21

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

Comment on attachment 799138 [details]
Gaia PR, v2

Codewise looking perfect but unit test are not valid.

Please merge once unit test are fixed.

Take into account that |window.mozContact| has to be mocked as well. Also I guess we will need to mock the VariantManager to return the information (dispatch the event) that we want.

A part from that looking pretty good.

Thanks!

Comment 22

[Reuben Morais [:reuben]]

Okay, updated the PR, I'll be in classes for the next couple of hours, can someone please merge if Travis looks green? Thanks!

Comment 23

[Reuben Morais [:reuben]]

I'm going to leave the Gecko patch unlanded for now, since ftu/js/variant.js doesn't support loading the '000000' customization when no SIM is found.

https://github.com/mozilla-b2g/gaia/commit/84e2d96144ce9e94cc1bd2f8da6a1d2f97c39181

Comment 24

[James Lal [:lightsofapollo]]

perma-red travis failures... backed out here: https://github.com/mozilla-b2g/gaia/commit/c3dcd3a48aee93f0cba7ec17509b61a4963e1701

Comment 25

[Reuben Morais [:reuben]]

This passes locally using a debug profile + "http://test-agent.gaiamobile.org", and passes locally using the test-agent-server/test-agent-test mechanism. I have no idea what's going on with Travis.

Comment 26

[Borja Salguero [:borjasalguero]]

Reuben, as the format is under discussion, could we wait for re-landing this patch? Probably we could include the changes directly here. Let me know if you agree! Thanks :)

Comment 27

[Borja Salguero [:borjasalguero]]

Comment on attachment 799138 [details]
Gaia PR, v2

Adding a new review due to the discussion related with https://bugzilla.mozilla.org/show_bug.cgi?id=917740

Comment 28

[Antonio Manuel Amaya Calvo (:amac)]

Sorry for piping in so late here but, why are we removing the loading of the default contacts list from Gecko? In fact, why do we even need a Gaia patch here?

The way it was working before (with a single contacts.json) made more sense to me since it did separate the initial contact populating logic from the frontend completely. So the frontend could be completely replaced and still the initial DB (which is a core part of the product) would still be correctly propagated.

Wouldn't it make more sense here to modify the code at http://mxr.mozilla.org/mozilla-central/source/dom/contacts/fallback/ContactDB.jsm#120 so the loading is deferred until we have a valid mcc-mnc and the file we load is determined by the mcc-mnc configuration? We wouldn't need a Gaia patch at all and it would work with the next evolution of the UI as well as with this one :).

Comment 29

[Reuben Morais [:reuben]]

(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #28)
> Sorry for piping in so late here but, why are we removing the loading of the
> default contacts list from Gecko? In fact, why do we even need a Gaia patch
> here?

Notice that the Gecko logic is still there, though it isn't SIM aware. All our user stories are centered on the FTU app. Gaia already has logic in place to launch the FTU app at the right time, and centralizing SIM customizations means we can share more code, build system machinery, etc.

> The way it was working before (with a single contacts.json) made more sense
> to me since it did separate the initial contact populating logic from the
> frontend completely. So the frontend could be completely replaced and still
> the initial DB (which is a core part of the product) would still be
> correctly propagated.

Notice that the customization is permanent, we make no effort to remove imported contacts when the SIM is removed or anything like that. You could also argue that market customization stuff is a front-end feature.

Comment 30

[Gregor Wagner [:gwagner]]

(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #28)
> Sorry for piping in so late here but, why are we removing the loading of the
> default contacts list from Gecko? In fact, why do we even need a Gaia patch
> here?

Adding contacts based on the locale shouldn't be within the API implementation and within the API.
Gaia should use the API to add these contacts.

> 
> The way it was working before (with a single contacts.json) made more sense
> to me since it did separate the initial contact populating logic from the
> frontend completely. So the frontend could be completely replaced and still
> the initial DB (which is a core part of the product) would still be
> correctly propagated.

No I argue for the opposite. This is a frontend feature and not a backend feature.
We would never be able to put this feature in any standard.

> 
> Wouldn't it make more sense here to modify the code at
> http://mxr.mozilla.org/mozilla-central/source/dom/contacts/fallback/
> ContactDB.jsm#120 so the loading is deferred until we have a valid mcc-mnc
> and the file we load is determined by the mcc-mnc configuration? We wouldn't
> need a Gaia patch at all and it would work with the next evolution of the UI
> as well as with this one :).

This code will eventually go away if we use the datastore approach. So we need a frontend solution anyways.

Comment 31

[Antonio Manuel Amaya Calvo (:amac)]

(In reply to Gregor Wagner [:gwagner] from comment #30)
> (In reply to Antonio Manuel Amaya Calvo (:amac) from comment #28)
> > Sorry for piping in so late here but, why are we removing the loading of the
> > default contacts list from Gecko? In fact, why do we even need a Gaia patch
> > here?
> 
> Adding contacts based on the locale shouldn't be within the API
> implementation and within the API.
> Gaia should use the API to add these contacts.

> > 
> > The way it was working before (with a single contacts.json) made more sense
> > to me since it did separate the initial contact populating logic from the
> > frontend completely. So the frontend could be completely replaced and still
> > the initial DB (which is a core part of the product) would still be
> > correctly propagated.
> 
> No I argue for the opposite. This is a frontend feature and not a backend
> feature.
> We would never be able to put this feature in any standard.

It can be on the platform and not be part of the API per se. I wasn't proposing adding the way to load the default contact lists to the API definition. This is akin to providing a pre-created DB at build time (in fact I had to check it out because I wasn't sure if we were creating the initial contacts DB at build time or at runtime). So it's just *one* way (and nothing to do with any standard) to get a initial load up on the DB. It's an implementation detail, not a API detail.

To me it makes more sense having this as a backend feature because the frontend can change dramatically (will change and in fact some operators might even change it for their branded phones) and having it on the frontend means having to port this importing of data to all the UIs that want to have some contact data preloaded... which doesn't have anything to do with the UI.

Now, personalizing wallpapers, intro videos, ringtones... those *are* part of the UI and as such they belong into the frontend. But personalizing/loading a initial data set on a database... IMO, that shouldn't.

> > Wouldn't it make more sense here to modify the code at
> > http://mxr.mozilla.org/mozilla-central/source/dom/contacts/fallback/
> > ContactDB.jsm#120 so the loading is deferred until we have a valid mcc-mnc
> > and the file we load is determined by the mcc-mnc configuration? We wouldn't
> > need a Gaia patch at all and it would work with the next evolution of the UI
> > as well as with this one :).
> 
> This code will eventually go away if we use the datastore approach. So we
> need a frontend solution anyways.

Wherever the data are stored (and unless they're stored at a particular app datastore, which would break the API I think) they can be preloaded by the backend anyway... and make it independent on any frontend we might want to use there.

Comment 32

[Gregor Wagner [:gwagner]]

(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #31)
> (In reply to Gregor Wagner [:gwagner] from comment #30)
> > (In reply to Antonio Manuel Amaya Calvo (:amac) from comment #28)
> > > Sorry for piping in so late here but, why are we removing the loading of the
> > > default contacts list from Gecko? In fact, why do we even need a Gaia patch
> > > here?
> > 
> > Adding contacts based on the locale shouldn't be within the API
> > implementation and within the API.
> > Gaia should use the API to add these contacts.
> 
> > > 
> > > The way it was working before (with a single contacts.json) made more sense
> > > to me since it did separate the initial contact populating logic from the
> > > frontend completely. So the frontend could be completely replaced and still
> > > the initial DB (which is a core part of the product) would still be
> > > correctly propagated.
> > 
> > No I argue for the opposite. This is a frontend feature and not a backend
> > feature.
> > We would never be able to put this feature in any standard.
> 
> It can be on the platform and not be part of the API per se. I wasn't
> proposing adding the way to load the default contact lists to the API
> definition. This is akin to providing a pre-created DB at build time (in
> fact I had to check it out because I wasn't sure if we were creating the
> initial contacts DB at build time or at runtime). So it's just *one* way
> (and nothing to do with any standard) to get a initial load up on the DB.
> It's an implementation detail, not a API detail.
> 
> To me it makes more sense having this as a backend feature because the
> frontend can change dramatically (will change and in fact some operators
> might even change it for their branded phones) and having it on the frontend
> means having to port this importing of data to all the UIs that want to have
> some contact data preloaded... which doesn't have anything to do with the UI.
> 
> Now, personalizing wallpapers, intro videos, ringtones... those *are* part
> of the UI and as such they belong into the frontend. But
> personalizing/loading a initial data set on a database... IMO, that
> shouldn't.




> 
> > > Wouldn't it make more sense here to modify the code at
> > > http://mxr.mozilla.org/mozilla-central/source/dom/contacts/fallback/
> > > ContactDB.jsm#120 so the loading is deferred until we have a valid mcc-mnc
> > > and the file we load is determined by the mcc-mnc configuration? We wouldn't
> > > need a Gaia patch at all and it would work with the next evolution of the UI
> > > as well as with this one :).
> > 
> > This code will eventually go away if we use the datastore approach. So we
> > need a frontend solution anyways.
> 
> Wherever the data are stored (and unless they're stored at a particular app
> datastore, which would break the API I think) they can be preloaded by the
> backend anyway... and make it independent on any frontend we might want to
> use there.

So we have a working patch here that already had an r+.
If you request to rewrite the patch and take a new approach we have to move this feature to 1.3.
I am strictly against moving this logic into the contacts DB and I won't r+ any patch that does this. We have an API for contacts and if we want to add contacts we should use the API and not load data directly into the DB. This breaks all our sanity checks and it needs a much more complex patch and a whole set of new tests.

Comment 33

[Antonio Manuel Amaya Calvo (:amac)]

(In reply to Gregor Wagner [:gwagner] from comment #32)
> 
> So we have a working patch here that already had an r+.
> If you request to rewrite the patch and take a new approach we have to move
> this feature to 1.3.
> I am strictly against moving this logic into the contacts DB and I won't r+
> any patch that does this. We have an API for contacts and if we want to add
> contacts we should use the API and not load data directly into the DB. This
> breaks all our sanity checks and it needs a much more complex patch and a
> whole set of new tests.

I agree we should use the API to add contacts. What we're doing here, though, isn't adding contacts. It's preloading them, as in creating an initial database state. We can use the 'add' operation as a syntax to create that initial database, as in having a default database. In fact, for single operator variant devices, that initial database could even have been created at build time, without any code on the device. There's nothing on the semantics of what we want to do (have a initial database state depending on the SIM that's present at boot time) that actually has anything to do with the frontend. Keeping it on the frontend just make it worse for future customizations of the UI, cause they'll have to take care of keeping things that really aren't UI. We also have an API to install apps, and yet we ship the devices with an app database already created, we don't use that API to add the initial apps, nor even the privileged ones, for example.

Still, 'con la Iglesia hemos topado, amigo Sancho'. Or if you want to be a purist that would be 'con la iglesia hemos dado, Sancho' :P. With that set of preconditions (you don't really want to hear about changing this and even if we provided a patch you would veto it anyway) I think this is a no productive argument. I'm not going to propose changing this (or even changing it myself) when I know beforehand it will be shot down when it's done. So in Gaia it goes :P.

Comment 34

[Gregor Wagner [:gwagner]]

(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #33)
> (In reply to Gregor Wagner [:gwagner] from comment #32)
> > 
> > So we have a working patch here that already had an r+.
> > If you request to rewrite the patch and take a new approach we have to move
> > this feature to 1.3.
> > I am strictly against moving this logic into the contacts DB and I won't r+
> > any patch that does this. We have an API for contacts and if we want to add
> > contacts we should use the API and not load data directly into the DB. This
> > breaks all our sanity checks and it needs a much more complex patch and a
> > whole set of new tests.
> 
> I agree we should use the API to add contacts. What we're doing here,
> though, isn't adding contacts. It's preloading them, as in creating an
> initial database state. We can use the 'add' operation as a syntax to create
> that initial database, as in having a default database. In fact, for single
> operator variant devices, that initial database could even have been created
> at build time, without any code on the device. There's nothing on the
> semantics of what we want to do (have a initial database state depending on
> the SIM that's present at boot time) that actually has anything to do with
> the frontend. Keeping it on the frontend just make it worse for future
> customizations of the UI, cause they'll have to take care of keeping things
> that really aren't UI. We also have an API to install apps, and yet we ship
> the devices with an app database already created, we don't use that API to
> add the initial apps, nor even the privileged ones, for example.
> 
> Still, 'con la Iglesia hemos topado, amigo Sancho'. Or if you want to be a
> purist that would be 'con la iglesia hemos dado, Sancho' :P. With that set
> of preconditions (you don't really want to hear about changing this and even
> if we provided a patch you would veto it anyway) I think this is a no
> productive argument. I'm not going to propose changing this (or even
> changing it myself) when I know beforehand it will be shot down when it's
> done. So in Gaia it goes :P.

Alright so what are the options we have here:
1) Preload the contacts in ContactDB.jsm.
I believe that the contacts DB should be reusable and replaceable. It should be a generic implementation for desktop, phones, tablets, watches, gaming consoles, TVs... If we load contacts based on information fromt he SIM card we have to add complex code that deals with country and network codes. And why should my contacts DB care if we have a sim lock or not? Most of the devices I mentioned earlier don't even have a SIM card so we would have to make the import functionality replacable and this means we have to expose it.

The current import function is just for testing purposes and for developers that wanted to add contacts when we didn't have vcard, facebook.... etc import. In order to make this import function robust enough for partners we have to add a lot of complexity. Currently we totally brick the phone if the file with the contacts has a syntax error.
It would also delay our first startup time quite a bit because we have to wait for SD card access.
Furthermore we are likely to remove this file for the datastore approach for 1.3 so we would have to re-implement it in gaia anyways.

I also have other points that I don't want to discuss in full length here: we would couple the json format with the internal DB format and when we do a DB schema upgrade for new releases we would also have to update the contacts file in order to support a phone swipe.

So it adds a whole lot of dependencies that we don't need.

2) We could move the functionality to ContactsService.jsm which also runs in the parent.
This would be doable with a flag that indicates the very first run. But we would have to copy a few hundred lines that deals with sanitizing data from the ContactManager.js to the parent and keep it in sync with it.
This also goes away with datastore.

3) We could move it to ContactManager.js, avoid the huge code duplication and use the existing sensitization code in the child. The child implementation has no idea if we use the ContactDB the very first time and we would have to add an ugly hack for it.

4) My favorite is a customization service that lives in the system app and deals with customizations. It runs every time we startup the phone and it can deal with one-time customizations and customizations when we switch the sim card. This has the problem of DBs that live within other apps like the we have for the browser. But with the datastore approach this becomes less of a problem because most DBs move into the system app. Maybe we can do this for 1.3.

5) The current code does the customization within FTU. It works and I think it's a good place for it. We don't delay the already horrible slow first startup any longer, we can deal with SIM locks at a unified place and we already do other customizations there. We also avoid all the complications and code duplications and we can add the feature with a few lines instead of a few hundred lines that we most likely would have to remove with 1.3 anyways.

Comment 35

[Jose Manuel Cantera]

Comment on attachment 799138 [details]
Gaia PR, v2

Borja is on PTO today, so taking over.

Comment 36

[Borja Salguero [:borjasalguero]]

Comment on attachment 799138 [details]
Gaia PR, v2

Restoring! Not PTO for me today :(

Comment 37

[Borja Salguero [:borjasalguero]]

Reuben, please could you update this with a new PR to review? We would like to land this today, and we are quite close to land this! Please re-upload your patch, and we will take a look to the details and tests. THanks!

Comment 38

[Borja Salguero [:borjasalguero]]

Comment on attachment 799138 [details]
Gaia PR, v2

This patch is obsolete, please upload a new one.

Comment 39

[Reuben Morais [:reuben]]

Attachment: Gaia PR, v3

Comment 40

[Borja Salguero [:borjasalguero]]

Comment on attachment 807701 [details]
Gaia PR, v3

R+. Small modifications about how to load the array will be addressed here:
https://bugzilla.mozilla.org/show_bug.cgi?id=917740

Thanks!

Comment 41

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

Uplifted 3610f985413ba8c12f721a937eddbca3468cca1e to:
v1.2: 7b6147372cbf560744a02be50e0a862a825caef6

Comment 42

[Jason Smith [:jsmith]]

Reuben - I'm trying to follow the patch to understand how I can the reference customization up to date to understand how to test this. Can you provide a sample contacts.json demonstrating how I would customize preloaded contacts by SIM?

Comment 43

[Candice Serran (:cserran)]

Gregor Wagner changed story state to finished in Pivotal Tracker

Comment 44

[Jason Smith [:jsmith]]

Reuben clarified this in IRC. He says I need to generate a file with <MCC>-<MNC>.json that points to other relevant files. Something like:

214-02.json which contains:

{
   "default_contacts" : "contacts.json"
}

That should result in preloading contacts for the MCC = 214, MNC = 02 SIM.

Comment 45

[Reuben Morais [:reuben]]

214-002.json, and for now it's

{
    "default_contacts": [{givenName: ["Foo"]}, {familyName: ["Bar"]}, etc…]
}

It'll point to a JSON file after bug 917740 is fixed.

Comment 46

[Jason Smith [:jsmith]]

Tried testing against the repo here - https://github.com/jds2501/gaia-distribution-sample/tree/simcontacts. No luck though - I got the default customization here, not the sim-based customization when I had the MCC/MNC match my SIM. Any ideas?

FWIW - the SIM was recognized with MCC 310, MNC 260, as the bookmark customization did end up working.

Comment 47

[Jason Smith [:jsmith]]

Figured out in IRC with reuben & gwagner. The existing SIM customization design requires the customization to part of Core Gaia, not in a distribution directory. This misaligns with the 1.1 & 1.01 design, which will likely cause a lot of confusion with our partners and potentially, data loss if there's data conflicts between the distribution directory & core Gaia customization. I'm filing a bug on this shortly.

Comment 48

[Antonio Manuel Amaya Calvo (:amac)]

(In reply to Jason Smith [:jsmith] from comment #47)
> Figured out in IRC with reuben & gwagner. The existing SIM customization
> design requires the customization to part of Core Gaia, not in a
> distribution directory. This misaligns with the 1.1 & 1.01 design, which
> will likely cause a lot of confusion with our partners and potentially, data
> loss if there's data conflicts between the distribution directory & core
> Gaia customization. I'm filing a bug on this shortly.

Actually you don't need to file a bug for that. Bug 917740 also fixes that (the files are generated at build time from an input file on the personalization/distribution directory).

Comment 49

[Candice Serran (:cserran)]

Jason Smith changed story state to accepted in Pivotal Tracker

Comment 50

[Candice Serran (:cserran)]

Jason Smith changed story state to accepted in Pivotal Tracker

Comment 51

[Jason Smith [:jsmith]]

Tested through the basics on this - the dependencies highlight the immediate issues that were found. Marking verified to indicate sanity testing was done on this.