Closed
Bug 1267837
Opened 8 years ago
Closed 8 years ago
Update grid item telemetry to indicate whether an item is pinned
Categories
(Firefox for Android Graveyard :: Metrics, defect)
Firefox for Android Graveyard
Metrics
Tracking
(firefox47 fixed, firefox48 fixed, firefox49 fixed)
RESOLVED
FIXED
Firefox 49
People
(Reporter: Margaret, Assigned: Margaret)
Details
Attachments
(1 file)
58 bytes,
text/x-review-board-request
|
mfinkle
:
review+
ritu
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta+
|
Details |
We should edit this probe here to understand if an item is pinned or not: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/TopSitesPanel.java#217 The item has a type TopSites.TYPE_PINNED, so this would be straightforward to add. Should I just add this to the position extra, e.g. "0-pinned"? Or send it as an additional extra in an array?
Flags: needinfo?(mark.finkle)
Flags: needinfo?(bbermes)
Comment 1•8 years ago
|
||
Do we care about the position? Given that all of the Extras data is not already an array, I don't like the idea of some of the Extras data being array and some not. If we intend to create various types of tiles, or split into categories, we might need more context via Arrays. Let's think about how we'd use SQL to access old (non-array) and new (array) data. This might mean we'd switch to arrays for pinned and non-pinned tiles. If we can't use the data in SQL analyses, then it's not even worth collecting the data. Let's make sure we can use the data effectively.
Flags: needinfo?(mark.finkle)
Comment 2•8 years ago
|
||
We can use Presto's regexp_like(field, regex) to break up the Extras data. We could also try using the json_* methods like this:
>select
> date_trunc('day', submissiondate) as date,
> action,
> method,
> coalesce(json_array_get(extras, 0), json_parse(extras)),
> count(*) as count
>from android_events_v1
>where
> submissiondate > current_date - interval '7' day
> and action = 'loadurl.1' and method = 'griditem'
>group by 1, 2, 3, 4
The coalesce attempts to get the JSON array element, but that will be NULL if it's non array data, which falls back to the second argument, which needs to be of type 'JSON' for Presto. This does seem to return the data we'd want.
If we decide to use a JSON array, we need to decide that index 0 is the position and index 1 is the type, and so on. The indexes end up with special meaning. We could try using a JSON object, like { "p":0, "t":"pinned" } but who wants to deal with that shit?
Comment 3•8 years ago
|
||
I like the idea of tracking the pins in general, the rest of how and why to store it a certain way, I'll leave this up to you for now.
Flags: needinfo?(bbermes)
Comment 4•8 years ago
|
||
(In reply to :Margaret Leibovic from comment #0) > Should I just add this to the position extra, e.g. "0-pinned"? Or send it as > an additional extra in an array? After thinking about this a bit more, using "0-pinned" seems like the simple way forward. We can use regexp_* functions to split the data however we like.
Assignee | ||
Comment 5•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51975/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51975/
Attachment #8751342 -
Flags: review?(mark.finkle)
Comment 6•8 years ago
|
||
Comment on attachment 8751342 [details] MozReview Request: Bug 1267837 - Update grid item telemetry to indicate whether an item is pinned. r=mfinkle https://reviewboard.mozilla.org/r/51975/#review48793
Attachment #8751342 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 7•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/e62b981d0cbe620420b0c6fb7af38a07be913afb Bug 1267837 - Update grid item telemetry to indicate whether an item is pinned. r=mfinkle
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8751342 [details] MozReview Request: Bug 1267837 - Update grid item telemetry to indicate whether an item is pinned. r=mfinkle Approval Request Comment [Feature/regressing bug #]: none [User impact if declined]: we can't understand whether people use pinned top sites [Describe test coverage new/current, TreeHerder]: tested locally [Risks and why]: low-risk, only a change to a probe (no functional changes) [String/UUID change made/needed]: none
Attachment #8751342 -
Flags: approval-mozilla-beta?
Attachment #8751342 -
Flags: approval-mozilla-aurora?
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e62b981d0cbe
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
status-firefox47:
--- → affected
status-firefox48:
--- → affected
Comment on attachment 8751342 [details] MozReview Request: Bug 1267837 - Update grid item telemetry to indicate whether an item is pinned. r=mfinkle More telemetry data, verified locally, Aurora48+, Beta47+
Attachment #8751342 -
Flags: approval-mozilla-beta?
Attachment #8751342 -
Flags: approval-mozilla-beta+
Attachment #8751342 -
Flags: approval-mozilla-aurora?
Attachment #8751342 -
Flags: approval-mozilla-aurora+
Comment 11•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/9a4077c55f8b
Comment 12•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/629e46640752
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•