Merge pull request #8156 from woocommerce/issue/8110-user-role

JorgeMucientes · web-flow · commit bad1d31720f1 · 2023-01-13T12:22:23.000+01:00
[REST API] Check user role
diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/AppInitializer.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/AppInitializer.kt
@@ -192,11 +192,15 @@ class AppInitializer @Inject constructor() : ApplicationLifecycleListener {
     }
 
     override fun onFirstActivityResumed() {
-        // Update the WP.com account details, settings, and site list every time the app is completely restarted,
-        // only if the logged in
-        if (networkStatus.isConnected() && accountStore.hasAccessToken()) {
-            dispatcher.dispatch(AccountActionBuilder.newFetchAccountAction())
-            dispatcher.dispatch(AccountActionBuilder.newFetchSettingsAction())
+        // App is completely restarted
+        if (networkStatus.isConnected()) {
+            if (accountStore.hasAccessToken()) {
+                // Update the WPCom account if the user is signed in using a WPCom account
+                dispatcher.dispatch(AccountActionBuilder.newFetchAccountAction())
+                dispatcher.dispatch(AccountActionBuilder.newFetchSettingsAction())
+            }
+
+            // Update the list of sites
             appCoroutineScope.launch {
                 wooCommerceStore.fetchWooCommerceSites()
 
@@ -212,7 +216,7 @@ class AppInitializer @Inject constructor() : ApplicationLifecycleListener {
                 }
             }
 
-            // Update the user info for the currently logged in user
+            // Update the user info
             if (selectedSite.exists()) {
                 userEligibilityFetcher.fetchUserEligibility()
             }
diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/model/User.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/model/User.kt
@@ -14,11 +14,11 @@ data class User(
     val roles: List<String>
 ) : Parcelable {
     fun getUserNameForDisplay(): String {
-        val name = "$firstName $lastName"
+        val name = "$firstName $lastName".trim()
         return when {
-            name.isEmpty() && username.isEmpty() -> email
-            name.isEmpty() && username.isNotEmpty() -> username
-            else -> name
+            name.isNotEmpty() -> name
+            username.isNotEmpty() -> username
+            else -> email
         }
     }
 }
diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/common/UserEligibilityErrorViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/common/UserEligibilityErrorViewModel.kt
@@ -6,8 +6,7 @@ import com.woocommerce.android.AppPrefs
 import com.woocommerce.android.R.string
 import com.woocommerce.android.model.User
 import com.woocommerce.android.model.toAppModel
-import com.woocommerce.android.util.WooLog
-import com.woocommerce.android.util.WooLog.T
+import com.woocommerce.android.ui.login.AccountRepository
 import com.woocommerce.android.viewmodel.LiveDataDelegate
 import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.Exit
 import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.Logout
@@ -16,46 +15,32 @@ import com.woocommerce.android.viewmodel.ScopedViewModel
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.launch
 import kotlinx.parcelize.Parcelize
-import org.greenrobot.eventbus.Subscribe
-import org.greenrobot.eventbus.ThreadMode
-import org.wordpress.android.fluxc.Dispatcher
-import org.wordpress.android.fluxc.generated.AccountActionBuilder
-import org.wordpress.android.fluxc.generated.SiteActionBuilder
-import org.wordpress.android.fluxc.store.AccountStore
-import org.wordpress.android.fluxc.store.AccountStore.OnAccountChanged
 import javax.inject.Inject
 
 @HiltViewModel
 class UserEligibilityErrorViewModel @Inject constructor(
     savedState: SavedStateHandle,
     private val appPrefs: AppPrefs,
-    private val dispatcher: Dispatcher,
-    private val accountStore: AccountStore,
+    private val accountRepository: AccountRepository,
     private val userEligibilityFetcher: UserEligibilityFetcher
 ) : ScopedViewModel(savedState) {
     final val viewStateData = LiveDataDelegate(savedState, ViewState())
     private var viewState by viewStateData
 
-    init {
-        dispatcher.register(this)
-    }
-
-    override fun onCleared() {
-        super.onCleared()
-        dispatcher.unregister(this)
-    }
-
-    final fun start() {
+    fun start() {
         val email = appPrefs.getUserEmail()
         if (email.isNotEmpty()) {
             val user = userEligibilityFetcher.getUserByEmail(email)
             viewState = viewState.copy(user = user?.toAppModel())
         }
     }
 
-    fun onLogoutButtonClicked() {
-        dispatcher.dispatch(AccountActionBuilder.newSignOutAction())
-        dispatcher.dispatch(SiteActionBuilder.newRemoveWpcomAndJetpackSitesAction())
+    fun onLogoutButtonClicked() = launch {
+        accountRepository.logout().let {
+            if (it) {
+                triggerEvent(Logout)
+            }
+        }
     }
 
     fun onRetryButtonClicked() {
@@ -80,18 +65,4 @@ class UserEligibilityErrorViewModel @Inject constructor(
         val user: User? = null,
         val isProgressDialogShown: Boolean? = null
     ) : Parcelable
-
-    @Suppress("unused")
-    @Subscribe(threadMode = ThreadMode.MAIN)
-    fun onAccountChanged(event: OnAccountChanged) {
-        if (event.isError) {
-            WooLog.e(
-                T.LOGIN,
-                "Account error [type = ${event.causeOfChange}] : " +
-                    "${event.error.type} > ${event.error.message}"
-            )
-        } else if (!accountStore.hasAccessToken()) {
-            triggerEvent(Logout)
-        }
-    }
 }
diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/sitecredentials/LoginSiteCredentialsViewModel.kt
@@ -11,6 +11,7 @@ import com.woocommerce.android.applicationpasswords.ApplicationPasswordsNotifier
 import com.woocommerce.android.model.UiString.UiStringRes
 import com.woocommerce.android.model.UiString.UiStringText
 import com.woocommerce.android.tools.SelectedSite
+import com.woocommerce.android.ui.common.UserEligibilityFetcher
 import com.woocommerce.android.ui.login.WPApiSiteRepository
 import com.woocommerce.android.viewmodel.MultiLiveEvent
 import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.Exit
@@ -50,7 +51,8 @@ class LoginSiteCredentialsViewModel @Inject constructor(
     private val selectedSite: SelectedSite,
     private val loginAnalyticsListener: LoginAnalyticsListener,
     private val resourceProvider: ResourceProvider,
-    applicationPasswordsNotifier: ApplicationPasswordsNotifier
+    applicationPasswordsNotifier: ApplicationPasswordsNotifier,
+    private val userEligibilityFetcher: UserEligibilityFetcher
 ) : ScopedViewModel(savedStateHandle) {
     companion object {
         const val SITE_ADDRESS_KEY = "site-address"
@@ -100,8 +102,17 @@ class LoginSiteCredentialsViewModel @Inject constructor(
 
     fun onContinueClick() = launch {
         loginAnalyticsListener.trackSubmitClicked()
-        isLoading.value = true
+        if (selectedSite.exists()) {
+            // The login already succeeded, proceed to fetching user info
+            fetchUserInfo()
+        } else {
+            login()
+        }
+    }
+
+    private suspend fun login() {
         val state = requireNotNull(this@LoginSiteCredentialsViewModel.state.value)
+        isLoading.value = true
         wpApiSiteRepository.login(
             url = siteAddress,
             username = state.username,
@@ -111,7 +122,6 @@ class LoginSiteCredentialsViewModel @Inject constructor(
                 checkWooStatus(it)
             },
             onFailure = { exception ->
-                isLoading.value = false
                 var errorMessage: Int? = null
                 if (exception is OnChangedException && exception.error is AuthenticationError) {
                     errorMessage = exception.error.toErrorMessage()
@@ -137,6 +147,7 @@ class LoginSiteCredentialsViewModel @Inject constructor(
                 )
             }
         )
+        isLoading.value = false
     }
 
     private suspend fun checkWooStatus(site: SiteModel) {
@@ -146,7 +157,7 @@ class LoginSiteCredentialsViewModel @Inject constructor(
                 if (isWooInstalled) {
                     loginAnalyticsListener.trackAnalyticsSignIn(false)
                     selectedSite.set(site)
-                    triggerEvent(LoggedIn(site.id))
+                    fetchUserInfo()
                 } else {
                     triggerEvent(ShowNonWooErrorScreen(siteAddress))
                 }
@@ -158,6 +169,18 @@ class LoginSiteCredentialsViewModel @Inject constructor(
         isLoading.value = false
     }
 
+    private suspend fun fetchUserInfo() {
+        isLoading.value = true
+        val userInfo = userEligibilityFetcher.fetchUserInfo()
+        if (userInfo != null) {
+            userEligibilityFetcher.updateUserInfo(userInfo)
+            triggerEvent(LoggedIn(selectedSite.getSelectedSiteId()))
+        } else {
+            triggerEvent(ShowSnackbar(R.string.error_generic))
+        }
+        isLoading.value = false
+    }
+
     fun onResetPasswordClick() {
         triggerEvent(ShowResetPasswordScreen(siteAddress))
     }
diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/common/UserEligibilityErrorViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/common/UserEligibilityErrorViewModelTest.kt
@@ -6,42 +6,32 @@ import com.woocommerce.android.R.string
 import com.woocommerce.android.extensions.takeIfNotEqualTo
 import com.woocommerce.android.model.toAppModel
 import com.woocommerce.android.ui.common.UserEligibilityErrorViewModel.ViewState
+import com.woocommerce.android.ui.login.AccountRepository
 import com.woocommerce.android.viewmodel.BaseUnitTest
-import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.*
+import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.Exit
+import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.Logout
+import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowSnackbar
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.Before
 import org.junit.Test
-import org.mockito.kotlin.KArgumentCaptor
 import org.mockito.kotlin.any
-import org.mockito.kotlin.argumentCaptor
-import org.mockito.kotlin.clearInvocations
 import org.mockito.kotlin.doReturn
 import org.mockito.kotlin.mock
-import org.mockito.kotlin.spy
 import org.mockito.kotlin.times
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
-import org.wordpress.android.fluxc.Dispatcher
-import org.wordpress.android.fluxc.action.AccountAction.SIGN_OUT
-import org.wordpress.android.fluxc.action.SiteAction
-import org.wordpress.android.fluxc.annotations.action.Action
 import org.wordpress.android.fluxc.model.user.WCUserModel
-import org.wordpress.android.fluxc.store.AccountStore
-import org.wordpress.android.fluxc.store.AccountStore.OnAccountChanged
-import kotlin.test.assertEquals
 import kotlin.test.assertFalse
 import kotlin.test.assertTrue
 
 @ExperimentalCoroutinesApi
 class UserEligibilityErrorViewModelTest : BaseUnitTest() {
     private val appPrefsWrapper: AppPrefs = mock()
-    private val dispatcher: Dispatcher = mock()
-    private val accountStore: AccountStore = mock()
+    private val accountRepository: AccountRepository = mock()
     private val userEligibilityFetcher: UserEligibilityFetcher = mock()
 
     private lateinit var viewModel: UserEligibilityErrorViewModel
-    private lateinit var actionCaptor: KArgumentCaptor<Action<*>>
 
     private val testUser = WCUserModel().apply {
         remoteUserId = 1L
@@ -56,22 +46,11 @@ class UserEligibilityErrorViewModelTest : BaseUnitTest() {
 
     @Before
     fun setup() {
-        actionCaptor = argumentCaptor()
-
-        viewModel = spy(
-            UserEligibilityErrorViewModel(
-                SavedStateHandle(),
-                appPrefsWrapper,
-                dispatcher,
-                accountStore,
-                userEligibilityFetcher
-            )
-        )
-
-        clearInvocations(
-            viewModel,
-            userEligibilityFetcher,
-            appPrefsWrapper
+        viewModel = UserEligibilityErrorViewModel(
+            SavedStateHandle(),
+            appPrefsWrapper,
+            accountRepository,
+            userEligibilityFetcher
         )
     }
 
@@ -149,23 +128,17 @@ class UserEligibilityErrorViewModelTest : BaseUnitTest() {
     }
 
     @Test
-    fun `Handles logout button click correctly`() {
-        doReturn(false).whenever(accountStore).hasAccessToken()
+    fun `Handles logout button click correctly`() = testBlocking {
+        doReturn(true).whenever(accountRepository).logout()
 
         var logoutEvent: Logout? = null
         viewModel.event.observeForever {
-            if (it is Logout) logoutEvent = it
+            logoutEvent = it as? Logout
         }
 
         viewModel.onLogoutButtonClicked()
 
-        // note that we expect two dispatches because there's one to sign out the user and
-        // the other to remove WPcom and Jetpack sites from local db
-        verify(dispatcher, times(2)).dispatch(actionCaptor.capture())
-        assertEquals(SIGN_OUT, actionCaptor.firstValue.type)
-        assertEquals(SiteAction.REMOVE_WPCOM_AND_JETPACK_SITES, actionCaptor.secondValue.type)
-
-        viewModel.onAccountChanged(OnAccountChanged())
+        verify(accountRepository).logout()
         assertThat(logoutEvent).isNotNull
     }
 }
diff --git a/build.gradle b/build.gradle
@@ -99,7 +99,7 @@ tasks.register("installGitHooks", Copy) {
 }
 
 ext {
-    fluxCVersion = '2.10.0'
+    fluxCVersion = 'trunk-463c753ae48797fd0543f237f668b322345eb69b'
     glideVersion = '4.13.2'
     coilVersion = '2.1.0'
     constraintLayoutVersion = '1.2.0'

Original file line number	Diff line number	Diff line change
`@@ -14,11 +14,11 @@ data class User(`
`14`	`14`	`val roles: List<String>`
`15`	`15`	`) : Parcelable {`
`16`	`16`	`fun getUserNameForDisplay(): String {`
`17`		`- val name = "$firstName $lastName"`
	`17`	`+ val name = "$firstName $lastName".trim()`
`18`	`18`	`return when {`
`19`		`- name.isEmpty() && username.isEmpty() -> email`
`20`		`- name.isEmpty() && username.isNotEmpty() -> username`
`21`		`- else -> name`
	`19`	`+ name.isNotEmpty() -> name`
	`20`	`+ username.isNotEmpty() -> username`
	`21`	`+ else -> email`
`22`	`22`	`}`
`23`	`23`	`}`
`24`	`24`	`}`
Original file line number	Diff line number	Diff line change
`@@ -99,7 +99,7 @@ tasks.register("installGitHooks", Copy) {`
`99`	`99`	`}`
`100`	`100`
`101`	`101`	`ext {`
`102`		`- fluxCVersion = '2.10.0'`
	`102`	`+ fluxCVersion = 'trunk-463c753ae48797fd0543f237f668b322345eb69b'`
`103`	`103`	`glideVersion = '4.13.2'`
`104`	`104`	`coilVersion = '2.1.0'`
`105`	`105`	`constraintLayoutVersion = '1.2.0'`