1050684 - [Contacts-FTU] When FTU import Facebook Friends the next sync cycle must be scheduled

Status: VERIFIED FIXED

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

Description

[Jose Manuel Cantera]

We need to set a flag in the ImportStatusData datastore. That flag will be read during an idle cycle in contacts and, then the new sync will be scheduled.

Comment 1

[Adrián de la Rosa]

Attachment: Pull request #22730

Comment 2

[Jose Manuel Cantera]

Comment on attachment 8470787 [details]
Pull request #22730

the solution implemented has many flaws. please see comments on GH.

Comment 3

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8470787 [details]
Pull request #22730

Minor change on github, good job as usual, thanks a lot Adrian

Comment 4

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

Sorry for giving my comments now, I don't follow github comments anymore, you could flag me for f?, will be much more responsive.

With that said, unfortunately I would like to backout this commit.

Despite that is being executed asynchronously, the lazyload and the code parsed and executed is on the critical path of the app and we don't need it there.

A place for doing this could be when the list is fully loaded, or at least when we are above the fold. Will prefer actually when the list finished loading, remember you have a custom event that is triggered when this happens.

Can we sync tomorrow before backing it out?

Comment 5

[Adrián de la Rosa]

First of all, I'm sorry, I'm pretty new to this codebase, so sorry for messing up. Nonetheless I'm willing to learn so it would be great if we can meet on Skype tomorrow morning to discuss about this and the other contacts bug.

Thanks.

Comment 6

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

(In reply to Adrián de la Rosa from comment #5)
> First of all, I'm sorry, I'm pretty new to this codebase, so sorry for
> messing up. Nonetheless I'm willing to learn so it would be great if we can
> meet on Skype tomorrow morning to discuss about this and the other contacts
> bug.
> 
> Thanks.

Hei Adrian, no worries mate, we all need to learn, and that's where I can help you, to grow and learn on the currect codebase.

Tomorrow we have the contacts daily, we can sync there, and after that we can chat about this bug.

Cheers!

Comment 7

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

We were talking about leaving this bug as it is right now, it's working perfectly and open a new one for moving the code outside the critical path.

That work can be done through the stabilisation phase.

Thanks everyone folks!

Comment 8

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

Wooot, totally wrong bug here xD

Comment 9

[Adrián de la Rosa]

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #7)
> We were talking about leaving this bug as it is right now, it's working
> perfectly and open a new one for moving the code outside the critical path.
> 
> That work can be done through the stabilisation phase.
> 
> Thanks everyone folks!

Should we merge this and mark it fixed now? I don't know if you prefer to change this only when we land bug 1056647.

Comment 10

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

(In reply to Adrián de la Rosa from comment #9)
> (In reply to Francisco Jordano [:arcturus] [:francisco] from comment #7)
> > We were talking about leaving this bug as it is right now, it's working
> > perfectly and open a new one for moving the code outside the critical path.
> > 
> > That work can be done through the stabilisation phase.
> > 
> > Thanks everyone folks!
> 
> Should we merge this and mark it fixed now? I don't know if you prefer to
> change this only when we land bug 1056647.

Adrian you are right, this morning I got confused with so many tabs.

Let's land and we do the follow up in bug 1056647 moving the things around.

Thanks!

Comment 11

[Cristian Rodriguez (:crdlc)]

Merged in master:

https://github.com/mozilla-b2g/gaia/commit/e99b0c0b707470271ddc5d03b48140084f518271

Comment 12

[SandKing]

This issue has been verified successfully on Flame2.1
Verify video:"verify_1050684.mp4".

Flame2.1 build:
Gaia-Rev        dbaf3e31c9ba9c3436e074381744f2971e15c7bf
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/ebce587d2194
Build-ID        20141203001205
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141203.034907
FW-Date         Wed Dec  3 03:49:18 EST 2014
Bootloader      L1TC00011880

Comment 13

[SandKing]

Attachment: Verified video: verify_1050684.mp4