For #2267: Fixes telemetry for reader view (#4328)

mozilla-mobile · Aug 1, 2019 · 97f7d5b · 97f7d5b
1 parent 2e7a618
commit 97f7d5b
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 30 deletions.
diff --git a/app/metrics.yaml b/app/metrics.yaml
@@ -314,17 +314,6 @@ quick_action_sheet:
     notification_emails:
     - fenix-core@mozilla.com
     expires: "2020-03-01"
-  read_tapped:
-    type: event
-    description: >
-      A user tapped the read button
-    bugs:
-    - 1195
-    data_reviews:
-    - https://github.com/mozilla-mobile/fenix/pull/1362#issuecomment-479668466
-    notification_emails:
-    - fenix-core@mozilla.com
-    expires: "2020-03-01"
 
 metrics:
   default_browser:
@@ -930,6 +919,17 @@ reader_mode:
     notification_emails:
       - fenix-core@mozilla.com
     expires: "2020-03-01"
+  closed:
+    type: event
+    description: >
+      A user closed reader mode
+    bugs:
+      - 2267
+    data_reviews:
+      - https://github.com/mozilla-mobile/fenix/pull/4328
+    notification_emails:
+      - fenix-core@mozilla.com
+    expires: "2020-03-01"
   appearance:
     type: event
     description: >

diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt
@@ -31,6 +31,7 @@ import org.mozilla.fenix.GleanMetrics.QuickActionSheet
 import org.mozilla.fenix.GleanMetrics.SearchDefaultEngine
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.GleanMetrics.Collections
+import org.mozilla.fenix.GleanMetrics.ReaderMode
 import org.mozilla.fenix.GleanMetrics.SyncAccount
 import org.mozilla.fenix.GleanMetrics.SyncAuth
 
@@ -121,9 +122,6 @@ private val Event.wrapper
         is Event.QuickActionSheetDownloadTapped -> EventWrapper<NoExtraKeys>(
             { QuickActionSheet.downloadTapped.record(it) }
         )
-        is Event.QuickActionSheetReadTapped -> EventWrapper<NoExtraKeys>(
-            { QuickActionSheet.readTapped.record(it) }
-        )
         is Event.OpenedBookmarkInNewTab -> EventWrapper<NoExtraKeys>(
             { BookmarksManagement.openInNewTab.record(it) }
         )
@@ -265,6 +263,12 @@ private val Event.wrapper
         is Event.CollectionTabSelectOpened -> EventWrapper<NoExtraKeys>(
             { Collections.tabSelectOpened.record(it) }
         )
+        is Event.ReaderModeOpened -> EventWrapper<NoExtraKeys>(
+            { ReaderMode.opened.record(it) }
+        )
+        is Event.ReaderModeAppearanceOpened -> EventWrapper<NoExtraKeys>(
+            { ReaderMode.appearance.record(it) }
+        )
         is Event.CollectionTabLongPressed -> EventWrapper<NoExtraKeys>(
             { Collections.longPress.record(it) }
         )

diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt
@@ -73,7 +73,6 @@ sealed class Event {
     object QuickActionSheetShareTapped : Event()
     object QuickActionSheetBookmarkTapped : Event()
     object QuickActionSheetDownloadTapped : Event()
-    object QuickActionSheetReadTapped : Event()
     object CustomTabsClosed : Event()
     object CustomTabsActionTapped : Event()
     object CustomTabsMenuOpened : Event()
@@ -100,6 +99,7 @@ sealed class Event {
     object HistoryAllItemsRemoved : Event()
     object ReaderModeAvailable : Event()
     object ReaderModeOpened : Event()
+    object ReaderModeClosed : Event()
     object ReaderModeAppearanceOpened : Event()
     object CollectionRenamed : Event()
     object CollectionTabRestored : Event()

diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserInteractor.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserInteractor.kt
@@ -50,11 +50,13 @@ class BrowserInteractor(
     }
 
     override fun onQuickActionSheetReadPressed() {
-        context.metrics.track(Event.QuickActionSheetReadTapped)
         val enabled = currentSession.readerMode
+
         if (enabled) {
+            context.metrics.track(Event.QuickActionSheetClosed)
             readerModeController.hideReaderView()
         } else {
+            context.metrics.track(Event.QuickActionSheetOpened)
             readerModeController.showReaderView()
         }
         store.dispatch(QuickActionSheetAction.ReaderActiveStateChange(!enabled))
@@ -65,7 +67,7 @@ class BrowserInteractor(
     }
 
     override fun onQuickActionSheetAppearancePressed() {
-        // TODO telemetry: https://github.com/mozilla-mobile/fenix/issues/2267
+        context.metrics.track(Event.ReaderModeAppearanceOpened)
         readerModeController.showControls()
     }
 }
diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserInteractorTest.kt
@@ -181,11 +181,11 @@ class BrowserInteractorTest {
         every { context.metrics } returns metrics
         every { context.components.core.sessionManager.selectedSession } returns session
         every { session.readerMode } returns false
-        every { metrics.track(Event.QuickActionSheetReadTapped) } just Runs
+        every { metrics.track(Event.QuickActionSheetOpened) } just Runs
 
         interactor.onQuickActionSheetReadPressed()
 
-        verify { metrics.track(Event.QuickActionSheetReadTapped) }
+        verify { metrics.track(Event.QuickActionSheetOpened) }
         verify { readerModeController.showReaderView() }
     }
 
@@ -209,11 +209,11 @@ class BrowserInteractorTest {
         every { context.metrics } returns metrics
         every { context.components.core.sessionManager.selectedSession } returns session
         every { session.readerMode } returns true
-        every { metrics.track(Event.QuickActionSheetReadTapped) } just Runs
+        every { metrics.track(Event.QuickActionSheetClosed) } just Runs
 
         interactor.onQuickActionSheetReadPressed()
 
-        verify { metrics.track(Event.QuickActionSheetReadTapped) }
+        verify { metrics.track(Event.QuickActionSheetClosed) }
         verify { readerModeController.hideReaderView() }
     }
 
@@ -240,8 +240,12 @@ class BrowserInteractorTest {
     @Test
     fun onQuickActionSheetAppearancePressed() {
         val context: Context = mockk()
+        val metrics: MetricController = mockk()
         val readerModeController: ReaderModeController = mockk(relaxed = true)
 
+        every { context.metrics } returns metrics
+        every { metrics.track(Event.ReaderModeAppearanceOpened) } just Runs
+
         val interactor = BrowserInteractor(
             context,
             mockk(),
@@ -253,6 +257,9 @@ class BrowserInteractorTest {
 
         interactor.onQuickActionSheetAppearancePressed()
 
-        verify { readerModeController.showControls() }
+        verify {
+            metrics.track(Event.ReaderModeAppearanceOpened)
+            readerModeController.showControls()
+        }
     }
 }
diff --git a/docs/metrics.md b/docs/metrics.md
@@ -314,14 +314,6 @@ tracking_protection</td>
         <td></td>
         <td>2020-03-01</td>
     </tr>
-    <tr>
-        <td>read_tapped</td>
-        <td>event</td>
-        <td>A user tapped the read button</td>
-        <td><a href="https://github.com/mozilla-mobile/fenix/pull/1362#issuecomment-479668466">link</a></td>
-        <td></td>
-        <td>2020-03-01</td>
-    </tr>
 </table>
 </pre>
 
@@ -789,6 +781,14 @@ tracking_protection</td>
         <td></td>
         <td>2020-03-01</td>
     </tr>
+    <tr>
+        <td>opened</td>
+        <td>event</td>
+        <td>A user closed reader mode</td>
+        <td><a href="https://github.com/mozilla-mobile/fenix/pull/4328">link</a></td>
+        <td></td>
+        <td>2020-03-01</td>
+    </tr>
     <tr>
         <td>appearance</td>
         <td>event</td>