Skip to content

Commit

Permalink
Merge pull request #13645 from woocommerce/issue/13629-jetpack-connec…
Browse files Browse the repository at this point in the history
…tion-api-prep

[Jetpack Setup] Minor refactoring to prepare for Jetpack Connection API integration
  • Loading branch information
hichamboushaba authored Mar 7, 2025
2 parents e6bd397 + 5f58646 commit 05eb2e6
Show file tree
Hide file tree
Showing 12 changed files with 147 additions and 784 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ class FetchJetpackStatus @Inject constructor(
JetpackStatusFetchResponse.Success(
JetpackStatus(
isJetpackInstalled = isJetpackInstalled,
isJetpackConnected = userResult.user!!.isConnected,
wpComEmail = userResult.user!!.wpcomEmail.orNullIfEmpty()
isJetpackConnected = userResult.data!!.isConnected,
wpComEmail = userResult.data!!.wpcomEmail.orNullIfEmpty()
)
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,25 @@ class AccountMismatchRepository @Inject constructor(

suspend fun fetchJetpackConnectionUrl(site: SiteModel): Result<String> {
WooLog.d(WooLog.T.LOGIN, "Fetching Jetpack Connection URL")
val result = jetpackStore.fetchJetpackConnectionUrl(site, autoRegisterSiteIfNeeded = true)
val result = jetpackStore.fetchJetpackConnectionUrl(
site,
useApplicationPasswords = false,
autoRegisterSiteIfNeeded = true
)
return when {
result.isError -> {
WooLog.w(WooLog.T.LOGIN, "Fetching Jetpack Connection URL failed: ${result.error.message}")
Result.failure(OnChangedException(result.error, result.error.message))
}

result.data.isNullOrEmpty() -> {
WooLog.w(WooLog.T.LOGIN, "Fetching Jetpack Connection URL failed, result empty")
Result.failure(IllegalStateException("Response Empty"))
}

else -> {
WooLog.d(WooLog.T.LOGIN, "Jetpack connection URL fetched successfully")
Result.success(result.url)
Result.success(result.data!!)
}
}
}
Expand Down Expand Up @@ -89,11 +98,11 @@ class AccountMismatchRepository @Inject constructor(
}

private suspend fun fetchJetpackUser(site: SiteModel): Result<JetpackUser?> {
return jetpackStore.fetchJetpackUser(site).let {
return jetpackStore.fetchJetpackUser(site, useApplicationPasswords = false).let {
if (it.isError) {
Result.failure(OnChangedException(it.error, it.error.message))
} else {
Result.success(it.user)
Result.success(it.data)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import org.wordpress.android.fluxc.Dispatcher
import org.wordpress.android.fluxc.generated.SiteActionBuilder
import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.store.JetpackStore
import org.wordpress.android.fluxc.store.JetpackStore.JetpackConnectionUrlError
import org.wordpress.android.fluxc.store.JetpackStore.JetpackUserError
import org.wordpress.android.fluxc.store.SiteStore
import org.wordpress.android.fluxc.store.WooCommerceStore
import org.wordpress.android.login.util.SiteUtils
Expand All @@ -40,11 +38,11 @@ class JetpackActivationRepository @Inject constructor(

suspend fun fetchJetpackConnectionUrl(
site: SiteModel,
useApplicationPasswords: Boolean = false
useApplicationPasswords: Boolean
): Result<String> = runWithRetry {
WooLog.d(WooLog.T.LOGIN, "Fetching Jetpack Connection URL")
val result = jetpackStore.fetchJetpackConnectionUrl(
site,
site = site,
autoRegisterSiteIfNeeded = true,
useApplicationPasswords = useApplicationPasswords
)
Expand All @@ -54,33 +52,41 @@ class JetpackActivationRepository @Inject constructor(
Result.failure(OnChangedException(result.error, result.error.message))
}

result.data.isNullOrEmpty() -> {
WooLog.w(WooLog.T.LOGIN, "Fetching Jetpack Connection URL failed, result empty")
Result.failure(IllegalStateException("Response Empty"))
}

else -> {
WooLog.d(WooLog.T.LOGIN, "Jetpack connection URL fetched successfully")
Result.success(result.url)
Result.success(result.data!!)
}
}
}

suspend fun fetchJetpackConnectedEmail(site: SiteModel): Result<String> = runWithRetry {
suspend fun fetchJetpackConnectedEmail(
site: SiteModel,
useApplicationPasswords: Boolean
): Result<String> = runWithRetry {
WooLog.d(WooLog.T.LOGIN, "Fetching email of Jetpack User")
val result = jetpackStore.fetchJetpackUser(site)
val result = jetpackStore.fetchJetpackUser(site = site, useApplicationPasswords = useApplicationPasswords)
return@runWithRetry when {
result.isError -> {
WooLog.w(WooLog.T.LOGIN, "Fetching Jetpack User failed error: $result.error.message")
Result.failure(OnChangedException(result.error, result.error.message))
}

result.user?.wpcomEmail.isNullOrEmpty() -> {
result.data?.wpcomEmail.isNullOrEmpty() -> {
analyticsTrackerWrapper.track(
stat = AnalyticsEvent.LOGIN_JETPACK_SETUP_CANNOT_FIND_WPCOM_USER
)
WooLog.w(WooLog.T.LOGIN, "Cannot find Jetpack Email in response")
Result.failure(JetpackMissingConnectionEmailException)
Result.failure(JetpackMissingConnectionEmailException())
}

else -> {
WooLog.d(WooLog.T.LOGIN, "Jetpack User fetched successfully")
Result.success(result.user!!.wpcomEmail)
Result.success(result.data!!.wpcomEmail)
}
}
}
Expand Down Expand Up @@ -150,11 +156,7 @@ class JetpackActivationRepository @Inject constructor(
},
onFailure = {
if (it is OnChangedException) {
val errorCode = when (it.error) {
is JetpackConnectionUrlError -> it.error.errorCode
is JetpackUserError -> it.error.errorCode
else -> null
}
val errorCode = (it.error as? JetpackStore.JetpackError)?.errorCode
// Skip retrying on 4xx errors
if (errorCode.is4xx()) return Result.failure(it)
}
Expand All @@ -169,5 +171,5 @@ class JetpackActivationRepository @Inject constructor(
return Result.failure(lastError!!)
}

object JetpackMissingConnectionEmailException : RuntimeException("Email missing from response")
class JetpackMissingConnectionEmailException : RuntimeException("Email missing from response")
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import kotlinx.parcelize.Parcelize
import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.store.JetpackStore.JetpackConnectionUrlError
import org.wordpress.android.fluxc.store.JetpackStore.JetpackUserError
import org.wordpress.android.fluxc.store.JetpackStore
import org.wordpress.android.util.UrlUtils
import javax.inject.Inject

Expand Down Expand Up @@ -472,7 +471,7 @@ class JetpackActivationMainViewModel @Inject constructor(
}
},
onFailure = {
val error = (it as? OnChangedException)?.error as? JetpackConnectionUrlError
val error = (it as? OnChangedException)?.error as? JetpackStore.JetpackError

if (isFromBanner) {
analyticsTrackerWrapper.track(
Expand All @@ -498,7 +497,7 @@ class JetpackActivationMainViewModel @Inject constructor(

private suspend fun startJetpackValidation() {
WooLog.d(WooLog.T.LOGIN, "Jetpack Activation: start Jetpack Connection validation")
jetpackActivationRepository.fetchJetpackConnectedEmail(site.await()).fold(
jetpackActivationRepository.fetchJetpackConnectedEmail(site.await(), useApplicationPasswords).fold(
onSuccess = { email ->
jetpackConnectedEmail = email
if (accountRepository.getUserAccount()?.email != email) {
Expand All @@ -517,7 +516,7 @@ class JetpackActivationMainViewModel @Inject constructor(
}
},
onFailure = {
val error = (it as? OnChangedException)?.error as? JetpackUserError
val error = (it as? OnChangedException)?.error as? JetpackStore.JetpackError

if (isFromBanner) {
analyticsTrackerWrapper.track(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class AccountMismatchRepositoryTest : BaseUnitTest() {
@Test
fun `given a non-connected Jetpack Account, when fetching status, then return correct status`() = testBlocking {
whenever(jetpackStore.fetchJetpackUser(any(), eq(false)))
.thenReturn(JetpackStore.JetpackUserResult(createJetpackUser(isConnected = false)))
.thenReturn(JetpackStore.JetpackResult(createJetpackUser(isConnected = false)))

val result = sut.checkJetpackConnection(
siteUrl = "https://example.com",
Expand All @@ -48,7 +48,7 @@ class AccountMismatchRepositoryTest : BaseUnitTest() {
@Test
fun `given a null jetpack user, when fetching connection status, then assume non-connected`() = testBlocking {
whenever(jetpackStore.fetchJetpackUser(any(), eq(false)))
.thenReturn(JetpackStore.JetpackUserResult(null))
.thenReturn(JetpackStore.JetpackResult(null))

val result = sut.checkJetpackConnection(
siteUrl = "https://example.com",
Expand All @@ -62,7 +62,7 @@ class AccountMismatchRepositoryTest : BaseUnitTest() {
@Test
fun `given a connected Jetpack Account, when fetching status, then return correct status`() = testBlocking {
whenever(jetpackStore.fetchJetpackUser(any(), eq(false)))
.thenReturn(JetpackStore.JetpackUserResult(createJetpackUser(isConnected = true, wpcomEmail = "email")))
.thenReturn(JetpackStore.JetpackResult(createJetpackUser(isConnected = true, wpcomEmail = "email")))

val result = sut.checkJetpackConnection(
siteUrl = "https://example.com",
Expand All @@ -77,7 +77,7 @@ class AccountMismatchRepositoryTest : BaseUnitTest() {
@Test
fun `given a correctly connected Jetpack account, when fetching email, then return it`() = testBlocking {
whenever(jetpackStore.fetchJetpackUser(any(), eq(false)))
.thenReturn(JetpackStore.JetpackUserResult(createJetpackUser(isConnected = true, wpcomEmail = "email")))
.thenReturn(JetpackStore.JetpackResult(createJetpackUser(isConnected = true, wpcomEmail = "email")))

val result = sut.fetchJetpackConnectedEmail(SiteModel())

Expand All @@ -87,7 +87,7 @@ class AccountMismatchRepositoryTest : BaseUnitTest() {
@Test
fun `given an issue with Jetpack account, when fetching email, then return error`() = testBlocking {
whenever(jetpackStore.fetchJetpackUser(any(), eq(false)))
.thenReturn(JetpackStore.JetpackUserResult(createJetpackUser(isConnected = true, wpcomEmail = "")))
.thenReturn(JetpackStore.JetpackResult(createJetpackUser(isConnected = true, wpcomEmail = "")))

val result = sut.fetchJetpackConnectedEmail(SiteModel())

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,11 @@ class JetpackActivationMainViewModelTest : BaseUnitTest() {
fun `when connection step succeeds, then start connection validation`() = testBlocking {
setup(isJetpackInstalled = true) {
whenever(
jetpackActivationRepository.fetchJetpackConnectionUrl(site = site)
jetpackActivationRepository.fetchJetpackConnectionUrl(site = site, useApplicationPasswords = false)
).thenReturn(Result.success("https://example.com/connect"))

whenever(
jetpackActivationRepository.fetchJetpackConnectedEmail(site = site)
jetpackActivationRepository.fetchJetpackConnectedEmail(site = site, useApplicationPasswords = false)
).doSuspendableAnswer {
// The duration value is not important, it's just to make sure we suspend at this step
delay(100)
Expand All @@ -170,11 +170,11 @@ class JetpackActivationMainViewModelTest : BaseUnitTest() {
fun `when validation step succeeds, then mark steps as done`() = testBlocking {
setup(isJetpackInstalled = true) {
whenever(
jetpackActivationRepository.fetchJetpackConnectionUrl(site = site)
jetpackActivationRepository.fetchJetpackConnectionUrl(site = site, useApplicationPasswords = false)
).thenReturn(Result.success("https://example.com/connect"))

whenever(
jetpackActivationRepository.fetchJetpackConnectedEmail(site = site)
jetpackActivationRepository.fetchJetpackConnectedEmail(site = site, useApplicationPasswords = false)
).doReturn(Result.success("email"))
}

Expand All @@ -194,19 +194,22 @@ class JetpackActivationMainViewModelTest : BaseUnitTest() {
testBlocking {
setup(isJetpackInstalled = true) {
whenever(
jetpackActivationRepository.fetchJetpackConnectionUrl(site = site)
jetpackActivationRepository.fetchJetpackConnectionUrl(site = site, useApplicationPasswords = false)
).thenReturn(Result.success("https://example.com/connect"))
whenever(
jetpackActivationRepository.fetchJetpackConnectedEmail(site = site)
).thenReturn(Result.failure(JetpackActivationRepository.JetpackMissingConnectionEmailException))
jetpackActivationRepository.fetchJetpackConnectedEmail(site = site, useApplicationPasswords = false)
).thenReturn(Result.failure(JetpackActivationRepository.JetpackMissingConnectionEmailException()))
}

val state = viewModel.viewState.runAndCaptureValues {
viewModel.onJetpackConnectionResult(JetpackActivationWebViewViewModel.ConnectionResult.Success)
viewModel.onRetryClick()
}.last()

verify(jetpackActivationRepository, times(2)).fetchJetpackConnectionUrl(site = site)
verify(jetpackActivationRepository, times(2)).fetchJetpackConnectionUrl(
site = site,
useApplicationPasswords = false
)
assertThat((state as ProgressViewState).steps.single { it.state == StepState.Ongoing }.type)
.isEqualTo(StepType.Connection)
}
Expand All @@ -215,7 +218,7 @@ class JetpackActivationMainViewModelTest : BaseUnitTest() {
fun `when connection is dismissed, then trigger correct event`() = testBlocking {
setup(isJetpackInstalled = true) {
whenever(
jetpackActivationRepository.fetchJetpackConnectionUrl(site = site)
jetpackActivationRepository.fetchJetpackConnectionUrl(site = site, useApplicationPasswords = false)
).thenReturn(Result.success("https://example.com/connect"))
}

Expand All @@ -230,7 +233,7 @@ class JetpackActivationMainViewModelTest : BaseUnitTest() {
fun `when connection fails due to an error, then show correct state`() = testBlocking {
setup(isJetpackInstalled = true) {
whenever(
jetpackActivationRepository.fetchJetpackConnectionUrl(site = site)
jetpackActivationRepository.fetchJetpackConnectionUrl(site = site, useApplicationPasswords = false)
).thenReturn(Result.success("https://example.com/connect"))
}

Expand Down
Loading

0 comments on commit 05eb2e6

Please sign in to comment.