Bug 1880490 - Moving Translations `supportedLanguages`

This patch refactors Translations `supportedLanguages` from `[SessionState.translationsState]` to `[BrowserState.translationEngine]` in AC. * Additionally adds `InitTranslationsBrowserState` to signal populating `[BrowserState.translationEngine]`. * This patch also makes adjustments to migrates Fenix's `TranslationsDialogBinding` and `TranslationsBinding` to also monitor this new store location.
3 months ago · 4bb2681d5d
parent bef24c2b0e
commit 4bb2681d5d
7 changed files with 130 additions and 62 deletions
--- a/app/src/main/java/org/mozilla/fenix/browser/TranslationsBinding.kt
+++ b/app/src/main/java/org/mozilla/fenix/browser/TranslationsBinding.kt
@ -5,6 +5,7 @@
 package org.mozilla.fenix.browser

 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChangedBy
 import kotlinx.coroutines.flow.mapNotNull
 import mozilla.components.browser.state.selector.findTab
@ -15,6 +16,7 @@ import mozilla.components.concept.engine.translate.Language
 import mozilla.components.concept.engine.translate.initialFromLanguage
 import mozilla.components.concept.engine.translate.initialToLanguage
 import mozilla.components.lib.state.helpers.AbstractBinding
+import org.mozilla.fenix.translations.TranslationsFlowState

 /**
 * A binding for observing [TranslationsState] changes
@ -36,37 +38,62 @@ class TranslationsBinding(
 ) : AbstractBinding<BrowserState>(browserStore) {

    override suspend fun onState(flow: Flow<BrowserState>) {
-        flow.mapNotNull { state -> state.findTab(sessionId) }.distinctUntilChangedBy {
-            it.translationsState
-        }.collect { sessionState ->
-            val translationsState = sessionState.translationsState
+        // Browser level flows
+        val browserFlow = flow.mapNotNull { state -> state }
+            .distinctUntilChangedBy {
+                it.translationEngine
+            }

-            if (translationsState.isTranslated) {
-                val fromSelected = translationsState.translationEngineState?.initialFromLanguage(
-                    translationsState.supportedLanguages?.fromLanguages,
-                )
-                val toSelected = translationsState.translationEngineState?.initialToLanguage(
-                    translationsState.supportedLanguages?.toLanguages,
+        // Session level flows
+        val sessionFlow = flow.mapNotNull { state -> state.findTab(sessionId) }
+            .distinctUntilChangedBy {
+                it.translationsState
+            }
+
+        // Applying the flows together
+        sessionFlow
+            .combine(browserFlow) { sessionState, browserState ->
+                TranslationsFlowState(
+                    sessionState,
+                    browserState,
                )
+            }
+            .collect { state ->
+                // Browser Translations State Behavior (Global)
+                val browserTranslationsState = state.browserState.translationEngine
+                val translateFromLanguages =
+                    browserTranslationsState.supportedLanguages?.fromLanguages
+                val translateToLanguages =
+                    browserTranslationsState.supportedLanguages?.toLanguages

-                if (fromSelected != null && toSelected != null) {
+                // Session Translations State Behavior (Tab)
+                val sessionTranslationsState = state.sessionState.translationsState
+                if (sessionTranslationsState.isTranslated) {
+                    val fromSelected = sessionTranslationsState.translationEngineState?.initialFromLanguage(
+                        translateFromLanguages,
+                    )
+                    val toSelected = sessionTranslationsState.translationEngineState?.initialToLanguage(
+                        translateToLanguages,
+                    )
+
+                    if (fromSelected != null && toSelected != null) {
+                        onStateUpdated(
+                            true,
+                            true,
+                            fromSelected,
+                            toSelected,
+                        )
+                    }
+                } else if (sessionTranslationsState.isExpectedTranslate) {
                    onStateUpdated(
                        true,
-                        true,
-                        fromSelected,
-                        toSelected,
+                        false,
+                        null,
+                        null,
                    )
+                } else {
+                    onStateUpdated(false, false, null, null)
                }
-            } else if (translationsState.isExpectedTranslate) {
-                onStateUpdated(
-                    true,
-                    false,
-                    null,
-                    null,
-                )
-            } else {
-                onStateUpdated(false, false, null, null)
            }
-        }
    }
 }
--- a/app/src/main/java/org/mozilla/fenix/translations/TranslationsDialogBinding.kt
+++ b/app/src/main/java/org/mozilla/fenix/translations/TranslationsDialogBinding.kt
@ -5,17 +5,20 @@
 package org.mozilla.fenix.translations

 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChangedBy
 import kotlinx.coroutines.flow.mapNotNull
 import mozilla.components.browser.state.selector.findTab
 import mozilla.components.browser.state.state.BrowserState
+import mozilla.components.browser.state.state.TabSessionState
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.translate.initialFromLanguage
 import mozilla.components.concept.engine.translate.initialToLanguage
 import mozilla.components.lib.state.helpers.AbstractBinding

 /**
- * Helper for observing Translation state from [BrowserStore].
+ * Helper for observing Translation state from both [BrowserState.translationEngine]
+ * and [TabSessionState.translationsState].
 */
 class TranslationsDialogBinding(
    browserStore: BrowserStore,
@ -24,17 +27,59 @@ class TranslationsDialogBinding(
    private val getTranslatedPageTitle: (localizedFrom: String?, localizedTo: String?) -> String,
 ) : AbstractBinding<BrowserState>(browserStore) {

+    @Suppress("LongMethod")
    override suspend fun onState(flow: Flow<BrowserState>) {
-        flow.mapNotNull { state -> state.findTab(sessionId) }
+        // Browser level flows
+        val browserFlow = flow.mapNotNull { state -> state }
+            .distinctUntilChangedBy {
+                it.translationEngine
+            }
+
+        // Session level flows
+        val sessionFlow = flow.mapNotNull { state -> state.findTab(sessionId) }
            .distinctUntilChangedBy {
                it.translationsState
            }
-            .collect { sessionState ->
-                val translationsState = sessionState.translationsState

+        // Applying the flows together
+        sessionFlow
+            .combine(browserFlow) { sessionState, browserState -> TranslationsFlowState(sessionState, browserState) }
+            .collect {
+                    state ->
+                // Browser Translations State Behavior (Global)
+                val browserTranslationsState = state.browserState.translationEngine
+                val translateFromLanguages =
+                    browserTranslationsState.supportedLanguages?.fromLanguages
+                translateFromLanguages?.let {
+                    translationsDialogStore.dispatch(
+                        TranslationsDialogAction.UpdateTranslateFromLanguages(
+                            translateFromLanguages,
+                        ),
+                    )
+                }
+
+                val translateToLanguages =
+                    browserTranslationsState.supportedLanguages?.toLanguages
+                translateToLanguages?.let {
+                    translationsDialogStore.dispatch(
+                        TranslationsDialogAction.UpdateTranslateToLanguages(
+                            translateToLanguages,
+                        ),
+                    )
+                }
+
+                // Dispatch engine level errors
+                if (browserTranslationsState.engineError != null) {
+                    translationsDialogStore.dispatch(
+                        TranslationsDialogAction.UpdateTranslationError(browserTranslationsState.engineError),
+                    )
+                }
+
+                // Session Translations State Behavior (Tab)
+                val sessionTranslationsState = state.sessionState.translationsState
                val fromSelected =
-                    translationsState.translationEngineState?.initialFromLanguage(
-                        translationsState.supportedLanguages?.fromLanguages,
+                    sessionTranslationsState.translationEngineState?.initialFromLanguage(
+                        translateFromLanguages,
                    )

                fromSelected?.let {
@ -46,8 +91,8 @@ class TranslationsDialogBinding(
                }

                val toSelected =
-                    translationsState.translationEngineState?.initialToLanguage(
-                        translationsState.supportedLanguages?.toLanguages,
+                    sessionTranslationsState.translationEngineState?.initialToLanguage(
+                        translateToLanguages,
                    )
                toSelected?.let {
                    translationsDialogStore.dispatch(
@ -71,35 +116,18 @@ class TranslationsDialogBinding(
                    )
                }

-                val translateFromLanguages = translationsState.supportedLanguages?.fromLanguages
-                translateFromLanguages?.let {
-                    translationsDialogStore.dispatch(
-                        TranslationsDialogAction.UpdateTranslateFromLanguages(
-                            translateFromLanguages,
-                        ),
-                    )
-                }
-
-                val translateToLanguages = translationsState.supportedLanguages?.toLanguages
-                translateToLanguages?.let {
-                    translationsDialogStore.dispatch(
-                        TranslationsDialogAction.UpdateTranslateToLanguages(
-                            translateToLanguages,
-                        ),
-                    )
-                }
-
-                if (translationsState.isTranslateProcessing) {
+                if (sessionTranslationsState.isTranslateProcessing) {
                    updateStoreIfIsTranslateProcessing()
                }

-                if (translationsState.isTranslated && !translationsState.isTranslateProcessing) {
+                if (sessionTranslationsState.isTranslated && !sessionTranslationsState.isTranslateProcessing) {
                    updateStoreIfTranslated()
                }

-                if (translationsState.translationError != null) {
+                // A session error may override a browser error
+                if (sessionTranslationsState.translationError != null) {
                    translationsDialogStore.dispatch(
-                        TranslationsDialogAction.UpdateTranslationError(translationsState.translationError),
+                        TranslationsDialogAction.UpdateTranslationError(sessionTranslationsState.translationError),
                    )
                }
            }
--- a/app/src/main/java/org/mozilla/fenix/translations/TranslationsDialogStore.kt
+++ b/app/src/main/java/org/mozilla/fenix/translations/TranslationsDialogStore.kt
@ -25,11 +25,7 @@ class TranslationsDialogStore(
    initialState,
    TranslationsDialogReducer::reduce,
    middlewares,
-) {
-    init {
-        dispatch(TranslationsDialogAction.FetchSupportedLanguages)
-    }
-}
+)

 /**
 * The current state of the Translations bottom sheet dialog.
--- a/app/src/main/java/org/mozilla/fenix/translations/TranslationsFlowState.kt
+++ b/app/src/main/java/org/mozilla/fenix/translations/TranslationsFlowState.kt
@ -0,0 +1,19 @@
+/* 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/. */
+
+package org.mozilla.fenix.translations
+
+import mozilla.components.browser.state.state.BrowserState
+import mozilla.components.browser.state.state.TabSessionState
+
+/**
+ * Convenience method to create a named pair for the translations flow.
+ *
+ * @property sessionState The session or tab state.
+ * @property browserState The browser or global state.
+ */
+data class TranslationsFlowState(
+    val sessionState: TabSessionState,
+    val browserState: BrowserState,
+)
--- a/app/src/test/java/org/mozilla/fenix/browser/TranslationsBindingTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/browser/TranslationsBindingTest.kt
@ -83,7 +83,6 @@ class TranslationsBindingTest {

            browserStore.dispatch(
                TranslationsAction.SetSupportedLanguagesAction(
-                    tabId = tab.id,
                    supportedLanguages = supportLanguages,
                ),
            ).joinBlocking()
--- a/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt
@ -86,6 +86,7 @@ class TelemetryMiddlewareTest {
        val engine: Engine = mockk()
        every { engine.enableExtensionProcessSpawning() } just runs
        every { engine.disableExtensionProcessSpawning() } just runs
+        every { engine.getSupportedTranslationLanguages(any(), any()) } just runs
        every { engine.isTranslationsEngineSupported(any(), any()) } just runs
        store = BrowserStore(
            middleware = listOf(telemetryMiddleware) + EngineMiddleware.create(engine),
--- a/app/src/test/java/org/mozilla/fenix/translations/TranslationsDialogBindingTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/translations/TranslationsDialogBindingTest.kt
@ -88,7 +88,6 @@ class TranslationsDialogBindingTest {

            browserStore.dispatch(
                TranslationsAction.SetSupportedLanguagesAction(
-                    tabId = tab.id,
                    supportedLanguages = supportLanguages,
                ),
            ).joinBlocking()
@ -199,7 +198,6 @@ class TranslationsDialogBindingTest {
            val supportedLanguages = TranslationSupport(listOf(fromLanguage), listOf(toLanguage))
            browserStore.dispatch(
                TranslationsAction.SetSupportedLanguagesAction(
-                    tabId = tab.id,
                    supportedLanguages = supportedLanguages,
                ),
            ).joinBlocking()