Closed
Bug 1141891
Opened 9 years ago
Closed 9 years ago
[Metrics] Search through suggestions are not counted
Categories
(Firefox OS Graveyard :: Gaia::System, defect)
Tracking
(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)
People
(Reporter: shinglyu, Assigned: thills)
References
Details
(Keywords: verifyme)
Attachments
(2 files)
*** Description Search by clicking the search suggestions are not counted in the app usage ping. *** Steps to Reproduce Pre-requisite: the search suggestion is enabled. 1. Click the rocket bar 2. Type "Mozilla" 3. Click the first search suggestions 4. Wait for the App Usage ping to reach the metrics server *** Expected Results Default search provider has a search count of 1 *** Actual Results Search count = 0 *** Other Notes Only the searchs performed by clicking the "magnifier glass" button on the keyboard are counted. Users are equally probable to use both methods, so the search count will be underestimated *** Reproduction Frequency *** Build
Flags: needinfo?(thills)
Reporter | ||
Comment 1•9 years ago
|
||
*** Reproduction Frequency 3/3 *** Build Gaia-Rev 3f032238a52f08e4c2f68a47ad065a96eb22d470 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/9ae4bcc9b5f2 Build-ID 20150309162503 Version 37.0 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20150309.201258 FW-Date Mon Mar 9 20:13:09 EDT 2015 Bootloader L1TC000118D0
Comment 2•9 years ago
|
||
Looks like its a broken functionality from a new metrics related feature, so blocking on this
blocking-b2g: 2.2? → 2.2+
Comment 3•9 years ago
|
||
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8576401 [details] [review] [gaia] tamarahills:bugfix/1141891-add-suggestion-search > mozilla-b2g:master Hi Marshall, sorry for the confusion. I originally inadvertently posted this patch on the wrong bug. I was wondering why the autolander didn't show up :)
Attachment #8576401 -
Flags: review?(marshall)
Assignee | ||
Comment 6•9 years ago
|
||
Hi Howie, Yes, sorry I forgot to set to assigned. I have posted a PR as well. Thanks, -tamara
Status: NEW → ASSIGNED
Comment 7•9 years ago
|
||
Comment on attachment 8576401 [details] [review] [gaia] tamarahills:bugfix/1141891-add-suggestion-search > mozilla-b2g:master Looks great! We should probably have someone that works on search take a look too..
Attachment #8576401 -
Flags: review?(marshall) → review+
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8576401 [details] [review] [gaia] tamarahills:bugfix/1141891-add-suggestion-search > mozilla-b2g:master Hi Dale, Would you be able to have a look? Thanks, -tamara
Attachment #8576401 -
Flags: feedback?(dale)
Comment 9•9 years ago
|
||
Comment on attachment 8576401 [details] [review] [gaia] tamarahills:bugfix/1141891-add-suggestion-search > mozilla-b2g:master This looks great, cheers
Attachment #8576401 -
Flags: feedback?(dale) → feedback+
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → thills
Assignee | ||
Comment 10•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=903e6f09d2db https://github.com/mozilla-b2g/gaia/commit/db48e2353effeb3e3f74dda7b9103ee7ce8bbfde
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 8576401 [details] [review] [gaia] tamarahills:bugfix/1141891-add-suggestion-search > mozilla-b2g:master [Approval Request Comment] [Bug caused by] (feature/regressing bug #): Search count metrics feature [User impact] if declined: Suggested searches will not be counted and we will significantly undercount the metrics if we don't have this [Testing completed]: yes [Risk to taking this patch] (and alternatives if risky): low-medium. Should be easy to backout as it's small. [String changes made]:no
Attachment #8576401 -
Flags: approval-gaia-v2.2?(bbajaj)
Updated•9 years ago
|
Attachment #8576401 -
Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
Comment 12•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/a668608eae3183ab232c54d1a1d342555b1d9c56
Comment 13•9 years ago
|
||
Reverted for Gaia unit test failures. v2.2: https://github.com/mozilla-b2g/gaia/commit/fd7fa0c8f7ca575aafc3568262247e529323f31f https://treeherder.mozilla.org/logviewer.html#?job_id=73272&repo=mozilla-b2g37_v2_2
Flags: needinfo?(thills)
Assignee | ||
Comment 14•9 years ago
|
||
Hi Dale, I'm having some trouble with my suggestions_test.js when it gets uplifted to 2.2 and realized that this line (SearchProvider.ready()) is removed in master, but still exists in 2.2 master: https://github.com/mozilla-b2g/gaia/commit/323d773af7c579cc7f33fbe571cc5c8631156b15#diff-a72719dbedcb33e670a082ee4341d876L36 2.2: https://github.com/mozilla-b2g/gaia/blob/v2.2/apps/search/js/providers/suggestions.js#L36 It looks like bug 1131966 removes the line, but that change doesn't seem to be in 2.2. Just wanted to know if this will be uplifted before I redo the unit test to work differently with 2.2. Thanks! -tamara
Flags: needinfo?(thills) → needinfo?(dale)
Comment 15•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/3c5565bcdf0045261e0fd76e700eb32c4bbf3dc0
Comment 16•9 years ago
|
||
Unfortunately, this is still hitting test failures on v2.2 even with bug 1136664 uplifted first :(. Backed out again. v2.2: https://github.com/mozilla-b2g/gaia/commit/f52a2ac8f42489587e95a9254045683e0454b2e6 https://treeherder.mozilla.org/logviewer.html#?job_id=76315&repo=mozilla-b2g37_v2_2
Flags: needinfo?(thills)
Comment 17•9 years ago
|
||
Ugh, I obviously can't read. I have no idea why I thought bug 1136664 was going to help here.
No longer depends on: 1136664
Flags: needinfo?(dale)
Reporter | ||
Comment 18•9 years ago
|
||
Hi Tamara, Can we uplift this to 2.2?
Comment 19•9 years ago
|
||
So somehow a line that was removed in master did not get removed in 2.2 https://github.com/mozilla-b2g/gaia/commit/323d773af7c579cc7f33fbe571cc5c8631156b15 is the master commit, https://github.com/mozilla-b2g/gaia/commit/df92a3c0d8ed757c04b81085c260da9234c59981 is the 2.2 one The |SearchProvider.ready()| was removed in master but not in the 2.2 commit, a little confused as to how that can happen since it wasnt a branch patch but I am not a git expert, Ryan do you have any ideas? Tamara is gonna do a branch patch to remove that and hopefully the existing test should work as is.
Flags: needinfo?(dale)
Comment 20•9 years ago
|
||
No clue, sorry.
Comment 21•9 years ago
|
||
Assignee | ||
Comment 22•9 years ago
|
||
Comment on attachment 8582392 [details] [review] [gaia] tamarahills:bugfix/1141891-add-suggestion-search-2.2-branch-patch > mozilla-b2g:v2.2 Hi Marshall, I talked with Dale and we decided best approach is to make a branch patch. Essentially, there was a merge conflict from bug 1131966 where a merge didn't get correctly resolved on the uplift. So, we decided I should remove this line in the branch patch. BTW, I'm not sure what the process is for branch patch review, so just adding a review flag on this. It's the same patch as the 3.0, except with this line removed: https://github.com/mozilla-b2g/gaia/pull/29097/files#diff-a72719dbedcb33e670a082ee4341d876L36 For reference here is the merge commit difference on bug 1131966 master commit: https://github.com/mozilla-b2g/gaia/commit/323d773af7c579cc7f33fbe571cc5c8631156b15 2.2 commit: https://github.com/mozilla-b2g/gaia/commit/df92a3c0d8ed757c04b81085c260da9234c59981
Flags: needinfo?(thills)
Attachment #8582392 -
Flags: review?(marshall)
Comment 23•9 years ago
|
||
Comment on attachment 8582392 [details] [review] [gaia] tamarahills:bugfix/1141891-add-suggestion-search-2.2-branch-patch > mozilla-b2g:v2.2 Looks good, just had one quick question about a removed line of code
Attachment #8582392 -
Flags: review?(marshall) → review+
Comment 24•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/a7ff88fb4dec59533dfe7194ff9bf760d4de3eae
Updated•9 years ago
|
QA Whiteboard: [QAExclude]
Flags: needinfo?(jmercado)
Updated•9 years ago
|
Flags: needinfo?(jmercado)
You need to log in
before you can comment on or make changes to this bug.
Description
•