Skip to content

Commit

Permalink
Merge branch 'trunk' of github.com:wordpress-mobile/WordPress-FluxC-A…
Browse files Browse the repository at this point in the history
…ndroid into analysis/add-missing-nullability-annotations-to-all-taxonomy-sqlutils-model-classes
  • Loading branch information
ParaskP7 committed Oct 3, 2023
2 parents 22e6a84 + 88de533 commit a32ed9f
Show file tree
Hide file tree
Showing 16 changed files with 319 additions and 169 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,6 @@ class MockedStack_WCProductsTest : MockedStack_Base() {
assertEquals(25, payload.reviews.size)
assertNull(payload.filterProductIds)
assertNull(payload.filterByStatus)
assertFalse(payload.loadedMore)
assertTrue(payload.canLoadMore)

// Save product reviews to the database
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,8 @@ class WooProductsFragment : StoreSelectingFragment() {
FetchProductReviewsPayload(
site,
productIds = listOf(remoteProductId)
)
),
deletePreviouslyCachedReviews = false
)
prependToLog("Fetched ${result.rowsAffected} product reviews")
}
Expand All @@ -321,7 +322,10 @@ class WooProductsFragment : StoreSelectingFragment() {
coroutineScope.launch {
prependToLog("Submitting request to fetch product reviews for site ${site.id}")
val payload = FetchProductReviewsPayload(site)
val result = wcProductStore.fetchProductReviews(payload)
val result = wcProductStore.fetchProductReviews(
payload,
deletePreviouslyCachedReviews = false
)
prependToLog("Fetched ${result.rowsAffected} product reviews")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class SiteRestClientTest {
}

@Test
fun `returns fetched sites`() = test {
fun `returns fetched sites using filter`() = test {
val response = SiteWPComRestResponse()
response.ID = siteId
val name = "Updated name"
Expand All @@ -123,15 +123,18 @@ class SiteRestClientTest {
sitesResponse.sites = listOf(response)

initSitesResponse(data = sitesResponse)
initSitesFeaturesResponse(data = SitesFeaturesRestResponse(emptyMap()))

val responseModel = restClient.fetchSites(listOf(WPCOM), false)
assertThat(responseModel.sites).hasSize(1)
assertThat(responseModel.sites[0].name).isEqualTo(name)
assertThat(responseModel.sites[0].siteId).isEqualTo(siteId)

assertThat(urlCaptor.lastValue)
assertThat(urlCaptor.firstValue)
.isEqualTo("https://public-api.wordpress.com/rest/v1.2/me/sites/")
assertThat(paramsCaptor.lastValue).isEqualTo(
assertThat(urlCaptor.lastValue)
.isEqualTo("https://public-api.wordpress.com/rest/v1.1/me/sites/features/")
assertThat(paramsCaptor.firstValue).isEqualTo(
mapOf(
"filters" to "wpcom",
"fields" to "ID,URL,name,description,jetpack,jetpack_connection," +
Expand All @@ -141,6 +144,35 @@ class SiteRestClientTest {
)
}

@Test
fun `returns fetched sites when not filtering`() = test {
val response = SiteWPComRestResponse()
response.ID = siteId
val name = "Updated name"
response.name = name
response.URL = "site.com"

val sitesResponse = SitesResponse()
sitesResponse.sites = listOf(response)

initSitesResponse(data = sitesResponse)

val responseModel = restClient.fetchSites(emptyList(), false)
assertThat(responseModel.sites).hasSize(1)
assertThat(responseModel.sites[0].name).isEqualTo(name)
assertThat(responseModel.sites[0].siteId).isEqualTo(siteId)

assertThat(urlCaptor.firstValue)
.isEqualTo("https://public-api.wordpress.com/rest/v1.1/me/sites/")
assertThat(paramsCaptor.firstValue).isEqualTo(
mapOf(
"fields" to "ID,URL,name,description,jetpack,jetpack_connection," +
"visible,is_private,options,plan,capabilities,quota,icon,meta,zendesk_site_meta," +
"organization_id,was_ecommerce_trial"
)
)
}

@Test
fun `fetched sites can filter JP connected package sites`() = test {
val response = SiteWPComRestResponse()
Expand All @@ -155,6 +187,7 @@ class SiteRestClientTest {
sitesResponse.sites = listOf(response)

initSitesResponse(data = sitesResponse)
initSitesFeaturesResponse(data = SitesFeaturesRestResponse(features = emptyMap()))

val responseModel = restClient.fetchSites(listOf(WPCOM), true)

Expand Down Expand Up @@ -521,6 +554,13 @@ class SiteRestClientTest {
return initGetResponse(PostFormatsResponse::class.java, data ?: mock(), error)
}

private suspend fun initSitesFeaturesResponse(
data: SitesFeaturesRestResponse? = null,
error: WPComGsonNetworkError? = null
): Response<SitesFeaturesRestResponse> {
return initGetResponse(SitesFeaturesRestResponse::class.java, data ?: mock(), error)
}

private suspend fun <T> initGetResponse(
clazz: Class<T>,
data: T,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooErrorType.INVALID_RE
import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooPayload
import org.wordpress.android.fluxc.network.rest.wpcom.wc.customer.CustomerRestClient
import org.wordpress.android.fluxc.network.rest.wpcom.wc.customer.dto.CustomerDTO
import org.wordpress.android.fluxc.network.rest.wpcom.wc.customer.dto.CustomerFromAnalyticsDTO
import org.wordpress.android.fluxc.persistence.CustomerSqlUtils
import org.wordpress.android.fluxc.persistence.WellSqlConfig
import org.wordpress.android.fluxc.test
Expand Down Expand Up @@ -395,95 +394,6 @@ class WCCustomerStoreTest {
assertEquals(customerModel, result.model)
}

@Test
fun `given page 1, when fetchCustomersFromAnalytics, then result deleted and stored`() =
test {
// given
val siteModelId = 1
val siteModel = SiteModel().apply { id = siteModelId }
val customerOne: CustomerFromAnalyticsDTO = mock()
val customerTwo: CustomerFromAnalyticsDTO = mock()
val response = arrayOf(customerOne, customerTwo)
whenever(
restClient.fetchCustomersFromAnalytics(
siteModel,
page = 1,
pageSize = 25
)
).thenReturn(WooPayload(response))
val modelOne = WCCustomerModel().apply {
remoteCustomerId = 1L
localSiteId = siteModelId
}
val modelTwo = WCCustomerModel().apply {
remoteCustomerId = 2L
localSiteId = siteModelId
}
whenever(mapper.mapToModel(siteModel, customerOne)).thenReturn(modelOne)
whenever(mapper.mapToModel(siteModel, customerTwo)).thenReturn(modelTwo)

// when
val result = store.fetchCustomersFromAnalytics(siteModel, 1)

// then
assertThat(result.isError).isFalse
assertThat(result.model).isEqualTo(listOf(modelOne, modelTwo))
assertThat(CustomerSqlUtils.getCustomersForSite(siteModel)).isEqualTo(
listOf(modelOne, modelTwo)
)
}

@Test
fun `given page 1 then page 2, when fetchCustomersFromAnalytics, then both result stored`() =
test {
// given
val siteModelId = 1
val siteModel = SiteModel().apply { id = siteModelId }
val customerOne: CustomerFromAnalyticsDTO = mock()
val customerTwo: CustomerFromAnalyticsDTO = mock()
val response = arrayOf(customerOne, customerTwo)
whenever(
restClient.fetchCustomersFromAnalytics(
siteModel,
page = 1,
pageSize = 25
)
).thenReturn(WooPayload(response))
whenever(
restClient.fetchCustomersFromAnalytics(
siteModel,
page = 2,
pageSize = 25
)
).thenReturn(WooPayload(response))
val modelOne = WCCustomerModel().apply {
remoteCustomerId = 1L
localSiteId = siteModelId
}
val modelTwo = WCCustomerModel().apply {
remoteCustomerId = 2L
localSiteId = siteModelId
}
whenever(mapper.mapToModel(siteModel, customerOne)).thenReturn(modelOne)
whenever(mapper.mapToModel(siteModel, customerTwo)).thenReturn(modelTwo)

// when
val result = store.fetchCustomersFromAnalytics(siteModel, 1)
val result2 = store.fetchCustomersFromAnalytics(siteModel, 2)

// then
assertThat(result.isError).isFalse
assertThat(result.model).isEqualTo(listOf(modelOne, modelTwo))
assertThat(CustomerSqlUtils.getCustomersForSite(siteModel)).isEqualTo(
listOf(modelOne, modelTwo)
)
assertThat(result2.isError).isFalse
assertThat(result2.model).isEqualTo(listOf(modelOne, modelTwo))
assertThat(CustomerSqlUtils.getCustomersForSite(siteModel)).isEqualTo(
listOf(modelOne, modelTwo)
)
}

@Test
fun `given error, when fetchCustomersFromAnalytics, then nothing is stored and error`() =
test {
Expand Down
1 change: 1 addition & 0 deletions fluxc-processor/src/main/resources/wp-com-endpoints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
/me/domain-contact-information/
/me/settings/
/me/sites/
/me/sites/features
/me/send-verification-email/
/me/social-login/connect/
/me/username/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class SiteWPAPIRestClient @Inject constructor(
}

wpApiRestUrl = discoveredWpApiUrl
this.url = response?.url ?: cleanedUrl.replaceBefore("://", urlScheme)
this.url = cleanedUrl.replaceBefore("://", urlScheme)
this.username = payload.username
this.password = payload.password
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,23 +132,51 @@ class SiteRestClient @Inject constructor(
val site: SiteModel? = null
) : Payload<SiteError>()

/**
* Fetches the user's sites from WPCom.
* Since the V1.2 endpoint doesn't return the plan features, we will handle the fetch by following two
* different approaches:
* 1. If we don't need any filtering, then we'll simply use the v1.1 endpoint which includes the features.
* 2. If we have some filters, then we'll send two requests: the first one to the v1.2 endpoint to fetch sites
* And the second one to the /me/sites/features to fetch the features separately, the combine the results.
*/
@Suppress("ComplexMethod")
suspend fun fetchSites(filters: List<SiteFilter?>, filterJetpackConnectedPackageSite: Boolean): SitesModel {
val useV2Endpoint = filters.isNotEmpty()
val params = getFetchSitesParams(filters)
val url = WPCOMREST.me.sites.urlV1_2
val url = WPCOMREST.me.sites.let { if (useV2Endpoint) it.urlV1_2 else it.urlV1_1 }
val response = wpComGsonRequestBuilder.syncGetRequest(this, url, params, SitesResponse::class.java)

val siteFeatures = if (useV2Endpoint) {
fetchSitesFeatures().let {
if (it is Error) {
val result = SitesModel()
result.error = it.error
return result
}
(it as Success).data
}
} else null

return when (response) {
is Success -> {
val siteArray = mutableListOf<SiteModel>()
val jetpackCPSiteArray = mutableListOf<SiteModel>()
for (siteResponse in response.data.sites) {
val siteModel = siteResponseToSiteModel(siteResponse)

siteFeatures?.get(siteModel.siteId)?.let {
siteModel.planActiveFeatures = it.joinToString(",")
}

if (siteModel.isJetpackCPConnected) jetpackCPSiteArray.add(siteModel)
// see https://github.com/wordpress-mobile/WordPress-Android/issues/15540#issuecomment-993752880
if (filterJetpackConnectedPackageSite && siteModel.isJetpackCPConnected) continue
siteArray.add(siteModel)
}
SitesModel(siteArray, jetpackCPSiteArray)
}

is Error -> {
val payload = SitesModel(emptyList())
payload.error = response.error
Expand All @@ -157,6 +185,21 @@ class SiteRestClient @Inject constructor(
}
}

private suspend fun fetchSitesFeatures(): Response<Map<Long, List<String>>> {
val url = WPCOMREST.me.sites.features.urlV1_1
return wpComGsonRequestBuilder.syncGetRequest(
restClient = this,
url = url,
params = emptyMap(),
clazz = SitesFeaturesRestResponse::class.java
).let {
when (it) {
is Success -> Success(it.data.features.mapValues { it.value.active })
is Error -> Error(it.error)
}
}
}

private fun getFetchSitesParams(filters: List<SiteFilter?>): Map<String, String> {
val params = mutableMapOf<String, String>()
if (filters.isNotEmpty()) params[FILTERS] = TextUtils.join(",", filters)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package org.wordpress.android.fluxc.network.rest.wpcom.site

data class SitesFeaturesRestResponse(
val features: Map<Long, SiteFeatures>
)

data class SiteFeatures(
val active: List<String>
)
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ package org.wordpress.android.fluxc.model
import com.google.gson.Gson
import com.google.gson.JsonArray
import com.google.gson.JsonObject
import org.wordpress.android.fluxc.model.WCProductModel.SubscriptionMetadataKeys
import org.wordpress.android.fluxc.model.WCProductModel.AddOnsMetadataKeys
import org.wordpress.android.fluxc.model.WCProductModel.OtherKeys
import org.wordpress.android.fluxc.model.WCProductModel.QuantityRulesMetadataKeys
import org.wordpress.android.fluxc.model.WCProductModel.SubscriptionMetadataKeys
import org.wordpress.android.fluxc.utils.EMPTY_JSON_ARRAY
import org.wordpress.android.fluxc.utils.isElementNullOrEmpty
import javax.inject.Inject
Expand All @@ -30,6 +31,7 @@ class StripProductMetaData @Inject internal constructor(private val gson: Gson)
add(AddOnsMetadataKeys.ADDONS_METADATA_KEY)
addAll(QuantityRulesMetadataKeys.ALL_KEYS)
addAll(SubscriptionMetadataKeys.ALL_KEYS)
add(OtherKeys.HEAD_START_POST)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ data class WCProductModel(@PrimaryKey @Column private var id: Int = 0) : Identif
?.find { it.key == ADDONS_METADATA_KEY }
?.addons

val isSampleProduct: Boolean
get() = Gson().fromJson(metadata, Array<WCMetaData>::class.java)
?.any { it.key == OtherKeys.HEAD_START_POST && it.value == "_hs_extra" }
?: false

@Suppress("SwallowedException", "TooGenericExceptionCaught") private val WCMetaData.addons
get() =
try {
Expand Down Expand Up @@ -596,4 +601,8 @@ data class WCProductModel(@PrimaryKey @Column private var id: Int = 0) : Identif
ALLOW_COMBINATION
)
}

object OtherKeys {
const val HEAD_START_POST = "_headstart_post"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1488,8 +1488,7 @@ class ProductRestClient @Inject constructor(
reviews,
productIds,
filterByStatus,
offset > 0,
reviews.size == WCProductStore.NUM_REVIEWS_PER_FETCH
canLoadMore = reviews.size == WCProductStore.NUM_REVIEWS_PER_FETCH
)
} else {
FetchProductReviewsResponsePayload(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,13 @@ object CustomerSqlUtils {
}

fun insertOrUpdateCustomers(customers: List<WCCustomerModel>): Int {
return customers.sumBy { insertOrUpdateCustomer(it) }
return customers.sumOf { insertOrUpdateCustomer(it) }
}

fun insertCustomers(customers: List<WCCustomerModel>) {
customers.forEach {
WellSql.insert(it).asSingleTransaction(true).execute()
}
}
// endregion

Expand Down
Loading

0 comments on commit a32ed9f

Please sign in to comment.