Skip to content

Commit

Permalink
For mozilla-mobile#12565: Pass metrics to share controller
Browse files Browse the repository at this point in the history
  • Loading branch information
@NotWoods @ekager
NotWoods authored and ekager committed Sep 29, 2020
1 parent d767cd1 commit bbaca06
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 64 deletions.
7 changes: 4 additions & 3 deletions app/src/main/java/org/mozilla/fenix/share/ShareController.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import mozilla.components.support.ktx.kotlin.isExtensionUrl
import org.mozilla.fenix.R
import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.metrics
import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.share.listadapters.AppShareOption

Expand Down Expand Up @@ -66,6 +66,7 @@ interface ShareController {
@Suppress("TooManyFunctions")
class DefaultShareController(
private val context: Context,
private val metrics: MetricController,
private val shareSubject: String?,
private val shareData: List<ShareData>,
private val sendTabUseCases: SendTabUseCases,
Expand Down Expand Up @@ -122,7 +123,7 @@ class DefaultShareController(
}

override fun handleShareToDevice(device: Device) {
context.metrics.track(Event.SendTab)
metrics.track(Event.SendTab)
shareToDevicesWithRetry { sendTabUseCases.sendToDeviceAsync(device.id, shareData.toTabData()) }
}

Expand All @@ -131,7 +132,7 @@ class DefaultShareController(
}

override fun handleSignIn() {
context.metrics.track(Event.SignInToSendTab)
metrics.track(Event.SignInToSendTab)
val directions =
ShareFragmentDirections.actionGlobalTurnOnSync(padSnackbar = true)
navController.nav(R.id.shareFragment, directions)
Expand Down
1 change: 1 addition & 0 deletions app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class ShareFragment : AppCompatDialogFragment() {
shareInteractor = ShareInteractor(
DefaultShareController(
context = requireContext(),
metrics = requireComponents.analytics.metrics,
shareSubject = args.shareSubject,
shareData = shareData,
snackbar = FenixSnackbar.make(
Expand Down
133 changes: 72 additions & 61 deletions app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import android.content.Context
import android.content.Intent
import androidx.navigation.NavController
import com.google.android.material.snackbar.Snackbar
import io.mockk.MockKAnnotations
import io.mockk.Runs
import io.mockk.every
import io.mockk.impl.annotations.MockK
import io.mockk.just
import io.mockk.mockk
import io.mockk.slot
import io.mockk.spyk
import io.mockk.verify
import io.mockk.verifyOrder
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand All @@ -38,17 +39,22 @@ import org.mozilla.fenix.R
import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.ext.metrics
import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
import org.mozilla.fenix.share.listadapters.AppShareOption

@RunWith(FenixRobolectricTestRunner::class)
@ExperimentalCoroutinesApi
class ShareControllerTest {
// Need a valid context to retrieve Strings for example, but we also need it to return our "metrics"
private val context: Context = spyk(testContext)
private val metrics: MetricController = mockk(relaxed = true)

@MockK(relaxed = true) private lateinit var metrics: MetricController
@MockK(relaxed = true) private lateinit var sendTabUseCases: SendTabUseCases
@MockK(relaxed = true) private lateinit var snackbar: FenixSnackbar
@MockK(relaxed = true) private lateinit var navController: NavController
@MockK(relaxed = true) private lateinit var dismiss: (ShareController.Result) -> Unit
@MockK private lateinit var recentAppStorage: RecentAppsStorage

private val context: Context = testContext
private val shareSubject = "shareSubject"
private val shareData = listOf(
ShareData(url = "url0", title = "title0"),
Expand All @@ -61,41 +67,36 @@ class ShareControllerTest {
)
private val textToShare = "${shareData[0].url}\n\n${shareData[1].url}"
private val testCoroutineScope = TestCoroutineScope()
private val sendTabUseCases = mockk<SendTabUseCases>(relaxed = true)
private val snackbar = mockk<FenixSnackbar>(relaxed = true)
private val navController = mockk<NavController>(relaxed = true)
private val dismiss = mockk<(ShareController.Result) -> Unit>(relaxed = true)
private val recentAppStorage = mockk<RecentAppsStorage>(relaxed = true)
private val controller = DefaultShareController(
context, shareSubject, shareData, sendTabUseCases, snackbar, navController,
recentAppStorage, testCoroutineScope, dismiss
)

@Before
fun setUp() {
every { context.metrics } returns metrics
MockKAnnotations.init(this)
}

@Test
fun `handleShareClosed should call a passed in delegate to close this`() {
val controller = buildController()
controller.handleShareClosed()

verify { dismiss(ShareController.Result.DISMISSED) }
}

@Test
fun `handleShareToApp should start a new sharing activity and close this`() = runBlocking {
val appPackageName = "package"
val appClassName = "activity"
val appShareOption = AppShareOption("app", mockk(), appPackageName, appClassName)
val appShareOption = AppShareOption(
name = "app",
icon = mockk(),
packageName = "package",
activityName = "activity"
)
val shareIntent = slot<Intent>()
// Our share Intent uses `FLAG_ACTIVITY_NEW_TASK` but when resolving the startActivity call
// needed for capturing the actual Intent used the `slot` one doesn't have this flag so we
// need to use an Activity Context.
val activityContext: Context = mockk<Activity>()
val testController = DefaultShareController(activityContext, shareSubject, shareData, mockk(),
mockk(), mockk(), recentAppStorage, testCoroutineScope, dismiss)
every { activityContext.startActivity(capture(shareIntent)) } just Runs
val activityContext: Context = mockk<Activity> {
every { startActivity(capture(shareIntent)) } just Runs
}
val testController = buildController(context = activityContext)
every { recentAppStorage.updateRecentApp(appShareOption.activityName) } just Runs

testController.handleShareToApp(appShareOption)
Expand All @@ -107,8 +108,8 @@ class ShareControllerTest {
assertEquals(textToShare, shareIntent.captured.extras!![Intent.EXTRA_TEXT])
assertEquals("text/plain", shareIntent.captured.type)
assertEquals(Intent.FLAG_ACTIVITY_NEW_TASK, shareIntent.captured.flags)
assertEquals(appPackageName, shareIntent.captured.component!!.packageName)
assertEquals(appClassName, shareIntent.captured.component!!.className)
assertEquals("package", shareIntent.captured.component!!.packageName)
assertEquals("activity", shareIntent.captured.component!!.className)

verify { recentAppStorage.updateRecentApp(appShareOption.activityName) }
verifyOrder {
Expand All @@ -119,18 +120,21 @@ class ShareControllerTest {

@Test
fun `handleShareToApp should dismiss with an error start when a security exception occurs`() {
val appPackageName = "package"
val appClassName = "activity"
val appShareOption = AppShareOption("app", mockk(), appPackageName, appClassName)
val appShareOption = AppShareOption(
name = "app",
icon = mockk(),
packageName = "package",
activityName = "activity"
)
val shareIntent = slot<Intent>()
// Our share Intent uses `FLAG_ACTIVITY_NEW_TASK` but when resolving the startActivity call
// needed for capturing the actual Intent used the `slot` one doesn't have this flag so we
// need to use an Activity Context.
val activityContext: Context = mockk<Activity>()
val testController = DefaultShareController(activityContext, shareSubject, shareData, mockk(),
snackbar, mockk(), mockk(), testCoroutineScope, dismiss)
every { activityContext.startActivity(capture(shareIntent)) } throws SecurityException()
every { activityContext.getString(R.string.share_error_snackbar) } returns "Cannot share to this app"
val activityContext: Context = mockk<Activity> {
every { startActivity(capture(shareIntent)) } throws SecurityException()
every { getString(R.string.share_error_snackbar) } returns "Cannot share to this app"
}
val testController = buildController(context = activityContext)

testController.handleShareToApp(appShareOption)

Expand Down Expand Up @@ -175,6 +179,7 @@ class ShareControllerTest {
val deviceId = slot<String>()
val tabsShared = slot<List<TabData>>()

val controller = buildController()
controller.handleShareToDevice(deviceToShareTo)

// Verify all the needed methods are called.
Expand All @@ -199,6 +204,7 @@ class ShareControllerTest {
)
val tabsShared = slot<List<TabData>>()

val controller = buildController()
controller.handleShareToAllDevices(devicesToShareTo)

verifyOrder {
Expand All @@ -213,6 +219,7 @@ class ShareControllerTest {

@Test
fun `handleSignIn should navigate to the Sync Fragment and dismiss this one`() {
val controller = buildController()
controller.handleSignIn()

verifyOrder {
Expand All @@ -227,6 +234,7 @@ class ShareControllerTest {

@Test
fun `handleReauth should navigate to the Account Problem Fragment and dismiss this one`() {
val controller = buildController()
controller.handleReauth()

verifyOrder {
Expand All @@ -240,6 +248,7 @@ class ShareControllerTest {

@Test
fun `showSuccess should show a snackbar with a success message`() {
val controller = buildController()
val expectedMessage = controller.getSuccessMessage()
val expectedTimeout = Snackbar.LENGTH_SHORT

Expand All @@ -259,7 +268,7 @@ class ShareControllerTest {
val expectedRetryMessage =
context.getString(R.string.sync_sent_tab_error_snackbar_action)

controller.showFailureWithRetryOption(operation)
buildController().showFailureWithRetryOption(operation)

verify {
snackbar.apply {
Expand All @@ -273,18 +282,12 @@ class ShareControllerTest {

@Test
fun `getSuccessMessage should return different strings depending on the number of shared tabs`() {
val controllerWithOneSharedTab = DefaultShareController(
context,
shareSubject,
listOf(ShareData(url = "url0", title = "title0")),
mockk(),
mockk(),
mockk(),
mockk(),
mockk(),
mockk()
val controllerWithOneSharedTab = buildController(
data = listOf(ShareData(url = "url0", title = "title0"))
)
val controllerWithMoreSharedTabs = buildController(
data = shareData
)
val controllerWithMoreSharedTabs = controller
val expectedTabSharedMessage = context.getString(R.string.sync_sent_tab_snackbar)
val expectedTabsSharedMessage = context.getString(R.string.sync_sent_tabs_snackbar)

Expand All @@ -298,7 +301,7 @@ class ShareControllerTest {

@Test
fun `getShareText should respect concatenate shared tabs urls`() {
assertEquals(textToShare, controller.getShareText())
assertEquals(textToShare, buildController(data = shareData).getShareText())
}

@Test
Expand All @@ -308,56 +311,64 @@ class ShareControllerTest {
ShareData(url = "moz-extension://eb8df45a-895b-4f3a-896a-c0c71ae5/page.html?url=url0"),
ShareData(url = "url1")
)
val controller = DefaultShareController(
context, shareSubject, shareData, sendTabUseCases, snackbar, navController,
recentAppStorage, testCoroutineScope, dismiss
)
val controller = buildController(data = shareData)

val expectedShareText = "${shareData[0].url}\n\nurl0\n\n${shareData[2].url}"
assertEquals(expectedShareText, controller.getShareText())
}

@Test
fun `getShareSubject will return "shareSubject" if that is non null`() {
assertEquals(shareSubject, controller.getShareSubject())
assertEquals(shareSubject, buildController(subject = shareSubject).getShareSubject())
}

@Test
fun `getShareSubject will return a concatenation of tab titles if "shareSubject" is null`() {
val controller = DefaultShareController(
context, null, shareData, sendTabUseCases, snackbar, navController,
recentAppStorage, testCoroutineScope, dismiss
)
val controller = buildController(subject = null)

assertEquals("title0, title1", controller.getShareSubject())
}

@Test
fun `ShareTab#toTabData maps a list of ShareTab to a TabData list`() {
var tabData: List<TabData>

with(controller) {
tabData = shareData.toTabData()
val tabData = with(buildController()) {
shareData.toTabData()
}

assertEquals(tabsData, tabData)
}

@Test
fun `ShareTab#toTabData creates a data url from text if no url is specified`() {
var tabData: List<TabData>
val expected = listOf(
TabData(title = "title0", url = ""),
TabData(title = "title1", url = "data:,Hello%2C%20World!")
)

with(controller) {
tabData = listOf(
val tabData = with(buildController()) {
listOf(
ShareData(title = "title0"),
ShareData(title = "title1", text = "Hello, World!")
).toTabData()
}

assertEquals(expected, tabData)
}

private fun buildController(
context: Context = this.context,
subject: String? = shareSubject,
data: List<ShareData> = shareData
) = DefaultShareController(
context = context,
metrics = metrics,
shareSubject = subject,
shareData = data,
sendTabUseCases = sendTabUseCases,
snackbar = snackbar,
navController = navController,
recentAppsStorage = recentAppStorage,
viewLifecycleScope = testCoroutineScope,
dismiss = dismiss
)
}

0 comments on commit bbaca06

Please sign in to comment.