For #974: Telemetry for Bookmarks

2019-04-17 12:37:36 -05:00 · 2019-04-17 12:37:36 -05:00 · 849764d23a
parent c939aee4cf
commit 849764d23a
8 changed files with 234 additions and 48 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - #1079 - Managing site permissions exceptions
 - #1312 - Added clear textfield buttons for editing bookmarks
 - #1312 - Added a missing edit action for bookmark selections
+- #974 - Added telemetry for bookmarks

 ### Changed
 - #1429 - Updated site permissions ui for MVP
--- a/app/metrics.yaml
+++ b/app/metrics.yaml
@ -382,6 +382,129 @@ search.default_engine:
    - fenix-core@mozilla.com
    expires: "2019-09-01"

+bookmarks_management:
+  open_in_new_tab:
+    type: event
+    description: >
+      A user opened a bookmark in a new tab.
+    bugs:
+    - 974
+    data_reviews:
+    - https://github.com/mozilla-mobile/fenix/pull/1708
+    notification_emails:
+    - fenix-core@mozilla.com
+    expires: "2020-03-01"
+  open_in_new_tabs:
+    type: event
+    description: >
+      A user opened multiple bookmarks at once in new tabs.
+    bugs:
+    - 974
+    data_reviews:
+    - https://github.com/mozilla-mobile/fenix/pull/1708
+    notification_emails:
+    - fenix-core@mozilla.com
+    expires: "2020-03-01"
+  open_in_private_tab:
+    type: event
+    description: >
+      A user opened a bookmark in a new private tab.
+    bugs:
+    - 974
+    data_reviews:
+    - https://github.com/mozilla-mobile/fenix/pull/1708
+    notification_emails:
+    - fenix-core@mozilla.com
+    expires: "2020-03-01"
+  open_in_private_tabs:
+    type: event
+    description: >
+      A user opened multiple bookmarks at once in new private tabs.
+    bugs:
+    - 974
+    data_reviews:
+    - https://github.com/mozilla-mobile/fenix/pull/1708
+    notification_emails:
+    - fenix-core@mozilla.com
+    expires: "2020-03-01"
+  edited:
+    type: event
+    description: >
+      A user edited the title and/or URL of an existing bookmark.
+    bugs:
+    - 974
+    data_reviews:
+    - https://github.com/mozilla-mobile/fenix/pull/1708
+    notification_emails:
+    - fenix-core@mozilla.com
+    expires: "2020-03-01"
+  moved:
+    type: event
+    description: >
+      A user moved an existing bookmark or folder to another folder.
+    bugs:
+    - 974
+    data_reviews:
+    - https://github.com/mozilla-mobile/fenix/pull/1708
+    notification_emails:
+    - fenix-core@mozilla.com
+    expires: "2020-03-01"
+  removed:
+    type: event
+    description: >
+      A user removed a bookmark item.
+    bugs:
+    - 974
+    data_reviews:
+    - https://github.com/mozilla-mobile/fenix/pull/1708
+    notification_emails:
+    - fenix-core@mozilla.com
+    expires: "2020-03-01"
+  multi_removed:
+    type: event
+    description: >
+      A user removed multiple bookmarks at once.
+    bugs:
+    - 974
+    data_reviews:
+    - https://github.com/mozilla-mobile/fenix/pull/1708
+    notification_emails:
+    - fenix-core@mozilla.com
+    expires: "2020-03-01"
+  shared:
+    type: event
+    description: >
+      A user shared a bookmark.
+    bugs:
+    - 974
+    data_reviews:
+    - https://github.com/mozilla-mobile/fenix/pull/1708
+    notification_emails:
+    - fenix-core@mozilla.com
+    expires: "2020-03-01"
+  copied:
+    type: event
+    description: >
+      A user copied a bookmark.
+    bugs:
+    - 974
+    data_reviews:
+    - https://github.com/mozilla-mobile/fenix/pull/1708
+    notification_emails:
+    - fenix-core@mozilla.com
+    expires: "2020-03-01"
+  folder_add:
+    type: event
+    description: >
+      A user added a new bookmark folder.
+    bugs:
+    - 974
+    data_reviews:
+    - https://github.com/mozilla-mobile/fenix/pull/1708
+    notification_emails:
+    - fenix-core@mozilla.com
+    expires: "2020-03-01"
+
 custom_tab:
  closed:
    type: event
--- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt
@ -357,6 +357,7 @@ class BrowserFragment : Fragment(), BackHandler, CoroutineScope {
                            val guid = requireComponents.core.bookmarksStorage
                                .addItem(BookmarkRoot.Mobile.id, session.url, session.title, null)
                            launch(Main) {
+                                requireComponents.analytics.metrics.track(Event.AddBookmark)
                                FenixSnackbar.make(
                                    view!!,
                                    Snackbar.LENGTH_LONG
--- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt
@ -7,13 +7,14 @@ import android.content.Context
 import mozilla.components.service.glean.Glean
 import mozilla.components.service.glean.private.NoExtraKeys
 import mozilla.components.support.utils.Browsers
+import org.mozilla.fenix.GleanMetrics.BookmarksManagement
+import org.mozilla.fenix.GleanMetrics.ContextMenu
 import org.mozilla.fenix.GleanMetrics.CrashReporter
+import org.mozilla.fenix.GleanMetrics.CustomTab
 import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.FindInPage
-import org.mozilla.fenix.GleanMetrics.ContextMenu
-import org.mozilla.fenix.GleanMetrics.QuickActionSheet
 import org.mozilla.fenix.GleanMetrics.Metrics
-import org.mozilla.fenix.GleanMetrics.CustomTab
+import org.mozilla.fenix.GleanMetrics.QuickActionSheet
 import org.mozilla.fenix.GleanMetrics.SearchDefaultEngine
 import org.mozilla.fenix.ext.components

@ -107,6 +108,39 @@ private val Event.wrapper
        is Event.QuickActionSheetReadTapped -> EventWrapper<NoExtraKeys>(
            { QuickActionSheet.readTapped.record(it) }
        )
+        is Event.OpenedBookmarkInNewTab -> EventWrapper<NoExtraKeys>(
+            { BookmarksManagement.openInNewTab.record(it) }
+        )
+        is Event.OpenedBookmarksInNewTabs -> EventWrapper<NoExtraKeys>(
+            { BookmarksManagement.openInNewTabs.record(it) }
+        )
+        is Event.OpenedBookmarkInPrivateTab -> EventWrapper<NoExtraKeys>(
+            { BookmarksManagement.openInPrivateTab.record(it) }
+        )
+        is Event.OpenedBookmarksInPrivateTabs -> EventWrapper<NoExtraKeys>(
+            { BookmarksManagement.openInPrivateTabs.record(it) }
+        )
+        is Event.EditedBookmark -> EventWrapper<NoExtraKeys>(
+            { BookmarksManagement.edited.record(it) }
+        )
+        is Event.MovedBookmark -> EventWrapper<NoExtraKeys>(
+            { BookmarksManagement.moved.record(it) }
+        )
+        is Event.RemoveBookmark -> EventWrapper<NoExtraKeys>(
+            { BookmarksManagement.removed.record(it) }
+        )
+        is Event.RemoveBookmarks -> EventWrapper<NoExtraKeys>(
+            { BookmarksManagement.multiRemoved.record(it) }
+        )
+        is Event.ShareBookmark -> EventWrapper<NoExtraKeys>(
+            { BookmarksManagement.shared.record(it) }
+        )
+        is Event.CopyBookmark -> EventWrapper<NoExtraKeys>(
+            { BookmarksManagement.copied.record(it) }
+        )
+        is Event.AddBookmarkFolder -> EventWrapper<NoExtraKeys>(
+            { BookmarksManagement.folderAdd.record(it) }
+        )
        is Event.CustomTabsMenuOpened -> EventWrapper<NoExtraKeys>(
            { CustomTab.menu.record(it) }
        )
--- a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt
@ -51,6 +51,16 @@ sealed class Event {
    object AddBookmark : Event()
    object RemoveBookmark : Event()
    object OpenedBookmark : Event()
+    object OpenedBookmarkInNewTab : Event()
+    object OpenedBookmarksInNewTabs : Event()
+    object OpenedBookmarkInPrivateTab : Event()
+    object OpenedBookmarksInPrivateTabs : Event()
+    object EditedBookmark : Event()
+    object MovedBookmark : Event()
+    object ShareBookmark : Event()
+    object CopyBookmark : Event()
+    object AddBookmarkFolder : Event()
+    object RemoveBookmarks : Event()
    object QuickActionSheetOpened : Event()
    object QuickActionSheetClosed : Event()
    object QuickActionSheetShareTapped : Event()
--- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt
@ -27,7 +27,6 @@ import androidx.fragment.app.Fragment
 import androidx.navigation.Navigation
 import kotlinx.android.synthetic.main.fragment_bookmark.view.*
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.Dispatchers.IO
 import kotlinx.coroutines.Dispatchers.Main
 import kotlinx.coroutines.Job
@ -39,12 +38,13 @@ import mozilla.components.concept.sync.AccountObserver
 import mozilla.components.concept.sync.OAuthAccount
 import mozilla.components.concept.sync.Profile
 import mozilla.components.support.base.feature.BackHandler
+import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.BrowsingModeManager
 import org.mozilla.fenix.DefaultThemeManager
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
-import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.components.FenixSnackbar
+import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.allowUndo
 import org.mozilla.fenix.ext.getColorFromAttr
 import org.mozilla.fenix.ext.requireComponents
@ -64,7 +64,7 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
    private var currentRoot: BookmarkNode? = null

    override val coroutineContext: CoroutineContext
-        get() = Dispatchers.Main + job
+        get() = Main + job

    override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
        val view = inflater.inflate(R.layout.fragment_bookmark, container, false)
@ -84,6 +84,17 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
        super.onResume()
        (activity as AppCompatActivity).supportActionBar?.show()
        checkIfSignedIn()
+
+        val currentGuid = BookmarkFragmentArgs.fromBundle(arguments!!).currentRoot.ifEmpty { BookmarkRoot.Root.id }
+
+        launch(IO) {
+            currentRoot = requireComponents.core.bookmarksStorage.getTree(currentGuid) as BookmarkNode
+
+            launch(Main) {
+                if (currentGuid != BookmarkRoot.Root.id) (activity as HomeActivity).title = currentRoot!!.title
+                getManagedEmitter<BookmarkChange>().onNext(BookmarkChange.Change(currentRoot!!))
+            }
+        }
    }

    private fun checkIfSignedIn() {
@ -200,6 +211,7 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
                                    .openToBrowserAndLoad(url, from = BrowserDirection.FromBookmarks)
                            }
                        }
+                        requireComponents.analytics.metrics.track(Event.OpenedBookmark)
                    }
                    is BookmarkAction.Expand -> {
                        Navigation.findNavController(requireActivity(), R.id.container)
@ -225,20 +237,26 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
                        it.item.copyUrl(context!!)
                        FenixSnackbar.make(view!!, FenixSnackbar.LENGTH_LONG)
                            .setText(context!!.getString(R.string.url_copied)).show()
+                        requireComponents.analytics.metrics.track(Event.CopyBookmark)
                    }
                    is BookmarkAction.Share -> {
-                        it.item.url?.let { url -> requireContext().share(url) }
+                        it.item.url?.apply {
+                            requireContext().share(this)
+                            requireComponents.analytics.metrics.track(Event.ShareBookmark)
+                        }
                    }
                    is BookmarkAction.OpenInNewTab -> {
                        it.item.url?.let { url ->
                            requireComponents.useCases.tabsUseCases.addTab.invoke(url)
                            (activity as HomeActivity).browsingModeManager.mode = BrowsingModeManager.Mode.Normal
+                            requireComponents.analytics.metrics.track(Event.OpenedBookmarkInNewTab)
                        }
                    }
                    is BookmarkAction.OpenInPrivateTab -> {
                        it.item.url?.let { url ->
                            requireComponents.useCases.tabsUseCases.addPrivateTab.invoke(url)
                            (activity as HomeActivity).browsingModeManager.mode = BrowsingModeManager.Mode.Private
+                            requireComponents.analytics.metrics.track(Event.OpenedBookmarkInPrivateTab)
                        }
                    }
                    is BookmarkAction.Delete -> {
@ -247,6 +265,7 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
                            getString(R.string.bookmark_undo_deletion)
                        ) {
                            requireComponents.core.bookmarksStorage.deleteNode(it.item.guid)
+                            requireComponents.analytics.metrics.track(Event.RemoveBookmark)
                            refreshBookmarks()
                        }
                    }
