937361 - [B2G][Contacts] Facebook contacts imported do not appear in Contacts

Status: RESOLVED FIXED

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

Description

[Darren]

Attachment: Failed to Import From Facebook

Repro Steps:
1. Updated Buri to Build ID: 20131111040200
2. Open the Contacts app
3. Tap on the Settings Icon
4. Toggle on "Sync Friends"
5. Enter valid credentials
6. Select a contact

Actual:
After selecting contacts to import, none of the contacts are displayed.

Expected:
Contacts imported to the Device will display in the contacts app.

Environmental Variables
Device: Buri v1.3
Build ID: 20131111040200
Gecko: http://hg.mozilla.org/mozilla-central/rev/bc8c1eb0f2ba
Gaia: bcb4f94524ef02e9ef16b604d16c39e5a9c41281
Platform Version: 28.0a1
RIL Version: 01.02.00.019.102 
Base Build: 20131104

Comment 1

[Marcia Knous [:marcia]]

It looks as if I was running the build from the 10th, and I cannot reproduce it using this build:

Gaia   cc9fec48957c9d217221d403aac4b5f35a826dd6
SourceStamp 16949049f03d
BuildID 20131110040200
Version 28.0a1

Comment 2

[Matthew Vaughan]

This issue started reproducing on the 11/11/2013 1.3 build.

- Works -
Environmental Variables:
Device: Buri v1.3 COM RIL
BuildID: 20131110040200
Gaia: cc9fec48957c9d217221d403aac4b5f35a826dd6
Gecko: 16949049f03d
Version: 28.0a1
RIL Version: 01.02.00.019.102

- Broken -
Environmental Variables:
Device: Buri v1.3 MOZ RIL
BuildID: 20131111040200
Gaia: bcb4f94524ef02e9ef16b604d16c39e5a9c41281
Gecko: bc8c1eb0f2ba
Version: 28.0a1

Comment 3

[Jason Smith [:jsmith]]

Definitely caused by bug 936373.

Comment 4

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

Wonder if we can catch this with unit tests

Comment 5

[Jose Manuel Cantera]

(In reply to Jason Smith [:jsmith] from comment #3)
> Definitely caused by bug 936373.

That bug is not the cause

Comment 6

[Jose Manuel Cantera]

This regression has been caused by recent changes landed in Gecko the Datastore API. 

Jason, your suggestions about what bugs caused what regressions sometimes are useful and may help but PLEASE do not make statements like "Definetely" on areas you don't know in detail. It is better to say "Devs, it seems could be bug xx, guys, please could you check and in that case backout or fix it asap?". That way of doing things is helpful other ways are not appropriate IMHO.  

thanks

Comment 7

[Jose Manuel Cantera]

Attachment: 13622.html

simple patch to catch up with recent changes on Datastore API

Comment 8

[Cristian Rodriguez (:crdlc)]

Comment on attachment 830841 [details]
13622.html

Perfect

Comment 9

[Jason Smith [:jsmith]]

FWIW - If it is a gecko change that caused this, then it was either caused by bug 933050, bug 933049, or bug 923625.

Just so we know which bug caused this to know where we need tests:

Andrea - Which data store API change caused this regression?

Sorry - I should have checked the gecko changes as well before making that claim that bug was cause of this regression.

Comment 10

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #9)
> FWIW - If it is a gecko change that caused this, then it was either caused
> by bug 933050, bug 933049, or bug 923625.
> 
> Just so we know which bug caused this to know where we need tests:
> 
> Andrea - Which data store API change caused this regression?
> 
> Sorry - I should have checked the gecko changes as well before making that
> claim that bug was cause of this regression.

Actually, bug 923625 was backed out, so it isn't the cause of this regression.

Comment 11

[Jose Manuel Cantera]

https://github.com/mozilla-b2g/gaia/commit/02f709bfb705bcdead93f361edc4605f4f10539e

Comment 12

[Andrew Overholt [:overholt]]

Bug 933050 contained the .update() -> .put() rename.

Andrea or Ehsan can provide more info about the reasons for the change.

Comment 13

[Jason Smith [:jsmith]]

(In reply to Andrew Overholt [:overholt] from comment #12)
> Bug 933050 contained the .update() -> .put() rename.
> 
> Andrea or Ehsan can provide more info about the reasons for the change.

We should find out if this old API usage exists in Gaia in other places to identify other fallouts here.

Comment 14

[(no longer active)]

(In reply to Andrew Overholt [:overholt] from comment #12)
> Bug 933050 contained the .update() -> .put() rename.
> 
> Andrea or Ehsan can provide more info about the reasons for the change.

This was done to make the API interface more similar to the IndexedDB IDBObjectStore interface.

Comment 15

[Jason Smith [:jsmith]]

Looks like Ehsan clarified my needinfo.

Comment 16

[Andrew Overholt [:overholt]]

(In reply to Jason Smith [:jsmith] from comment #13)
> (In reply to Andrew Overholt [:overholt] from comment #12)
> > Bug 933050 contained the .update() -> .put() rename.
> > 
> > Andrea or Ehsan can provide more info about the reasons for the change.
> 
> We should find out if this old API usage exists in Gaia in other places to
> identify other fallouts here.

After git grepping a bit I think we're okay here.

Comment 17

[(no longer active)]

FWIW I should have pointed out that we need to file gaia bugs when I reviewed the original patch.  Sorry that slipped my mind. :(

Comment 18

[Natalya Kot [:nkot]]

This bug no longer reproduces on today's Buri m-c build  
- able to import Facebook contacts, once imported they appear in the Contacts app. 

Buri 1.3
BuildID: 20131113040200
Gaia: 7243b75041c93bc7deb240131748d0b164f3f0b0
Gecko: 7b014f0f3b03
Version: 28.0a1
Firmware: 20131104

Comment 19

[Jason Smith [:jsmith]]

Blocking+ for smoketest regression