For #2267: Fixes telemetry for reader view

mozilla-mobile · Jul 27, 2019 · bb0f5a2 · bb0f5a2
1 parent e6487ba
commit bb0f5a2
Show file tree

Hide file tree

Showing 6 changed files with 42 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,15 @@ private val Event.wrapper
         is Event.CollectionTabSelectOpened -> EventWrapper<NoExtraKeys>(
             { Collections.tabSelectOpened.record(it) }
         )
+        is Event.ReaderModeOpened -> EventWrapper<NoExtraKeys>(
+            { ReaderMode.opened.record(it) }
+        )
+        is Event.ReaderModeClosed -> EventWrapper<NoExtraKeys>(
+            { ReaderMode.closed.record(it) }
+        )
+        is Event.ReaderModeAppearanceOpened -> EventWrapper<NoExtraKeys>(
+            { ReaderMode.appearance.record(it) }
+        )
 
         // Don't track other events with Glean
         else -> null

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/quickactionsheet/QuickActionInteractor.kt b/app/src/main/java/org/mozilla/fenix/quickactionsheet/QuickActionInteractor.kt
@@ -60,11 +60,12 @@ class QuickActionInteractor(
 
     @CallSuper
     override fun onReadPressed() {
-        context.metrics.track(Event.QuickActionSheetReadTapped)
         val enabled = selectedSession?.readerMode ?: false
         if (enabled) {
+            context.metrics.track(Event.QuickActionSheetClosed)
             readerModeController.hideReaderView()
         } else {
+            context.metrics.track(Event.QuickActionSheetOpened)
             readerModeController.showReaderView()
         }
         quickActionStore.dispatch(QuickActionSheetAction.ReaderActiveStateChange(!enabled))
@@ -83,7 +84,7 @@ class QuickActionInteractor(
 
     @CallSuper
     override fun onAppearancePressed() {
-        // 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/quickactionsheet/QuickActionInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/quickactionsheet/QuickActionInteractorTest.kt
@@ -185,11 +185,11 @@ class QuickActionInteractorTest {
         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.onReadPressed()
 
-        verify { metrics.track(Event.QuickActionSheetReadTapped) }
+        verify { metrics.track(Event.QuickActionSheetOpened) }
         verify { readerModeController.showReaderView() }
     }
 
@@ -213,27 +213,31 @@ class QuickActionInteractorTest {
         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.onReadPressed()
 
-        verify { metrics.track(Event.QuickActionSheetReadTapped) }
+        verify { metrics.track(Event.QuickActionSheetClosed) }
         verify { readerModeController.hideReaderView() }
     }
 
     @Test
     fun onAppearancePressed() {
         val context: Context = mockk()
+        val metrics: MetricController = mockk()
         val readerModeController: ReaderModeController = mockk(relaxed = true)
+        val quickActionSheetStore: QuickActionSheetStore = mockk(relaxed = true)
 
         val interactor = QuickActionInteractor(
             context,
             readerModeController,
-            mockk(),
+            quickActionSheetStore,
             mockk(),
             mockk(),
             mockk()
         )
+        every { context.metrics } returns metrics
+        every { metrics.track(Event.ReaderModeAppearanceOpened) } just Runs
 
         interactor.onAppearancePressed()
 

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>