1027005 - [Collection] Population task triggers

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Everything.me, defect)

Comment 1

[Jason Smith [:jsmith]]

Looks like this is required feature work for collections.

Candice - Can you add the feature-b2g flag here?

Comment 2

[Dale Harvey (:daleharvey)]

ran / amir / kevin, just a quick pointer asking what a test for this looks like, so it sounds like if I have a smart collection with facebook in it, then install facebook, I should see the icon update? just a simple workflow I can verify with (also pointer to the e.me integration tests would be handy)

Comment 3

[Kevin Grandon :kgrandon]

Yes, that's correct. If you install facebook, I belive you should see the "Social" collections app icon update.

Also if you remove it, it should update as well. We only have a handful of e.me integration tests in the verticalhome collection_test.js file, working on adding more though.

Comment 4

[Kevin Grandon :kgrandon]

I'm going to free this bug for now to look at it with Cristian. If we don't make much progress, will assign back to you Dale, or you can steal it in the morning. Thanks.

Comment 5

[Kevin Grandon :kgrandon]

Cristian - we could really use your help tonight to look into this if you don't have more important things. I think we will need to use IAC perhaps from the homescreen app whenever an app is installed to check if it needs to be part of the 'native info'.

There are two pieces left, app installing/unintalling and collection creation. It would probably help to look at the code here: https://github.com/mozilla-b2g/gaia/pull/20231

I know this is a bit tricky, so if you don't finish a patch or anything, whatever findings you come up with could be useful for us tomorrow. Thanks!

Comment 6

[Cristian Rodriguez (:crdlc)]

OK, implementing something

Comment 7

[Cristian Rodriguez (:crdlc)]

I could do the code for app install/uninstall - add/remove app from collections bug 1028087

Comment 8

[Amir Nissim (:amirn)]

Attachment: bug_1027005: WIP Population triggers

hope this patch can help, it's something I did real quick and was not tested

Comment 9

[Cristian Rodriguez (:crdlc)]

Kevin told me that I did it and the bug 1028087 does it

(In reply to Amir Nissim (Everything.me) from comment #8)
> Created attachment 8443391 [details]
> bug_1027005: WIP Population triggers
> 
> hope this patch can help, it's something I did real quick and was not tested

Comment 10

[Amir Nissim (:amirn)]

(In reply to Cristian Rodriguez (:crdlc) from comment #9)
> Kevin told me that I did it and the bug 1028087 does it

Sorry Cristian, didn't know about that. I should have checked the bug before I worked on it :(

Anyway, I only handled the collection-app side so you can use my patch for native_info.js and objects.js. It's small and also handles the case collection is removed.

Thanks

Comment 11

[Cristian Rodriguez (:crdlc)]

Yes it will be very useful for who will do the collection part (collection install). The path is smoother for the last bug here :)

(In reply to Amir Nissim [on PTO until Jul 6] (:amirn) from comment #10)
> (In reply to Cristian Rodriguez (:crdlc) from comment #9)
> > Kevin told me that I did it and the bug 1028087 does it
> 
> Sorry Cristian, didn't know about that. I should have checked the bug before
> I worked on it :(
> 
> Anyway, I only handled the collection-app side so you can use my patch for
> native_info.js and objects.js. It's small and also handles the case
> collection is removed.
> 
> Thanks

Comment 12

[Kevin Grandon :kgrandon]

Cristian has taken the first part, and the harder one, in the dependent bug. I'm going to take this one to land the more simple install collections part.

Comment 13

[Kevin Grandon :kgrandon]

Attachment: Github pull request

Comment 14

[Kevin Grandon :kgrandon]

Comment on attachment 8443453 [details] [review]
Github pull request

Dale - this is one of the last FL blockers for 2.0, wondering if you could give it a review. There's two commits in it, but the first one already has an R+.

I'll work on an integration test for this next week.

Comment 15

[Dale Harvey (:daleharvey)]

Yeh I didnt have the background to write this without tests so it was gonna take a while longer, cheers for stealing, taking a look now

Comment 16

[Dale Harvey (:daleharvey)]

Comment on attachment 8443453 [details] [review]
Github pull request

Tested this out, works good for me

Comment 17

[Kevin Grandon :kgrandon]

Landed: https://github.com/mozilla-b2g/gaia/commit/ea40c008305f214c80fccd571284adce8517a4ab

Comment 18

[Kevin Grandon :kgrandon]

Comment on attachment 8443453 [details] [review]
Github pull request

Needed for the vertical homescreen.

Comment 19

[Kevin Grandon :kgrandon]

2.0: https://github.com/mozilla-b2g/gaia/commit/809b2138c9c33afc621d73e5a2f6f484ad612719

Comment 20

[Johan Lorenzo [:jlorenzo]]

Verified with Social Smart Collection: 
  * Populated when created Smart Collection => OK
  * Populated when installed app from Market Place => NOK. See 1029009
  * Removed when app is unistalled => OK