-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Closes #18173: Add Telemetry When User Opens a Bookmark #18174
Conversation
Request for data collection review
|
This pull request has conflicts when rebasing. Could you fix it @rocketsroger? 🙏 |
@rocketsroger In regards to the data-review, I have a question about this one. It looks like Fenix already records counts of when bookmarks are opened (in several different contexts) and I am just confirming that having an event here is necessary to answer the questions for this experiment/project.
Data Review
Yes, via the metrics.md and other Glean tools.
Yes
Not applicable, the collection is set to end 2021-08-01.
Category 2, User Interaction
Default on
No
Yes
No, unless the owner decides to extend beyond 2021-08-01
No Result:data-review+ |
Yes, we already do track when bookmarks are opened. However, Fenix didn't distinguish those events correctly and so we couldn't tell which method the user used to opened the bookmarks. This change is to make sure each method have a different event so we can know what feature is being used to open bookmarks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for writing the tests for everything! ✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also see we have unit tests for this in BookmarkFragmentInteractorTest.
open: | ||
type: event | ||
description: | | ||
A user opened a bookmark. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add where the bookmark was opened from? I can imagine in the future we might want to extend top sites to allow bookmarks so it would be good to know where its source is from the description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do track where the bookmark is opened from. But for some reason we never tracked the normal use case where the user taps on the bookmark directly. This is to fix it so we have all 5 cases covered. The 5 cases have the following events:
open (User tapes on the bookmark directly)
open_in_new_tab (User taps on the 3 dot menu and opens the bookmark)
open_in_new_tabs (User selected multiple bookmark and then tap on the 3 dot menu to open the bookmarks)
open_in_private_tab (User taps on the 3dot menu to open the bookmark in a private tab)
open_in_private_tabs (User taps on the 3 dot menu to open multiple bookmarks in private tabs)
I added this sheet in the meta issue but forgot to include it here. https://docs.google.com/spreadsheets/d/1EB6Mme0LR3GRE00YkX7Vm1VJOuTSnhwrSc4MOoySX5A/edit?usp=sharing. This will help clear what event tracks what use case. Thanks
Codecov Report
@@ Coverage Diff @@
## master #18174 +/- ##
============================================
+ Coverage 33.17% 33.21% +0.03%
- Complexity 1415 1416 +1
============================================
Files 461 461
Lines 19514 19518 +4
Branches 3005 3005
============================================
+ Hits 6474 6483 +9
- Misses 12191 12196 +5
+ Partials 849 839 -10
Continue to review full report at Codecov.
|
Fixes #18173
Pull Request checklist
To download an APK when reviewing a PR: