Bug 1877205 - Translations Fetch Supported Languages Refactor

This patch refactors a few things for fetching supported languages:

* `TranslateExpectedAction` is decoupled from fetching languages
* Now use `OperationRequestedAction` w/ `FETCH_SUPPORTED_LANGUAGES`
* Refactors the Fenix fragment to send `OperationRequestedAction` as a
placeholder for data initialization
* `TranslateSetLanguagesAction` is now `SetSupportedLanguagesAction`

android-components/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt

@@ -954,7 +954,7 @@ sealed class TranslationsAction : BrowserAction() {
      * @property tabId The ID of the tab the [EngineSession] that requested the list.
      * @property supportedLanguages The languages the engine supports for translation.
      */
-    data class TranslateSetLanguagesAction(
+    data class SetSupportedLanguagesAction(
         override val tabId: String,
         val supportedLanguages: TranslationSupport?,
     ) : TranslationsAction(), ActionWithTab

android-components/components/browser/state/src/main/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddleware.kt

@@ -8,7 +8,6 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
 import mozilla.components.browser.state.action.BrowserAction
 import mozilla.components.browser.state.action.TranslationsAction
-import mozilla.components.browser.state.action.TranslationsAction.TranslateExpectedAction
 import mozilla.components.browser.state.selector.findTab
 import mozilla.components.browser.state.state.BrowserState
 import mozilla.components.concept.engine.Engine
@@ -40,16 +39,12 @@ class TranslationsMiddleware(
     ) {
         // Pre process actions
         when (action) {
-            is TranslateExpectedAction -> {
-                scope.launch {
-                    requestSupportedLanguages(context, action.tabId)
-                }
-            }
-
             is TranslationsAction.OperationRequestedAction -> {
                 when (action.operation) {
-                    TranslationOperation.FETCH_LANGUAGES -> {
-                        // Bug 1877205
+                    TranslationOperation.FETCH_SUPPORTED_LANGUAGES -> {
+                        scope.launch {
+                            requestSupportedLanguages(context, action.tabId)
+                        }
                     }
                     TranslationOperation.FETCH_PAGE_SETTINGS -> {
                         scope.launch {
@@ -86,7 +81,7 @@ class TranslationsMiddleware(
 
             onSuccess = {
                 context.store.dispatch(
-                    TranslationsAction.TranslateSetLanguagesAction(
+                    TranslationsAction.SetSupportedLanguagesAction(
                         tabId = tabId,
                         supportedLanguages = it,
                     ),
@@ -98,7 +93,7 @@ class TranslationsMiddleware(
                 context.store.dispatch(
                     TranslationsAction.TranslateExceptionAction(
                         tabId = tabId,
-                        operation = TranslationOperation.FETCH_LANGUAGES,
+                        operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES,
                         translationError = TranslationError.CouldNotLoadLanguagesError(it),
                     ),
                 )

android-components/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/TranslationsStateReducer.kt

@@ -83,9 +83,9 @@ internal object TranslationsStateReducer {
                     }
                 }
 
-                TranslationOperation.FETCH_LANGUAGES -> {
+                TranslationOperation.FETCH_SUPPORTED_LANGUAGES -> {
                     // Reset the error state, and then generally expect
-                    // [TranslationsAction.TranslateSetLanguagesAction] to update state in the
+                    // [TranslationsAction.SetSupportedLanguagesAction] to update state in the
                     // success case.
                     state.copyWithTranslationsState(action.tabId) {
                         it.copy(
@@ -127,7 +127,7 @@ internal object TranslationsStateReducer {
                     }
                 }
 
-                TranslationOperation.FETCH_LANGUAGES -> {
+                TranslationOperation.FETCH_SUPPORTED_LANGUAGES -> {
                     state.copyWithTranslationsState(action.tabId) {
                         it.copy(
                             supportedLanguages = null,
@@ -147,7 +147,7 @@ internal object TranslationsStateReducer {
             }
         }
 
-        is TranslationsAction.TranslateSetLanguagesAction ->
+        is TranslationsAction.SetSupportedLanguagesAction ->
             state.copyWithTranslationsState(action.tabId) {
                 it.copy(
                     supportedLanguages = action.supportedLanguages,
@@ -163,10 +163,25 @@ internal object TranslationsStateReducer {
             }
 
         is TranslationsAction.OperationRequestedAction ->
-            state.copyWithTranslationsState(action.tabId) {
-                it.copy(
-                    pageSettings = null,
-                )
+            when (action.operation) {
+                TranslationOperation.FETCH_SUPPORTED_LANGUAGES -> {
+                    state.copyWithTranslationsState(action.tabId) {
+                        it.copy(
+                            supportedLanguages = null,
+                        )
+                    }
+                }
+                TranslationOperation.FETCH_PAGE_SETTINGS -> {
+                    state.copyWithTranslationsState(action.tabId) {
+                        it.copy(
+                            pageSettings = null,
+                        )
+                    }
+                }
+                TranslationOperation.TRANSLATE, TranslationOperation.RESTORE -> {
+                    // No state change for these operations
+                    state
+                }
             }
     }
 

android-components/components/browser/state/src/test/java/mozilla/components/browser/state/action/TranslationsActionTest.kt

@@ -183,7 +183,7 @@ class TranslationsActionTest {
     }
 
     @Test
-    fun `WHEN a TranslateSetLanguagesAction is dispatched AND successful THEN update supportedLanguages`() {
+    fun `WHEN a SetSupportedLanguagesAction is dispatched AND successful THEN update supportedLanguages`() {
         // Initial
         assertEquals(null, tabState().translationsState.supportedLanguages)
 
@@ -192,7 +192,7 @@ class TranslationsActionTest {
         val fromLanguage = Language("es", "Spanish")
         val supportedLanguages = TranslationSupport(listOf(fromLanguage), listOf(toLanguage))
         store.dispatch(
-            TranslationsAction.TranslateSetLanguagesAction(
+            TranslationsAction.SetSupportedLanguagesAction(
                 tabId = tab.id,
                 supportedLanguages = supportedLanguages,
             ),
@@ -235,7 +235,7 @@ class TranslationsActionTest {
         store.dispatch(
             TranslationsAction.TranslateExceptionAction(
                 tabId = tab.id,
-                operation = TranslationOperation.FETCH_LANGUAGES,
+                operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES,
                 translationError = fetchError,
             ),
         ).joinBlocking()
@@ -275,7 +275,7 @@ class TranslationsActionTest {
         store.dispatch(
             TranslationsAction.TranslateSuccessAction(
                 tabId = tab.id,
-                operation = TranslationOperation.FETCH_LANGUAGES,
+                operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES,
             ),
         ).joinBlocking()
         assertEquals(null, tabState().translationsState.translationError)
@@ -306,7 +306,7 @@ class TranslationsActionTest {
     }
 
     @Test
-    fun `WHEN a OperationRequestedAction is dispatched THEN clear pageSettings`() {
+    fun `WHEN a OperationRequestedAction is dispatched for FETCH_PAGE_SETTINGS THEN clear pageSettings`() {
         // Setting first to have a more robust initial state
         assertNull(tabState().translationsState.pageSettings)
 
@@ -337,4 +337,35 @@ class TranslationsActionTest {
         // Action success
         assertNull(tabState().translationsState.pageSettings)
     }
+
+    @Test
+    fun `WHEN a OperationRequestedAction is dispatched for FETCH_SUPPORTED_LANGUAGES THEN clear supportLanguages`() {
+        // Setting first to have a more robust initial state
+        assertNull(tabState().translationsState.supportedLanguages)
+
+        val supportLanguages = TranslationSupport(
+            fromLanguages = listOf(Language("en", "English")),
+            toLanguages = listOf(Language("en", "English")),
+        )
+
+        store.dispatch(
+            TranslationsAction.SetSupportedLanguagesAction(
+                tabId = tab.id,
+                supportedLanguages = supportLanguages,
+            ),
+        ).joinBlocking()
+
+        assertEquals(supportLanguages, tabState().translationsState.supportedLanguages)
+
+        // Action started
+        store.dispatch(
+            TranslationsAction.OperationRequestedAction(
+                tabId = tab.id,
+                operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES,
+            ),
+        ).joinBlocking()
+
+        // Action success
+        assertNull(tabState().translationsState.supportedLanguages)
+    }
 }

android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddlewareTest.kt

@@ -4,7 +4,6 @@
 
 package mozilla.components.browser.state.engine.middleware
 
-import kotlinx.coroutines.launch
 import kotlinx.coroutines.test.runTest
 import mozilla.components.browser.state.action.BrowserAction
 import mozilla.components.browser.state.action.TranslationsAction
@@ -17,6 +16,7 @@ import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.Engine
 import mozilla.components.concept.engine.EngineSession
 import mozilla.components.concept.engine.translate.DetectedLanguages
+import mozilla.components.concept.engine.translate.Language
 import mozilla.components.concept.engine.translate.LanguageSetting
 import mozilla.components.concept.engine.translate.TranslationEngineState
 import mozilla.components.concept.engine.translate.TranslationError
@@ -30,6 +30,7 @@ import mozilla.components.support.test.libstate.ext.waitUntilIdle
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.rule.MainCoroutineRule
 import mozilla.components.support.test.whenever
+import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.mockito.Mockito.spy
@@ -54,6 +55,13 @@ class TranslationsMiddlewareTest {
     private val tabs = spy(listOf(tab))
     private val state = spy(BrowserState(tabs = tabs))
     private val store = spy(BrowserStore(middleware = listOf(translationsMiddleware), initialState = state))
+    private val context = mock<MiddlewareContext<BrowserState, BrowserAction>>()
+
+    @Before
+    fun setup() {
+        whenever(context.store).thenReturn(store)
+        whenever(context.state).thenReturn(state)
+    }
 
     private fun waitForIdle() {
         scope.testScheduler.runCurrent()
@@ -63,43 +71,50 @@ class TranslationsMiddlewareTest {
     }
 
     @Test
-    fun `WHEN TranslateExpectedAction is dispatched AND fetching languages is successful THEN TranslateSetLanguagesAction is dispatched`() {
-        val supportedLanguages = TranslationSupport(null, null)
-        val callbackCaptor = argumentCaptor<((TranslationSupport) -> Unit)>()
-        val action = TranslationsAction.TranslateExpectedAction(tabId = tab.id)
-        val middlewareContext = mock<MiddlewareContext<BrowserState, BrowserAction>>()
+    fun `WHEN OperationRequestedAction is dispatched to fetch languages AND succeeds THEN SetSupportedLanguagesAction is dispatched`() = runTest {
+        val supportedLanguages = TranslationSupport(
+            fromLanguages = listOf(Language("en", "English")),
+            toLanguages = listOf(Language("en", "English")),
+        )
+        val languageCallback = argumentCaptor<((TranslationSupport) -> Unit)>()
 
-        // Important for ensuring that the tests verify on the same store
-        whenever(middlewareContext.store).thenReturn(store)
-        whenever(engine.getSupportedTranslationLanguages(callbackCaptor.capture(), any()))
-            .thenAnswer { callbackCaptor.value(supportedLanguages) }
+        val action =
+            TranslationsAction.OperationRequestedAction(
+                tabId = tab.id,
+                operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES,
+            )
 
-        translationsMiddleware.invoke(middlewareContext, {}, action)
+        translationsMiddleware.invoke(context, {}, action)
+        verify(engine).getSupportedTranslationLanguages(onSuccess = languageCallback.capture(), onError = any())
+        languageCallback.value.invoke(supportedLanguages)
 
         waitForIdle()
 
-        scope.launch {
-            verify(store).dispatch(
-                TranslationsAction.TranslateSetLanguagesAction(
-                    tabId = tab.id,
-                    supportedLanguages = supportedLanguages,
-                ),
-            )
-        }
+        verify(context.store).dispatch(
+            TranslationsAction.SetSupportedLanguagesAction(
+                tabId = tab.id,
+                supportedLanguages = supportedLanguages,
+            ),
+        )
 
         waitForIdle()
     }
 
     @Test
-    fun `WHEN TranslateExpectedAction is dispatched AND fetching languages fails THEN TranslateExceptionAction is dispatched`() {
-        store.dispatch(TranslationsAction.TranslateExpectedAction(tabId = tab.id)).joinBlocking()
+    fun `WHEN OperationRequestedAction is dispatched with FETCH_SUPPORTED_LANGUAGES AND fails THEN TranslateExceptionAction is dispatched`() = runTest {
+        store.dispatch(
+            TranslationsAction.OperationRequestedAction(
+                tabId = tab.id,
+                operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES,
+            ),
+        ).joinBlocking()
 
         waitForIdle()
 
         verify(store).dispatch(
             TranslationsAction.TranslateExceptionAction(
                 tabId = tab.id,
-                operation = TranslationOperation.FETCH_LANGUAGES,
+                operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES,
                 translationError = TranslationError.CouldNotLoadLanguagesError(any()),
             ),
         )

android-components/components/concept/engine/src/main/java/mozilla/components/concept/engine/translate/TranslationOperation.kt

@@ -23,7 +23,7 @@ enum class TranslationOperation {
      * the languages supported for translating both "to" and "from" with their BCP-47 language tag
      * and localized name.
      */
-    FETCH_LANGUAGES,
+    FETCH_SUPPORTED_LANGUAGES,
 
     /**
      * The page related settings the translation engine should fetch.

fenix/app/src/main/java/org/mozilla/fenix/translations/TranslationsDialogFragment.kt

@@ -26,6 +26,7 @@ import com.google.android.material.bottomsheet.BottomSheetDialogFragment
 import mozilla.components.browser.state.action.TranslationsAction
 import mozilla.components.browser.state.selector.findTab
 import mozilla.components.browser.state.store.BrowserStore
+import mozilla.components.concept.engine.translate.TranslationOperation
 import mozilla.components.concept.engine.translate.initialFromLanguage
 import mozilla.components.concept.engine.translate.initialToLanguage
 import mozilla.components.lib.state.ext.observeAsComposableState
@@ -69,8 +70,13 @@ class TranslationsDialogFragment : BottomSheetDialogFragment() {
         container: ViewGroup?,
         savedInstanceState: Bundle?,
     ): View = ComposeView(requireContext()).apply {
-        // Signalling user intention to translate
-        browserStore.dispatch(TranslationsAction.TranslateExpectedAction(args.sessionId))
+        // Signalling need to fetch languages
+        browserStore.dispatch(
+            TranslationsAction.OperationRequestedAction(
+                tabId = args.sessionId,
+                operation = TranslationOperation.FETCH_SUPPORTED_LANGUAGES,
+            ),
+        )
 
         setContent {
             val translationsState = browserStore.observeAsComposableState {