diff --git a/CHANGELOG.md b/CHANGELOG.md index c5e3801074..6c196a3258 100644 --- 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 diff --git a/app/metrics.yaml b/app/metrics.yaml index 372060a641..9b6bab7699 100644 --- 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 diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index 223052c0b6..a06d1ad0ea 100644 --- 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 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 index c8a4f04e84..6be795e12f 100644 --- 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( { QuickActionSheet.readTapped.record(it) } ) + is Event.OpenedBookmarkInNewTab -> EventWrapper( + { BookmarksManagement.openInNewTab.record(it) } + ) + is Event.OpenedBookmarksInNewTabs -> EventWrapper( + { BookmarksManagement.openInNewTabs.record(it) } + ) + is Event.OpenedBookmarkInPrivateTab -> EventWrapper( + { BookmarksManagement.openInPrivateTab.record(it) } + ) + is Event.OpenedBookmarksInPrivateTabs -> EventWrapper( + { BookmarksManagement.openInPrivateTabs.record(it) } + ) + is Event.EditedBookmark -> EventWrapper( + { BookmarksManagement.edited.record(it) } + ) + is Event.MovedBookmark -> EventWrapper( + { BookmarksManagement.moved.record(it) } + ) + is Event.RemoveBookmark -> EventWrapper( + { BookmarksManagement.removed.record(it) } + ) + is Event.RemoveBookmarks -> EventWrapper( + { BookmarksManagement.multiRemoved.record(it) } + ) + is Event.ShareBookmark -> EventWrapper( + { BookmarksManagement.shared.record(it) } + ) + is Event.CopyBookmark -> EventWrapper( + { BookmarksManagement.copied.record(it) } + ) + is Event.AddBookmarkFolder -> EventWrapper( + { BookmarksManagement.folderAdd.record(it) } + ) is Event.CustomTabsMenuOpened -> EventWrapper( { CustomTab.menu.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 index a564bca74e..80a2798aa0 100644 --- 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() diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt index 298740cdcf..f4c26dd991 100644 --- 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().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().onNext(BookmarkChange.Change(currentRoot!!)) - } - } - } - override fun onBackPressed(): Boolean = (bookmarkComponent.uiView as BookmarkUIView).onBackPressed() override fun onAuthenticated(account: OAuthAccount) { diff --git 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 index 842e5d9f68..5d626a5a5f 100644 --- 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() } diff --git 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 index aa937003d5..3bfc5f7233 100644 --- 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) { - 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) { + 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) }