From c181d526d048aa4744c1af10a933b89566dc657c Mon Sep 17 00:00:00 2001 From: Elise Richards Date: Fri, 25 Mar 2022 14:54:43 -0700 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/23503: Respect studies and telemetry prefs when manually opting in to studies (https://github.com/mozilla-mobile/fenix/pull/23955) * For https://github.com/mozilla-mobile/fenix/issues/23503: Respect studies pref and telemetry enabled pref when manually opting in to studies * Add button to snackbar in nimbus secret settings that allows user to go directly to their data collection prefs * Remove refactoring Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .../fenix/nimbus/NimbusBranchesFragment.kt | 3 + .../controller/NimbusBranchesController.kt | 38 +++- app/src/main/res/navigation/nav_graph.xml | 8 + app/src/main/res/values/strings.xml | 5 + .../nimbus/NimbusBranchesControllerTest.kt | 166 +++++++++++++++++- 5 files changed, 218 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/nimbus/NimbusBranchesFragment.kt b/app/src/main/java/org/mozilla/fenix/nimbus/NimbusBranchesFragment.kt index 45a6d5d90..e98ffb85c 100644 --- a/app/src/main/java/org/mozilla/fenix/nimbus/NimbusBranchesFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/nimbus/NimbusBranchesFragment.kt @@ -10,6 +10,7 @@ import android.view.View import android.view.ViewGroup import androidx.fragment.app.Fragment import androidx.lifecycle.lifecycleScope +import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch @@ -47,6 +48,8 @@ class NimbusBranchesFragment : Fragment() { } controller = NimbusBranchesController( + context = requireContext(), + navController = findNavController(), nimbusBranchesStore = nimbusBranchesStore, experiments = requireContext().components.analytics.experiments, experimentId = args.experimentId diff --git a/app/src/main/java/org/mozilla/fenix/nimbus/controller/NimbusBranchesController.kt b/app/src/main/java/org/mozilla/fenix/nimbus/controller/NimbusBranchesController.kt index 46ae9479f..fda8e2b82 100644 --- a/app/src/main/java/org/mozilla/fenix/nimbus/controller/NimbusBranchesController.kt +++ b/app/src/main/java/org/mozilla/fenix/nimbus/controller/NimbusBranchesController.kt @@ -4,15 +4,22 @@ package org.mozilla.fenix.nimbus.controller +import android.content.Context +import androidx.navigation.NavController import mozilla.components.service.nimbus.NimbusApi import mozilla.components.service.nimbus.ui.NimbusBranchesAdapterDelegate import org.mozilla.experiments.nimbus.Branch +import org.mozilla.fenix.R +import org.mozilla.fenix.components.FenixSnackbar +import org.mozilla.fenix.ext.getRootView +import org.mozilla.fenix.ext.settings import org.mozilla.fenix.nimbus.NimbusBranchesAction +import org.mozilla.fenix.nimbus.NimbusBranchesFragmentDirections import org.mozilla.fenix.nimbus.NimbusBranchesStore /** * [NimbusBranchesFragment] controller. This implements [NimbusBranchesAdapterDelegate] to handle - * interactions with a Nimbus branch item. + * interactions with a Nimbus branch. * * @param nimbusBranchesStore An instance of [NimbusBranchesStore] for dispatching * [NimbusBranchesAction]s. @@ -20,12 +27,41 @@ import org.mozilla.fenix.nimbus.NimbusBranchesStore * @param experimentId The string experiment-id or "slug" for a Nimbus experiment. */ class NimbusBranchesController( + private val context: Context, + private val navController: NavController, private val nimbusBranchesStore: NimbusBranchesStore, private val experiments: NimbusApi, private val experimentId: String ) : NimbusBranchesAdapterDelegate { override fun onBranchItemClicked(branch: Branch) { + val telemetryEnabled = context.settings().isTelemetryEnabled + val experimentsEnabled = context.settings().isExperimentationEnabled + + updateOptInState(branch) + + if (!telemetryEnabled && !experimentsEnabled) { + val snackbarText = context.getString(R.string.experiments_snackbar) + val buttonText = context.getString(R.string.experiments_snackbar_button) + context.getRootView()?.let { v -> + FenixSnackbar.make( + view = v, + FenixSnackbar.LENGTH_LONG, + isDisplayedWithBrowserToolbar = false + ) + .setText(snackbarText) + .setAction(buttonText) { + navController.navigate( + NimbusBranchesFragmentDirections + .actionNimbusBranchesFragmentToDataChoicesFragment() + ) + } + .show() + } + } + } + + private fun updateOptInState(branch: Branch) { nimbusBranchesStore.dispatch( if (experiments.getExperimentBranch(experimentId) != branch.slug) { experiments.optInWithBranch(experimentId, branch.slug) diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml index 7052780db..db415a858 100644 --- a/app/src/main/res/navigation/nav_graph.xml +++ b/app/src/main/res/navigation/nav_graph.xml @@ -1160,6 +1160,14 @@ + diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 27c843137..f75ec348b 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1763,4 +1763,9 @@ Part of the Firefox family. %s Learn more + + + Enable telemetry to send data. + + Go to settings diff --git a/app/src/test/java/org/mozilla/fenix/nimbus/NimbusBranchesControllerTest.kt b/app/src/test/java/org/mozilla/fenix/nimbus/NimbusBranchesControllerTest.kt index 6f436f4b3..233c4d3d1 100644 --- a/app/src/test/java/org/mozilla/fenix/nimbus/NimbusBranchesControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/nimbus/NimbusBranchesControllerTest.kt @@ -4,32 +4,196 @@ package org.mozilla.fenix.nimbus +import android.R +import android.app.Activity +import android.content.Context +import android.view.View +import android.view.ViewGroup +import androidx.navigation.NavController +import io.mockk.every import io.mockk.mockk +import io.mockk.mockkObject import io.mockk.verify +import io.mockk.verifyAll import mozilla.components.service.nimbus.NimbusApi import mozilla.components.support.test.libstate.ext.waitUntilIdle import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test +import org.junit.runner.RunWith import org.mozilla.experiments.nimbus.Branch +import org.mozilla.fenix.components.Components +import org.mozilla.fenix.components.FenixSnackbar +import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.getRootView +import org.mozilla.fenix.ext.settings +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.nimbus.controller.NimbusBranchesController +import org.mozilla.fenix.utils.Settings +@RunWith(FenixRobolectricTestRunner::class) class NimbusBranchesControllerTest { private val experiments: NimbusApi = mockk(relaxed = true) private val experimentId = "id" private lateinit var controller: NimbusBranchesController + private lateinit var navController: NavController private lateinit var nimbusBranchesStore: NimbusBranchesStore + private lateinit var settings: Settings + private lateinit var activity: Context + private lateinit var components: Components + private lateinit var metrics: MetricController + private lateinit var snackbar: FenixSnackbar + private lateinit var rootView: View @Before fun setup() { + components = mockk(relaxed = true) + settings = mockk(relaxed = true) + metrics = mockk(relaxed = true) + snackbar = mockk(relaxed = true) + navController = mockk(relaxed = true) + + rootView = mockk(relaxed = true) + activity = mockk(relaxed = true) { + every { findViewById(R.id.content) } returns rootView + every { getRootView() } returns rootView + } + + mockkObject(FenixSnackbar) + every { FenixSnackbar.make(any(), any(), any(), any()) } returns snackbar + + every { activity.settings() } returns settings + every { activity.components.analytics.metrics } returns metrics + + every { navController.currentDestination } returns mockk { + every { id } returns org.mozilla.fenix.R.id.nimbusBranchesFragment + } + nimbusBranchesStore = NimbusBranchesStore(NimbusBranchesState(emptyList())) - controller = NimbusBranchesController(nimbusBranchesStore, experiments, experimentId) + controller = NimbusBranchesController( + activity, + navController, + nimbusBranchesStore, + experiments, + experimentId + ) } @Test fun `WHEN branch item is clicked THEN branch is opted into and selectedBranch state is updated`() { + every { settings.isTelemetryEnabled } returns true + every { settings.isExperimentationEnabled } returns true + + val branch = Branch( + slug = "slug", + ratio = 1 + ) + + controller.onBranchItemClicked(branch) + + nimbusBranchesStore.waitUntilIdle() + + verify { + experiments.optInWithBranch(experimentId, branch.slug) + } + + assertEquals(branch.slug, nimbusBranchesStore.state.selectedBranch) + } + + @Test + fun `WHEN branch item is clicked THEN branch is opted out and selectedBranch state is updated`() { + every { settings.isTelemetryEnabled } returns true + every { settings.isExperimentationEnabled } returns true + every { experiments.getExperimentBranch(experimentId) } returns "slug" + + val branch = Branch( + slug = "slug", + ratio = 1 + ) + + controller.onBranchItemClicked(branch) + + nimbusBranchesStore.waitUntilIdle() + + verify { + experiments.optOut(experimentId) + } + } + + @Test + fun `WHEN studies and telemetry are ON and item is clicked THEN branch is opted in`() { + every { settings.isTelemetryEnabled } returns true + every { settings.isExperimentationEnabled } returns true + + val branch = Branch( + slug = "slug", + ratio = 1 + ) + + controller.onBranchItemClicked(branch) + + nimbusBranchesStore.waitUntilIdle() + + verify { + experiments.optInWithBranch(experimentId, branch.slug) + } + + assertEquals(branch.slug, nimbusBranchesStore.state.selectedBranch) + } + + @Test + fun `WHEN studies and telemetry are Off THEN branch is opted in AND data is not sent`() { + every { settings.isTelemetryEnabled } returns false + every { settings.isExperimentationEnabled } returns false + every { activity.getString(any()) } returns "hello" + + val branch = Branch( + slug = "slug", + ratio = 1 + ) + + controller.onBranchItemClicked(branch) + + nimbusBranchesStore.waitUntilIdle() + + verifyAll { + experiments.getExperimentBranch(experimentId) + experiments.optInWithBranch(experimentId, branch.slug) + snackbar.setText("hello") + } + + assertEquals(branch.slug, nimbusBranchesStore.state.selectedBranch) + } + + @Test + fun `WHEN studies are ON and telemetry Off THEN branch is opted in`() { + every { settings.isExperimentationEnabled } returns true + every { settings.isTelemetryEnabled } returns false + + val branch = Branch( + slug = "slug", + ratio = 1 + ) + + controller.onBranchItemClicked(branch) + + nimbusBranchesStore.waitUntilIdle() + + verify { + experiments.optInWithBranch(experimentId, branch.slug) + } + + assertEquals(branch.slug, nimbusBranchesStore.state.selectedBranch) + } + + @Test + fun `WHEN studies are OFF and telemetry ON THEN branch is opted in`() { + every { settings.isExperimentationEnabled } returns false + every { settings.isTelemetryEnabled } returns true + val branch = Branch( slug = "slug", ratio = 1