@ -287,6 +306,7 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
                (activity as HomeActivity).browsingModeManager.mode = BrowsingModeManager.Mode.Normal
                Navigation.findNavController(requireActivity(), R.id.container)
                    .navigate(BookmarkFragmentDirections.actionBookmarkFragmentToHomeFragment())
+                requireComponents.analytics.metrics.track(Event.OpenedBookmarksInNewTabs)
                true
            }
            R.id.edit_bookmark_multi_select -> {
@ -308,6 +328,7 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
                (activity as HomeActivity).browsingModeManager.mode = BrowsingModeManager.Mode.Private
                Navigation.findNavController(requireActivity(), R.id.container)
                    .navigate(BookmarkFragmentDirections.actionBookmarkFragmentToHomeFragment())
+                requireComponents.analytics.metrics.track(Event.OpenedBookmarksInPrivateTabs)
                true
            }
            R.id.delete_bookmarks_multi_select -> {
@ -316,6 +337,7 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
                    getString(R.string.bookmark_undo_deletion)
                ) {
                    deleteSelectedBookmarks()
+                    requireComponents.analytics.metrics.track(Event.RemoveBookmarks)
                    refreshBookmarks()
                }
                true
@ -324,21 +346,6 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
        }
    }

-    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
-        super.onViewCreated(view, savedInstanceState)
-
-        val currentGuid = BookmarkFragmentArgs.fromBundle(arguments!!).currentRoot.ifEmpty { BookmarkRoot.Root.id }
-
-        launch(IO) {
-            currentRoot = requireComponents.core.bookmarksStorage.getTree(currentGuid) as BookmarkNode
-
-            launch(Main) {
-                if (currentGuid != BookmarkRoot.Root.id) (activity as HomeActivity).title = currentRoot!!.title
-                getManagedEmitter<BookmarkChange>().onNext(BookmarkChange.Change(currentRoot!!))
-            }
-        }
-    }
-
    override fun onBackPressed(): Boolean = (bookmarkComponent.uiView as BookmarkUIView).onBackPressed()

    override fun onAuthenticated(account: OAuthAccount) {
--- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/addfolder/AddBookmarkFolderFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/addfolder/AddBookmarkFolderFragment.kt
@ -26,6 +26,7 @@ import kotlinx.coroutines.Job
 import kotlinx.coroutines.launch
 import mozilla.appservices.places.BookmarkRoot
 import org.mozilla.fenix.R
+import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.getColorFromAttr
 import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel
@ -99,6 +100,7 @@ class AddBookmarkFolderFragment : Fragment(), CoroutineScope {
                    requireComponents.core.bookmarksStorage.addFolder(
                        sharedViewModel.selectedFolder!!.guid, bookmark_add_folder_title_edit.text.toString(), null
                    )
+                    requireComponents.analytics.metrics.track(Event.AddBookmarkFolder)
                    launch(Main) {
                        Navigation.findNavController(requireActivity(), R.id.container).popBackStack()
                    }
--- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt
@ -29,13 +29,13 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers.IO
 import kotlinx.coroutines.Dispatchers.Main
 import kotlinx.coroutines.Job
-import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.launch
 import mozilla.appservices.places.UrlParseFailed
 import mozilla.components.concept.storage.BookmarkInfo
 import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.concept.storage.BookmarkNodeType
 import org.mozilla.fenix.R
+import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.getColorFromAttr
 import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel
@ -87,8 +87,16 @@ class EditBookmarkFragment : Fragment(), CoroutineScope {
                    }
                    else -> throw IllegalArgumentException()
                }
-                bookmark_name_edit.setText(bookmarkNode!!.title)
-                bookmark_url_edit.setText(bookmarkNode!!.url)
+
+                if (bookmarkNode != null) {
+                    bookmark_name_edit.setText(bookmarkNode!!.title)
+                    bookmark_url_edit.setText(bookmarkNode!!.url)
+
+                    if (sharedViewModel.selectedFolder != null) {
+                        val bookmarkPair = Pair(bookmarkNode?.title!!, bookmarkNode?.url!!)
+                        updateBookmarkNode(bookmarkPair)
+                    }
+                }
            }

            bookmarkParent?.let { node ->
@ -108,13 +116,6 @@ class EditBookmarkFragment : Fragment(), CoroutineScope {
        updateBookmarkFromObservableInput()
    }

-    override fun onPause() {
-        launch {
-            updateBookmarkNode(Pair(bookmark_name_edit.text.toString(), bookmark_url_edit.text.toString()))
-        }
-        super.onPause()
-    }
-
    private fun updateBookmarkFromObservableInput() {
        Observable.combineLatest(
            bookmark_name_edit.textChanges().skipInitialValue(),
@ -128,9 +129,7 @@ class EditBookmarkFragment : Fragment(), CoroutineScope {
            .observeOn(AndroidSchedulers.mainThread())
            .`as`(AutoDispose.autoDisposable(AndroidLifecycleScopeProvider.from(this@EditBookmarkFragment)))
            .subscribe {
-                launch(IO) {
-                    updateBookmarkNode(it)
-                }
+                updateBookmarkNode(it)
            }
    }

@ -150,6 +149,7 @@ class EditBookmarkFragment : Fragment(), CoroutineScope {
            R.id.delete_bookmark_button -> {
                launch(IO) {
                    requireComponents.core.bookmarksStorage.deleteNode(guidToEdit)
+                    requireComponents.analytics.metrics.track(Event.RemoveBookmark)
                    launch(Main) {
                        Navigation.findNavController(requireActivity(), R.id.container).popBackStack()
                    }
@ -160,19 +160,27 @@ class EditBookmarkFragment : Fragment(), CoroutineScope {
        }
    }

-    private suspend fun updateBookmarkNode(pair: Pair<String, String>) {
-        try {
-            requireComponents.core.bookmarksStorage.updateNode(
-                guidToEdit,
-                BookmarkInfo(
-                    sharedViewModel.selectedFolder?.guid ?: bookmarkNode!!.parentGuid,
-                    bookmarkNode!!.position,
-                    pair.first,
-                    if (bookmarkNode?.type == BookmarkNodeType.ITEM) pair.second else null
-                )
-            )
-        } catch (e: UrlParseFailed) {
-            coroutineScope {
+    private fun updateBookmarkNode(pair: Pair<String, String>) {
+        launch(IO) {
+            try {
+                requireComponents.let {
+                    if (pair != Pair(bookmarkNode?.title, bookmarkNode?.url)) {
+                        it.analytics.metrics.track(Event.EditedBookmark)
+                    }
+                    if (sharedViewModel.selectedFolder != null) {
+                        it.analytics.metrics.track(Event.MovedBookmark)
+                    }
+                    it.core.bookmarksStorage.updateNode(
+                        guidToEdit,
+                        BookmarkInfo(
+                            sharedViewModel.selectedFolder?.guid ?: bookmarkNode!!.parentGuid,
+                            bookmarkNode?.position,
+                            pair.first,
+                            if (bookmarkNode?.type == BookmarkNodeType.ITEM) pair.second else null
+                        )
+                    )
+                }
+            } catch (e: UrlParseFailed) {
                launch(Main) {
                    bookmark_url_edit.error = getString(R.string.bookmark_invalid_url_error)
                }