Merge branch 'trunk' into 12704-many-pop-ups-on-a-fresh-app-login-mov…

…e-products-with-ai-to-products-tab
woocommerce · Oct 1, 2024 · d99241c · d99241c
2 parents a522f6d + ed9b240
commit d99241c
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 15 deletions.
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
@@ -3,12 +3,9 @@
 *** For entries which are touching the Android Wear app's, start entry with `[WEAR]` too.
 20.8
 -----
-
-
-20.7
------
 - [**] Improve barcode scanner reading accuracy [https://github.com/woocommerce/woocommerce-android/pull/12673]
 - [Internal] AI product creation banner is removed [https://github.com/woocommerce/woocommerce-android/pull/12705]
+- [*] [Login] Fix an issue where the app doesn't show the correct error screen when application passwords are disabled [https://github.com/woocommerce/woocommerce-android/pull/12717]
 - [**] Fixed bug with coupons disappearing from the order creation screen unexpectedly [https://github.com/woocommerce/woocommerce-android/pull/12724]
 
 20.6

diff --git a/.../kotlin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModel.kt b/.../kotlin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModel.kt
@@ -15,6 +15,7 @@ import com.woocommerce.android.analytics.AnalyticsTracker
 import com.woocommerce.android.analytics.AnalyticsTrackerWrapper
 import com.woocommerce.android.applicationpasswords.ApplicationPasswordGenerationException
 import com.woocommerce.android.applicationpasswords.ApplicationPasswordsNotifier
+import com.woocommerce.android.extensions.isNotNullOrEmpty
 import com.woocommerce.android.model.UiString
 import com.woocommerce.android.model.UiString.UiStringRes
 import com.woocommerce.android.model.UiString.UiStringText
@@ -85,6 +86,10 @@ class LoginSiteCredentialsViewModel @Inject constructor(
 
     private val loadingMessage = savedStateHandle.getStateFlow(viewModelScope, 0, "loading-message")
 
+    private val SiteModel?.fullAuthorizationUrl: String?
+        get() = this?.applicationPasswordsAuthorizeUrl
+            ?.let { url -> "$url?app_name=$applicationPasswordsClientId&success_url=$REDIRECTION_URL" }
+
     @OptIn(ExperimentalCoroutinesApi::class)
     val viewState = state.flatMapLatest {
         // Reset loading and error state when the state changes
@@ -240,18 +245,14 @@ class LoginSiteCredentialsViewModel @Inject constructor(
             fetchedSiteId.map { if (it == -1) null else wpApiSiteRepository.getSiteByLocalId(it) }
         ) { loadingMessage, errorDialogMessage, site ->
             ViewState.WebAuthorizationViewState(
-                authorizationUrl = generateAuthorizationUrl(site),
+                authorizationUrl = site?.fullAuthorizationUrl,
                 userAgent = userAgent,
                 loadingMessage = loadingMessage,
                 errorDialogMessage = errorDialogMessage
             )
         }
     }
 
-    private fun generateAuthorizationUrl(site: SiteModel?) =
-        site?.applicationPasswordsAuthorizeUrl
-            ?.let { url -> "$url?app_name=$applicationPasswordsClientId&success_url=$REDIRECTION_URL" }
-
     private suspend fun login() {
         val state = requireNotNull(this@LoginSiteCredentialsViewModel.viewState.value as ViewState.NativeLoginViewState)
         loadingMessage.value = R.string.logging_in
@@ -308,10 +309,17 @@ class LoginSiteCredentialsViewModel @Inject constructor(
                     val errorMessage = detectedErrorMessage
                         ?.toPresentableString()
                         ?: resourceProvider.getString(R.string.error_generic)
-                    ShowApplicationPasswordTutorialScreen(
-                        url = generateAuthorizationUrl(site).orEmpty(),
-                        errorMessage = errorMessage
-                    ).let { triggerEvent(it) }
+                    if (site.fullAuthorizationUrl.isNotNullOrEmpty()) {
+                        triggerEvent(
+                            ShowApplicationPasswordTutorialScreen(
+                                url = site.applicationPasswordsAuthorizeUrl,
+                                errorMessage = errorMessage
+                            )
+                        )
+                    } else {
+                        analyticsTracker.track(AnalyticsEvent.APPLICATION_PASSWORDS_AUTHORIZATION_URL_NOT_AVAILABLE)
+                        triggerEvent(ShowApplicationPasswordsUnavailableScreen(siteAddress, site.isJetpackConnected))
+                    }
                 } else {
                     triggerEvent(ShowNonWooErrorScreen(siteAddress))
                 }
@@ -342,7 +350,7 @@ class LoginSiteCredentialsViewModel @Inject constructor(
                     // Otherwise, the web authorization flow will handle the login
                     if (state.value == State.NativeLogin) {
                         fetchUserInfo()
-                    } else if (site.applicationPasswordsAuthorizeUrl == null) {
+                    } else if (site.applicationPasswordsAuthorizeUrl.isNullOrEmpty()) {
                         analyticsTracker.track(AnalyticsEvent.APPLICATION_PASSWORDS_AUTHORIZATION_URL_NOT_AVAILABLE)
                         triggerEvent(ShowApplicationPasswordsUnavailableScreen(siteAddress, site.isJetpackConnected))
                     }

diff --git a/...lin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModelTest.kt b/...lin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModelTest.kt
@@ -19,13 +19,15 @@ import com.woocommerce.android.util.observeForTesting
 import com.woocommerce.android.util.runAndCaptureValues
 import com.woocommerce.android.viewmodel.BaseUnitTest
 import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowSnackbar
+import com.woocommerce.android.viewmodel.ResourceProvider
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.MutableSharedFlow
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.Test
 import org.mockito.kotlin.any
 import org.mockito.kotlin.anyOrNull
 import org.mockito.kotlin.argThat
+import org.mockito.kotlin.doAnswer
 import org.mockito.kotlin.doReturn
 import org.mockito.kotlin.eq
 import org.mockito.kotlin.mock
@@ -72,6 +74,9 @@ class LoginSiteCredentialsViewModelTest : BaseUnitTest() {
     private val analyticsTracker: AnalyticsTrackerWrapper = mock()
     private val appPrefs: AppPrefsWrapper = mock()
     private val userAgent: UserAgent = mock()
+    private val resourceProvider: ResourceProvider = mock {
+        on { getString(any()) } doAnswer { it.arguments[0].toString() }
+    }
 
     private lateinit var viewModel: LoginSiteCredentialsViewModel
 
@@ -93,7 +98,7 @@ class LoginSiteCredentialsViewModelTest : BaseUnitTest() {
             appPrefs = appPrefs,
             userAgent = userAgent,
             applicationPasswordsClientId = clientId,
-            resourceProvider = mock()
+            resourceProvider = resourceProvider
         )
     }
 
@@ -362,4 +367,23 @@ class LoginSiteCredentialsViewModelTest : BaseUnitTest() {
         )
         verify(loginAnalyticsListener).trackFailure(anyOrNull())
     }
+
+    @Test
+    fun `given application pwd disabled and login fails, when attempting to sign in, then show error`() = testBlocking {
+        setup {
+            whenever(wpApiSiteRepository.login(siteAddress, testUsername, testPassword))
+                .thenReturn(Result.failure(Exception()))
+            whenever(wpApiSiteRepository.fetchSite(siteAddress, testUsername, testPassword))
+                .thenReturn(Result.success(testSite.apply { applicationPasswordsAuthorizeUrl = null }))
+        }
+
+        viewModel.viewState.observeForTesting {
+            viewModel.onUsernameChanged(testUsername)
+            viewModel.onPasswordChanged(testPassword)
+            viewModel.onContinueClick()
+        }
+
+        assertThat(viewModel.event.value)
+            .isEqualTo(ShowApplicationPasswordsUnavailableScreen(siteAddress, isJetpackConnected))
+    }
 }