Closed Bug 1016239 Opened 6 years ago Closed 6 years ago

[Collections App] Do not show web results that are pinned to Collection (dedupe)

Categories

(Firefox OS Graveyard :: Gaia, defect)

x86
macOS
defect
Not set

Tracking

(feature-b2g:2.0, tracking-b2g:backlog, b2g-v2.0 fixed)

RESOLVED FIXED
2.0 S3 (6june)
feature-b2g 2.0
tracking-b2g backlog
Tracking Status
b2g-v2.0 --- fixed

People

(Reporter: amirn, Assigned: kgrandon)

References

Details

(Whiteboard: [systemsfe][p=3])

Attachments

(2 files)

No description provided.
Depends on: 1016238
blocking-b2g: --- → backlog
feature-b2g: --- → 2.0
Whiteboard: [systemsfe]
I would like to abstract the dedeupe logic from the search app in order to handle this.
I did the dedupe logic in the search app, and would like to extract and use it here. Taking.
Assignee: nobody → kgrandon
Status: NEW → ASSIGNED
Whiteboard: [systemsfe] → [systemsfe][p=3]
Target Milestone: --- → 2.0 S3 (6june)
Hey guys - could you give this a quick review? This should make it easy to actually use this across apps. Thanks!
Attachment #8430226 - Flags: review?(ran)
Attachment #8430226 - Flags: review?(amirn)
Comment on attachment 8430226 [details] [review]
Part 1 - Abstract dedupe.js from search.js

Kevin, what do you think about making deduping a gaia-grid feature and extract the files to /shared?
Since it will also be used in Collections I think the term GridDedupe (instead of SearchDedupe) is more appropriate.

We can do that later of course.
Attachment #8430226 - Flags: review?(amirn) → review+
Attachment #8430226 - Flags: review?(ran) → review+
Amir's suggestion speaks to me.
Sounds like a good idea to me. This has a dependency on bug 1017222 though. The only reason I might be adverse to it is if we would ever find the need to use it outside of a "grid" view. Hopefully not though.
Attachment #8430226 - Attachment description: Github pull request → Part 1 - Abstract dedupe.js from search.js
Hey Guys - this commit contains the work I did in the other bug for displaying of pinned apps, as well as a working dedupe implementation. Can you take a look? Thanks!
Attachment #8433615 - Flags: review?(ran)
Attachment #8433615 - Flags: review?(amirn)
Comment on attachment 8433615 [details] [review]
Part 2 - Implement deduping of web results from pinned results

added comments on Github.
Thanks.
Attachment #8433615 - Flags: review?(amirn) → review+
Comment on attachment 8433615 [details] [review]
Part 2 - Implement deduping of web results from pinned results

Added comments on Github as well
Attachment #8433615 - Flags: review?(ran) → review+
Ok, went ahead and landed this: https://github.com/mozilla-b2g/gaia/commit/4506564ed5cb37dc007c83241e3e6f2a7671179e

I think the main concern is whether or not dedupe should be moved into the grid logic. I'm still not convinced, but we can do this in a follow-up if you guys feel strongly about it.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Landed a follow-up, missed the dedupe.js path in the search app, oops: https://github.com/mozilla-b2g/gaia/commit/8f94479062343666d5fc1eb29af7e3dec4b2c4a7

Will work on a marionette test for this.
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.