For #3700 - Add Setting to Delete Data on "Quit" menu action #5098

ekager · 2019-09-04T00:49:02Z

Pull Request checklist

Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
Tests: This PR includes thorough tests or an explanation of why it does not
Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

codecov-io · 2019-09-05T21:26:26Z

Codecov Report

❗ No coverage uploaded for pull request base (master@999d3cb). Click here to learn what that means.
The diff coverage is 31.53%.

@@            Coverage Diff            @@
##             master    #5098   +/-   ##
=========================================
  Coverage          ?   11.72%           
  Complexity        ?      242           
=========================================
  Files             ?      246           
  Lines             ?    10145           
  Branches          ?     1496           
=========================================
  Hits              ?     1189           
  Misses            ?     8882           
  Partials          ?       74

Impacted Files	Coverage Δ	Complexity Δ
...va/org/mozilla/fenix/components/metrics/Metrics.kt	`22.28% <ø> (ø)`	`0 <0> (?)`
...fenix/settings/DeleteBrowsingDataOnQuitFragment.kt	`0% <0%> (ø)`	`0 <0> (?)`
...a/org/mozilla/fenix/browser/BaseBrowserFragment.kt	`0% <0%> (ø)`	`0 <0> (?)`
...lla/fenix/components/toolbar/DefaultToolbarMenu.kt	`0% <0%> (ø)`	`0 <0> (?)`
...ava/org/mozilla/fenix/settings/SettingsFragment.kt	`0% <0%> (ø)`	`0 <0> (?)`
.../mozilla/fenix/settings/SharedPreferenceUpdater.kt	`0% <0%> (ø)`	`0 <0> (?)`
...rg/mozilla/fenix/components/toolbar/ToolbarMenu.kt	`94.44% <100%> (ø)`	`0 <0> (?)`
...pp/src/main/java/org/mozilla/fenix/FeatureFlags.kt	`90% <100%> (ø)`	`2 <0> (?)`
...nix/components/toolbar/BrowserToolbarController.kt	`92% <100%> (ø)`	`0 <0> (?)`
...lla/fenix/settings/DeleteBrowsingDataController.kt	`79.31% <71.42%> (ø)`	`0 <0> (?)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 999d3cb...0cf128b. Read the comment docs.

ekager · 2019-09-09T20:44:34Z

Request for data collection review form

All questions are mandatory. You must receive review from a data steward peer on your responses to these questions before shipping new data collection.

What questions will you answer with this data?

How often users tap "quit" from the browser toolbar menu

Why does Mozilla need to answer these questions? Are there benefits for users? Do we need this information to address product or business requirements?

To measure how often users interact with the quit button in Fenix to measure feature success and visibility.

What alternative methods did you consider to answer these questions? Why were they not sufficient?

N/A (These are baseline metrics)

Can current instrumentation answer these questions?

Currently there are no events to track this button

List all proposed measurements and indicate the category of data collection for each measurement, using the Firefox data c ollection categories on the found on the Mozilla wiki.

All data is Category 2.

How long will this data be collected?

Until 03/01/2020

What populations will you measure?

All release, beta, and nightly users with telemetry enabled.

Please provide a general description of how you will analyze this data.

Glean / Amplitude

Where do you intend to share the results of your analysis?

Only on Glean, Amplitude and with mobile teams.

ekager · 2019-09-09T20:46:57Z

Tagging @boek for data review and @colintheshots for the feature since it was similar to granular data options and I ended up using your categories and had to modify some of the functions to work with mine

liuche · 2019-09-10T17:41:24Z

app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt

@@ -252,7 +252,7 @@ sealed class Event {
        enum class Item {
            SETTINGS, LIBRARY, HELP, DESKTOP_VIEW_ON, DESKTOP_VIEW_OFF, FIND_IN_PAGE, NEW_TAB,
            NEW_PRIVATE_TAB, SHARE, REPORT_SITE_ISSUE, BACK, FORWARD, RELOAD, STOP, OPEN_IN_FENIX,
-            SAVE_TO_COLLECTION, ADD_TO_HOMESCREEN
+            SAVE_TO_COLLECTION, ADD_TO_HOMESCREEN, QUIT


@boek how do new events get into the metrics.md file? Do you have to change more files, or does the documentation automatically get added when you just add an event?

Should these be updated in the metrics.md file as part of the PR? At a quick glance I also didn't see save_to_collection or add_to_homescreen, so I'm wondering if I'm looking in the wrong place.

I think we need to manually add this to metrics.md for now. At least until the Glean auto-generate documentation PR is merged

@ekager

Added to metrics.md and metrics.yaml!

Unrelated, @boek does this mean open_in_fenix, save_to_collection, and add_to_homescreen did not get data review, or need to be updated?

@liuche that is correct
https://github.com/mozilla-mobile/fenix/pull/2131/files
and #4925 didn't get data reviews. In the short term I could put myself as a codeowner of metrics.kt until we get a better process

boek · 2019-09-11T20:29:04Z

app/metrics.yaml

@@ -78,7 +78,7 @@ events:
        description: >
          A string containing the name of the item the user tapped. These items include:
          Settings, Library, Help, Desktop Site toggle on/off, Find in Page, New Tab,
-          Private Tab, Share, Report Site Issue, Back/Forward button, Reload Button
+          Private Tab, Share, Report Site Issue, Back/Forward button, Reload Button, Quit
    bugs:
    - 1024
    data_reviews:


Could we add a link to this PR for this data review?

boek · 2019-09-11T21:51:39Z

Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?

Yes, updating existing probes w/ documentation

Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, Fenix data controls

If the request is for permanent data collection, is there someone who will monitor the data over time?

Fenix PM Vesta will monitor, and this has an expiry

Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Type 2

Is the data collection request for default-on or default-off?

Default on

Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?

No

Is the data collection covered by the existing Firefox privacy notice?

Yes

Does there need to be a check-in in the future to determine whether to renew the data? (Yes/No) (If yes, set a todo reminder or file a bug if appropriate)**

Has expiry, 3/2020

Does the data collection use a third-party collection tool? If yes, escalate to legal.

No

boek

Data review

boek

Just one comment/question! 🚢

app/src/main/java/org/mozilla/fenix/settings/DeleteBrowsingDataOnQuitFragment.kt

…ction

ekager force-pushed the 3700-clear-data branch 6 times, most recently from 60988e7 to f7653f3 Compare September 5, 2019 21:13

ekager force-pushed the 3700-clear-data branch 2 times, most recently from 07bb475 to f3504aa Compare September 9, 2019 20:39

ekager requested a review from boek September 9, 2019 20:45

ekager marked this pull request as ready for review September 9, 2019 20:45

ekager requested review from a team as code owners September 9, 2019 20:45

ekager requested a review from colintheshots September 9, 2019 20:46

ekager force-pushed the 3700-clear-data branch from f3504aa to 9807fbc Compare September 9, 2019 21:30

ekager added needs:data-review PR is awaiting a data review pr:needs-review labels Sep 10, 2019

liuche removed the pr:needs-review label Sep 10, 2019

liuche reviewed Sep 10, 2019

View reviewed changes

ekager force-pushed the 3700-clear-data branch 4 times, most recently from 2776901 to 70895b1 Compare September 11, 2019 18:45

boek suggested changes Sep 11, 2019

View reviewed changes

ekager force-pushed the 3700-clear-data branch from 70895b1 to 5a5de50 Compare September 11, 2019 21:36

ekager requested a review from boek September 11, 2019 21:37

boek approved these changes Sep 11, 2019

View reviewed changes

boek removed the needs:data-review PR is awaiting a data review label Sep 11, 2019

boek approved these changes Sep 11, 2019

View reviewed changes

app/src/main/java/org/mozilla/fenix/settings/DeleteBrowsingDataOnQuitFragment.kt Outdated Show resolved Hide resolved

For mozilla-mobile#3700 - Add Setting to Delete Data on "Quit" menu a…

0cf128b

…ction

ekager force-pushed the 3700-clear-data branch from 5a5de50 to 0cf128b Compare September 11, 2019 22:55

ekager merged commit e3209dc into mozilla-mobile:master Sep 11, 2019

ekager deleted the 3700-clear-data branch September 11, 2019 23:06

eliserichards mentioned this pull request Jun 14, 2021

For #19923: renew telemetry probes #19924

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

For #3700 - Add Setting to Delete Data on "Quit" menu action #5098

For #3700 - Add Setting to Delete Data on "Quit" menu action #5098

ekager commented Sep 4, 2019 •

edited

codecov-io commented Sep 5, 2019 •

edited

ekager commented Sep 9, 2019

ekager commented Sep 9, 2019

liuche Sep 10, 2019

sblatz Sep 10, 2019

ekager Sep 10, 2019

liuche Sep 10, 2019

boek Sep 11, 2019

boek Sep 11, 2019

ekager Sep 11, 2019

boek commented Sep 11, 2019 •

edited

boek left a comment

boek left a comment

For #3700 - Add Setting to Delete Data on "Quit" menu action #5098

For #3700 - Add Setting to Delete Data on "Quit" menu action #5098

Conversation

ekager commented Sep 4, 2019 • edited

Pull Request checklist

codecov-io commented Sep 5, 2019 • edited

Codecov Report

ekager commented Sep 9, 2019

Request for data collection review form

ekager commented Sep 9, 2019

liuche Sep 10, 2019

Choose a reason for hiding this comment

sblatz Sep 10, 2019

Choose a reason for hiding this comment

ekager Sep 10, 2019

Choose a reason for hiding this comment

liuche Sep 10, 2019

Choose a reason for hiding this comment

boek Sep 11, 2019

Choose a reason for hiding this comment

boek Sep 11, 2019

Choose a reason for hiding this comment

ekager Sep 11, 2019

Choose a reason for hiding this comment

boek commented Sep 11, 2019 • edited

boek left a comment

Choose a reason for hiding this comment

boek left a comment

Choose a reason for hiding this comment

ekager commented Sep 4, 2019 •

edited

codecov-io commented Sep 5, 2019 •

edited

boek commented Sep 11, 2019 •

edited