Commit c5330c68 authored by Mugurell's avatar Mugurell Committed by Sawyer Blatz
Browse files

For 5092 - Show a Snackbar with retry option if sharing to devices fails (#5158)

* For #5092 - Show a Snackbar with retry option if sharing to devices fails

ShareController will contain all the business logic for checking the result
status of the `share to device` operations. When this fails it will show a
snackbar which also offer the possibility to retry the just failed operation.
To allow this even in the event the user has closed the share fragment we'll
use a GlobalScope's coroutine.
Refactored out the TabsSharedCallback from ShareFragment because otherwise we
would have neede to sent through that the just failed operation. After the
refactor the ShareController is solely responsable for showing the right
snackbar and handling the retry actions.

* For #5092 - Refactor ShareControllerTest

* For #5092: Adds color theming of snackbars
parent 59e2c124
......@@ -20,7 +20,6 @@ import androidx.navigation.NavDestination
import androidx.navigation.fragment.NavHostFragment
import androidx.navigation.ui.AppBarConfiguration
import androidx.navigation.ui.NavigationUI
import com.google.android.material.snackbar.Snackbar
import kotlinx.coroutines.launch
import mozilla.components.browser.search.SearchEngine
import mozilla.components.browser.session.Session
......@@ -35,13 +34,11 @@ import org.mozilla.fenix.browser.UriOpenedObserver
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager
import org.mozilla.fenix.browser.browsingmode.DefaultBrowsingModeManager
import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.metrics.BreadcrumbsRecorder
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.exceptions.ExceptionsFragmentDirections
import org.mozilla.fenix.ext.alreadyOnDestination
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.getRootView
import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.home.HomeFragmentDirections
......@@ -57,12 +54,11 @@ import org.mozilla.fenix.search.SearchFragmentDirections
import org.mozilla.fenix.settings.AboutFragmentDirections
import org.mozilla.fenix.settings.SettingsFragmentDirections
import org.mozilla.fenix.settings.TrackingProtectionFragmentDirections
import org.mozilla.fenix.share.ShareFragment
import org.mozilla.fenix.theme.DefaultThemeManager
import org.mozilla.fenix.theme.ThemeManager
@SuppressWarnings("TooManyFunctions", "LargeClass")
open class HomeActivity : AppCompatActivity(), ShareFragment.TabsSharedCallback {
open class HomeActivity : AppCompatActivity() {
lateinit var themeManager: ThemeManager
lateinit var browsingModeManager: BrowsingModeManager
......@@ -310,17 +306,6 @@ open class HomeActivity : AppCompatActivity(), ShareFragment.TabsSharedCallback
return DefaultThemeManager(browsingModeManager.mode, this)
}
override fun onTabsShared(tabsSize: Int) {
getRootView()?.let {
FenixSnackbar.make(it, Snackbar.LENGTH_SHORT).setText(
getString(
if (tabsSize == 1) R.string.sync_sent_tab_snackbar else
R.string.sync_sent_tabs_snackbar
)
).show()
}
}
companion object {
const val OPEN_TO_BROWSER = "open_to_browser"
const val OPEN_TO_BROWSER_AND_LOAD = "open_to_browser_and_load"
......
......@@ -10,6 +10,7 @@ import android.view.View
import android.view.ViewGroup
import android.widget.FrameLayout
import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.core.content.ContextCompat
import androidx.core.widget.TextViewCompat
import com.google.android.material.snackbar.BaseTransientBottomBar
import com.google.android.material.snackbar.ContentViewCallback
......@@ -23,11 +24,19 @@ import org.mozilla.fenix.test.Mockable
class FenixSnackbar private constructor(
parent: ViewGroup,
content: View,
contentViewCallback: FenixSnackbarCallback
contentViewCallback: FenixSnackbarCallback,
isError: Boolean
) : BaseTransientBottomBar<FenixSnackbar>(parent, content, contentViewCallback) {
init {
view.background = null
view.snackbar_layout.background = if (isError) {
ContextCompat.getDrawable(context, R.drawable.fenix_snackbar_error_background)
} else {
ContextCompat.getDrawable(context, R.drawable.fenix_snackbar_background)
}
content.snackbar_btn.increaseTapArea(actionButtonIncreaseDps)
TextViewCompat.setAutoSizeTextTypeUniformWithConfiguration(
......@@ -64,7 +73,7 @@ class FenixSnackbar private constructor(
private const val actionButtonIncreaseDps = 16
private const val stepGranularity = 1
fun make(view: View, duration: Int): FenixSnackbar {
fun make(view: View, duration: Int, isError: Boolean = false): FenixSnackbar {
val parent = findSuitableParent(view) ?: run {
throw IllegalArgumentException(
"No suitable parent found from the given view. Please provide a valid view."
......@@ -75,7 +84,7 @@ class FenixSnackbar private constructor(
val content = inflater.inflate(R.layout.fenix_snackbar, parent, false)
val callback = FenixSnackbarCallback(content)
return FenixSnackbar(parent, content, callback).also {
return FenixSnackbar(parent, content, callback, isError).also {
it.duration = duration
}
}
......@@ -145,9 +154,10 @@ class FenixSnackbarPresenter(
text: String,
length: Int = FenixSnackbar.LENGTH_LONG,
action: (() -> Unit)? = null,
actionName: String? = null
actionName: String? = null,
isError: Boolean = false
) {
FenixSnackbar.make(view, length).setText(text).let {
FenixSnackbar.make(view, length, isError).setText(text).let {
if (action != null && actionName != null) it.setAction(actionName, action) else it
}.show()
}
......
......@@ -10,14 +10,18 @@ import android.content.Intent.ACTION_SEND
import android.content.Intent.EXTRA_TEXT
import android.content.Intent.FLAG_ACTIVITY_NEW_TASK
import androidx.annotation.VisibleForTesting
import androidx.fragment.app.Fragment
import androidx.navigation.NavController
import com.google.android.material.snackbar.Snackbar
import kotlinx.coroutines.Deferred
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.launch
import mozilla.components.concept.sync.Device
import mozilla.components.concept.sync.TabData
import mozilla.components.feature.sendtab.SendTabUseCases
import org.mozilla.fenix.R
import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.FenixSnackbarPresenter
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.getRootView
import org.mozilla.fenix.ext.metrics
......@@ -42,17 +46,19 @@ interface ShareController {
/**
* Default behavior of [ShareController]. Other implementations are possible.
*
* @param fragment the [ShareFragment] instance this controller handles business logic for.
* @param context [Context] used for various Android interactions.
* @param sharedTabs the list of [ShareTab]s that can be shared.
* @param sendTabUseCases instance of [SendTabUseCases] which allows sending tabs to account devices.
* @param snackbarPresenter - instance of [FenixSnackbarPresenter] for displaying styled snackbars
* @param navController - [NavController] used for navigation.
* @param dismiss - callback signalling sharing can be closed.
*/
@Suppress("TooManyFunctions")
class DefaultShareController(
private val context: Context,
private val fragment: Fragment,
private val sharedTabs: List<ShareTab>,
private val sendTabUseCases: SendTabUseCases,
private val snackbarPresenter: FenixSnackbarPresenter,
private val navController: NavController,
private val dismiss: () -> Unit
) : ShareController {
......@@ -75,7 +81,7 @@ class DefaultShareController(
}
try {
fragment.startActivity(intent)
context.startActivity(intent)
} catch (e: SecurityException) {
context.getRootView()?.let {
FenixSnackbar.make(it, Snackbar.LENGTH_LONG)
......@@ -93,15 +99,11 @@ class DefaultShareController(
override fun handleShareToDevice(device: Device) {
context.metrics.track(Event.SendTab)
sendTabUseCases.sendToDeviceAsync(device.id, sharedTabs.toTabData())
(fragment.activity as ShareFragment.TabsSharedCallback).onTabsShared(sharedTabs.size)
dismiss()
shareToDevicesWithRetry { sendTabUseCases.sendToDeviceAsync(device.id, sharedTabs.toTabData()) }
}
override fun handleShareToAllDevices(devices: List<Device>) {
sendTabUseCases.sendToAllAsync(sharedTabs.toTabData())
(fragment.activity as ShareFragment.TabsSharedCallback).onTabsShared(sharedTabs.size)
dismiss()
shareToDevicesWithRetry { sendTabUseCases.sendToAllAsync(sharedTabs.toTabData()) }
}
override fun handleSignIn() {
......@@ -111,12 +113,51 @@ class DefaultShareController(
dismiss()
}
private fun shareToDevicesWithRetry(shareOperation: () -> Deferred<Boolean>) {
// Use GlobalScope to allow the continuation of this method even if the share fragment is closed.
GlobalScope.launch(Dispatchers.Main) {
when (shareOperation.invoke().await()) {
true -> showSuccess()
false -> showFailureWithRetryOption { shareToDevicesWithRetry(shareOperation) }
}
dismiss()
}
}
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
fun showSuccess() {
snackbarPresenter.present(
getSuccessMessage(),
Snackbar.LENGTH_SHORT
)
}
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
fun showFailureWithRetryOption(operation: () -> Unit) {
snackbarPresenter.present(
text = context.getString(R.string.sync_sent_tab_error_snackbar),
length = Snackbar.LENGTH_LONG,
action = operation,
actionName = context.getString(R.string.sync_sent_tab_error_snackbar_action),
isError = true
)
}
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
fun getSuccessMessage(): String = with(context) {
when (sharedTabs.size) {
1 -> getString(R.string.sync_sent_tab_snackbar)
else -> getString(R.string.sync_sent_tabs_snackbar)
}
}
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
fun getShareText() = sharedTabs.joinToString("\n") { tab -> tab.url }
// Navigation between app fragments uses ShareTab as arguments. SendTabUseCases uses TabData.
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
fun ShareTab.toTabData() = TabData(title, url)
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
fun List<ShareTab>.toTabData() = map { it.toTabData() }
}
......@@ -31,17 +31,15 @@ import mozilla.components.concept.sync.DeviceType
import mozilla.components.feature.sendtab.SendTabUseCases
import mozilla.components.service.fxa.manager.FxaAccountManager
import org.mozilla.fenix.R
import org.mozilla.fenix.components.FenixSnackbarPresenter
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.getRootView
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.share.listadapters.AppShareOption
import org.mozilla.fenix.share.listadapters.SyncShareOption
@Suppress("TooManyFunctions")
class ShareFragment : AppCompatDialogFragment() {
interface TabsSharedCallback {
fun onTabsShared(tabsSize: Int)
}
private lateinit var shareInteractor: ShareInteractor
private lateinit var shareCloseView: ShareCloseView
private lateinit var shareToAccountDevicesView: ShareToAccountDevicesView
......@@ -125,8 +123,8 @@ class ShareFragment : AppCompatDialogFragment() {
shareInteractor = ShareInteractor(
DefaultShareController(
context = requireContext(),
fragment = this,
sharedTabs = tabs,
snackbarPresenter = FenixSnackbarPresenter(activity!!.getRootView()!!),
navController = findNavController(),
sendTabUseCases = SendTabUseCases(accountManager),
dismiss = ::dismiss
......
<?xml version="1.0" encoding="utf-8"?>
<!-- This Source Code Form is subject to the terms of the Mozilla Public
- License, v. 2.0. If a copy of the MPL was not distributed with this
- file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
<shape xmlns:android="http://schemas.android.com/apk/res/android"
android:shape="rectangle">
<solid android:color="@color/snackbar_error_color" />
<corners android:radius="8dp" />
</shape>
......@@ -10,6 +10,7 @@
android:layout_height="match_parent">
<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/snackbar_layout"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_margin="8dp"
......
......@@ -190,6 +190,7 @@
<color name="sync_error_background_color">#FFF36E</color>
<color name="sync_error_text_color">#960E18</color>
<color name="bottom_bar_shadow">#1A000000</color>
<color name="snackbar_error_color">#B52645</color>
<!-- Reader View colors -->
<color name="mozac_feature_readerview_text_color">@color/primary_text_light_theme</color>
......
......@@ -794,6 +794,10 @@
<string name="sync_sent_tabs_snackbar">Tabs sent!</string>
<!-- Text shown in snackbar when one tab has been sent to device -->
<string name="sync_sent_tab_snackbar">Tab sent!</string>
<!-- Text shown in snackbar when sharing tabs failed -->
<string name="sync_sent_tab_error_snackbar">Unable to send</string>
<!-- Text shown in snackbar for the "retry" action that the user has after sharing tabs failed -->
<string name="sync_sent_tab_error_snackbar_action">RETRY</string>
<!-- Title of QR Pairing Fragment -->
<string name="sync_scan_code">Scan the code</string>
<!-- Instructions on how to access pairing -->
......
......@@ -4,20 +4,25 @@
package org.mozilla.fenix.share
import android.app.Activity
import android.content.Context
import android.content.Intent
import androidx.fragment.app.Fragment
import androidx.navigation.NavController
import assertk.assertAll
import assertk.assertThat
import assertk.assertions.isDataClassEqualTo
import assertk.assertions.isEqualTo
import assertk.assertions.isNotEqualTo
import assertk.assertions.isSameAs
import assertk.assertions.isSuccess
import assertk.assertions.isTrue
import com.google.android.material.snackbar.Snackbar
import io.mockk.Runs
import io.mockk.every
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.ObsoleteCoroutinesApi
......@@ -25,12 +30,13 @@ import mozilla.components.concept.sync.Device
import mozilla.components.concept.sync.DeviceType
import mozilla.components.concept.sync.TabData
import mozilla.components.feature.sendtab.SendTabUseCases
import mozilla.components.support.test.robolectric.testContext
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.TestApplication
import org.mozilla.fenix.components.FenixSnackbarPresenter
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.ext.metrics
......@@ -43,9 +49,9 @@ import org.robolectric.annotation.Config
@RunWith(RobolectricTestRunner::class)
@Config(application = TestApplication::class)
class ShareControllerTest {
private val context: Context = mockk(relaxed = true)
// 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)
private val fragment = mockk<Fragment>(relaxed = true)
private val shareTabs = listOf(
ShareTab("url0", "title0"),
ShareTab("url1", "title1")
......@@ -57,9 +63,12 @@ class ShareControllerTest {
)
private val textToShare = "${shareTabs[0].url}\n${shareTabs[1].url}"
private val sendTabUseCases = mockk<SendTabUseCases>(relaxed = true)
private val snackbarPresenter = mockk<FenixSnackbarPresenter>(relaxed = true)
private val navController = mockk<NavController>(relaxed = true)
private val dismiss = mockk<() -> Unit>(relaxed = true)
private val controller = DefaultShareController(context, fragment, shareTabs, sendTabUseCases, navController, dismiss)
private val controller = DefaultShareController(
context, shareTabs, sendTabUseCases, snackbarPresenter, navController, dismiss
)
@Before
fun setUp() {
......@@ -79,9 +88,14 @@ class ShareControllerTest {
val appClassName = "activity"
val appShareOption = AppShareOption("app", mockk(), appPackageName, appClassName)
val shareIntent = slot<Intent>()
every { fragment.startActivity(capture(shareIntent)) } just Runs
// 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, shareTabs, mockk(), mockk(), mockk(), dismiss)
every { activityContext.startActivity(capture(shareIntent)) } just Runs
controller.handleShareToApp(appShareOption)
testController.handleShareToApp(appShareOption)
// Check that the Intent used for querying apps has the expected structre
assertAll {
......@@ -94,7 +108,7 @@ class ShareControllerTest {
assertThat(shareIntent.captured.component!!.className).isEqualTo(appClassName)
}
verifyOrder {
fragment.startActivity(shareIntent.captured)
activityContext.startActivity(shareIntent.captured)
dismiss()
}
}
......@@ -102,13 +116,10 @@ class ShareControllerTest {
@Test
@Suppress("DeferredResultUnused")
fun `handleShareToDevice should share to account device, inform callbacks and dismiss`() {
val deviceToShareTo =
Device("deviceId", "deviceName", DeviceType.UNKNOWN, false, 0L, emptyList(), false, null)
val tabSharedCallbackActivity = mockk<HomeActivity>(relaxed = true)
val sharedTabsNumber = slot<Int>()
val deviceToShareTo = Device(
"deviceId", "deviceName", DeviceType.UNKNOWN, false, 0L, emptyList(), false, null)
val deviceId = slot<String>()
val tabsShared = slot<List<TabData>>()
every { fragment.activity } returns tabSharedCallbackActivity
controller.handleShareToDevice(deviceToShareTo)
......@@ -116,48 +127,35 @@ class ShareControllerTest {
verifyOrder {
metrics.track(Event.SendTab)
sendTabUseCases.sendToDeviceAsync(capture(deviceId), capture(tabsShared))
tabSharedCallbackActivity.onTabsShared(capture(sharedTabsNumber))
dismiss()
// dismiss() is also to be called, but at the moment cannot test it in a coroutine.
}
assertAll {
assertThat(deviceId.isCaptured).isTrue()
assertThat(deviceId.captured).isEqualTo(deviceToShareTo.id)
assertThat(tabsShared.isCaptured).isTrue()
assertThat(tabsShared.captured).isEqualTo(tabsData)
// All current tabs should be shared
assertThat(sharedTabsNumber.isCaptured).isTrue()
assertThat(sharedTabsNumber.captured).isEqualTo(shareTabs.size)
}
}
@Test
@Suppress("DeferredResultUnused")
fun `handleShareToAllDevices calls handleShareToDevice multiple times`() {
val devicesToShareTo = listOf(
Device("deviceId0", "deviceName0", DeviceType.UNKNOWN, false, 0L, emptyList(), false, null),
Device("deviceId1", "deviceName1", DeviceType.UNKNOWN, true, 1L, emptyList(), false, null)
)
val tabSharedCallbackActivity = mockk<HomeActivity>(relaxed = true)
val sharedTabsNumber = slot<Int>()
val tabsShared = slot<List<TabData>>()
every { fragment.activity } returns tabSharedCallbackActivity
controller.handleShareToAllDevices(devicesToShareTo)
// Verify all the needed methods are called. sendTab() should be called for each account device.
verifyOrder {
sendTabUseCases.sendToAllAsync(capture(tabsShared))
tabSharedCallbackActivity.onTabsShared(capture(sharedTabsNumber))
dismiss()
// dismiss() is also to be called, but at the moment cannot test it in a coroutine.
}
assertAll {
// SendTabUseCases should send a the `shareTabs` mapped to tabData
assertThat(tabsShared.isCaptured).isTrue()
assertThat(tabsShared.captured).isEqualTo(tabsData)
// All current tabs should be shared
assertThat(sharedTabsNumber.isCaptured).isTrue()
assertThat(sharedTabsNumber.captured).isEqualTo(shareTabs.size)
}
}
......@@ -188,6 +186,83 @@ class ShareControllerTest {
}
}
@Test
fun `showSuccess should show a snackbar with a success message`() {
val expectedMessage = controller.getSuccessMessage()
val expectedTimeout = Snackbar.LENGTH_SHORT
val messageSlot = slot<String>()
val timeoutSlot = slot<Int>()
controller.showSuccess()
verify { snackbarPresenter.present(capture(messageSlot), capture(timeoutSlot)) }
assertAll {
assertThat(messageSlot.isCaptured).isTrue()
assertThat(timeoutSlot.isCaptured).isTrue()
assertThat(messageSlot.captured).isEqualTo(expectedMessage)
assertThat(timeoutSlot.captured).isEqualTo(expectedTimeout)
}
}
@Test
fun `showFailureWithRetryOption should show a snackbar with a retry action`() {
val expectedMessage = context.getString(R.string.sync_sent_tab_error_snackbar)
val expectedTimeout = Snackbar.LENGTH_LONG
val operation: () -> Unit = { println("Hello World") }
val expectedRetryMessage =
context.getString(R.string.sync_sent_tab_error_snackbar_action)
val messageSlot = slot<String>()
val timeoutSlot = slot<Int>()
val operationSlot = slot<() -> Unit>()
val retryMesageSlot = slot<String>()
val isFailureSlot = slot<Boolean>()
controller.showFailureWithRetryOption(operation)
verify {
snackbarPresenter.present(
capture(messageSlot),
capture(timeoutSlot),
capture(operationSlot),
capture(retryMesageSlot),
capture(isFailureSlot)
)
}
assertAll {
assertThat(messageSlot.isCaptured).isTrue()
assertThat(timeoutSlot.isCaptured).isTrue()
assertThat(operationSlot.isCaptured).isTrue()
assertThat(retryMesageSlot.isCaptured).isTrue()
assertThat(isFailureSlot.isCaptured).isTrue()
assertThat(messageSlot.captured).isEqualTo(expectedMessage)
assertThat(timeoutSlot.captured).isEqualTo(expectedTimeout)
assertThat { operationSlot.captured }.isSuccess().isSameAs(operation)
assertThat(retryMesageSlot.captured).isEqualTo(expectedRetryMessage)
assertThat(isFailureSlot.captured).isEqualTo(true)
}
}
@Test
fun `getSuccessMessage should return different strings depending on the number of shared tabs`() {
val controllerWithOneSharedTab = DefaultShareController(
context, listOf(ShareTab("url0", "title0")), mockk(), mockk(), mockk(), mockk()
)
val controllerWithMoreSharedTabs = controller
val expectedTabSharedMessage = context.getString(R.string.sync_sent_tab_snackbar)
val expectedTabsSharedMessage = context.getString(R.string.sync_sent_tabs_snackbar)
val tabSharedMessage = controllerWithOneSharedTab.getSuccessMessage()
val tabsSharedMessage = controllerWithMoreSharedTabs.getSuccessMessage()
assertAll {
assertThat(tabSharedMessage).isNotEqualTo(tabsSharedMessage)
assertThat(tabSharedMessage).isEqualTo(expectedTabSharedMessage)
assertThat(tabsSharedMessage).isEqualTo(expectedTabsSharedMessage)
}
}
@Test
fun `getShareText should respect concatenate shared tabs urls`() {
assertThat(controller.getShareText()).isEqualTo(textToShare)
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment