Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Woo] Extract subscriptions outside of WCProductModel table #3106

Merged
merged 9 commits into from
Oct 16, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.model.metadata.MetaDataParentItemType
import org.wordpress.android.fluxc.model.metadata.MetadataChanges
import org.wordpress.android.fluxc.model.metadata.UpdateMetadataRequest
import org.wordpress.android.fluxc.model.metadata.WCMetaData
import org.wordpress.android.fluxc.store.MetaDataStore
Expand Down Expand Up @@ -43,6 +44,7 @@ class CustomFieldsViewModel(
loadCustomFields()
}

@Suppress("LongMethod")
private fun observeLoadingState() {
val customFields = combine(
metaDataStore.observeDisplayableMetaData(
Expand All @@ -63,8 +65,8 @@ class CustomFieldsViewModel(
customFields,
pendingUpdateRequest.map {
it.insertedMetadata.isNotEmpty() ||
it.updatedMetadata.isNotEmpty() ||
it.deletedMetadataIds.isNotEmpty()
it.updatedMetadata.isNotEmpty() ||
it.deletedMetadataIds.isNotEmpty()
}
) { loadingState, metaData, hasChanges ->
when (loadingState) {
Expand All @@ -74,21 +76,27 @@ class CustomFieldsViewModel(
onDelete = { field ->
pendingUpdateRequest.update {
it.copy(
deletedMetadataIds = it.deletedMetadataIds + field.id
metadataChanges = it.metadataChanges.copy(
deletedMetadataIds = it.deletedMetadataIds + field.id
)
)
}
},
onEdit = { field ->
pendingUpdateRequest.update {
it.copy(
updatedMetadata = it.updatedMetadata + field
metadataChanges = it.metadataChanges.copy(
updatedMetadata = it.updatedMetadata + field
)
)
}
},
onAdd = { field ->
pendingUpdateRequest.update {
it.copy(
insertedMetadata = it.insertedMetadata + field
metadataChanges = it.metadataChanges.copy(
insertedMetadata = it.insertedMetadata + field
)
)
}
},
Expand Down Expand Up @@ -134,9 +142,7 @@ class CustomFieldsViewModel(
} else {
pendingUpdateRequest.update {
it.copy(
insertedMetadata = emptyList(),
updatedMetadata = emptyList(),
deletedMetadataIds = emptyList()
metadataChanges = MetadataChanges()
)
}
loadingState.value = LoadingState.Loaded
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,22 @@
package org.wordpress.android.fluxc.network.rest.wpcom.wc.product

import com.google.gson.Gson
import org.assertj.core.api.Assertions.assertThat
import org.junit.Test
import org.mockito.kotlin.any
import org.mockito.kotlin.doAnswer
import org.mockito.kotlin.mock
import org.wordpress.android.fluxc.JsonLoaderUtils.jsonFileAs
import org.wordpress.android.fluxc.model.LocalOrRemoteId
import org.wordpress.android.fluxc.model.metadata.WCMetaData
import org.wordpress.android.fluxc.model.addons.RemoteAddonDto
import org.wordpress.android.fluxc.model.addons.RemoteAddonDto.RemotePriceType.FlatFee
import org.wordpress.android.fluxc.model.addons.RemoteAddonDto.RemoteRestrictionsType.AnyText
import org.wordpress.android.fluxc.model.addons.RemoteAddonDto.RemoteType.Checkbox
import org.wordpress.android.fluxc.model.metadata.WCMetaData
import org.wordpress.android.fluxc.model.metadata.get
import kotlin.test.fail

class ProductDtoMapperTest {
private val productDtoMapper = ProductDtoMapper(
gson = Gson(),
stripProductMetaData = mock {
on { invoke(any()) } doAnswer {
@Suppress("UNCHECKED_CAST")
Expand Down Expand Up @@ -79,18 +77,6 @@ class ProductDtoMapperTest {
assertThat(addonOptions).isNotEmpty
}

@Test
fun `Product metadata is serialized correctly`() {
val productModelUnderTest =
"wc/product-with-addons.json"
.jsonFileAs(ProductApiResponse::class.java)
?.let { productDtoMapper.mapToModel(LocalOrRemoteId.LocalId(0), it) }
?.product

assertThat(productModelUnderTest).isNotNull
assertThat(productModelUnderTest?.metadata).isNotNull
}

@Test
fun `Bundled product with max size is serialized correctly`() {
val product = "wc/product-bundle-with-max-quantity.json"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ object WCLeaderboardsTestFixtures {
origin = SiteModel.ORIGIN_XMLRPC
}
private val productDtoMapper = ProductDtoMapper(
gson = Gson(),
stripProductMetaData = mock {
on { invoke(any()) } doAnswer {
@Suppress("UNCHECKED_CAST")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ open class WellSqlConfig : DefaultWellConfig {
annotation class AddOn

override fun getDbVersion(): Int {
return 204
return 205
}

override fun getDbName(): String {
Expand Down Expand Up @@ -2053,6 +2053,66 @@ open class WellSqlConfig : DefaultWellConfig {
)
""".trimIndent())
}

204 -> migrateAddOn(ADDON_WOOCOMMERCE, version) {
// Drop the column METADATA from WCProductModel
// Since SQLite version varies accross devices, we can't rely on the support of DROP COLUMN,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well spotted as most Android versions won't support alter table -> drop column operation. It seems this was added in SQLite 3.35.0 and it was in Android API 34 when SQLite was updated to a version > 3.35.0

Super minor np, typo in across word

// So we'll use an intermediate table
db.execSQL("""
CREATE TABLE WCProductModel_temp (
_id INTEGER PRIMARY KEY AUTOINCREMENT,LOCAL_SITE_ID INTEGER,
REMOTE_PRODUCT_ID INTEGER,NAME TEXT NOT NULL,SLUG TEXT NOT NULL,PERMALINK TEXT NOT NULL,
DATE_CREATED TEXT NOT NULL,DATE_MODIFIED TEXT NOT NULL,TYPE TEXT NOT NULL,STATUS TEXT NOT NULL,
FEATURED INTEGER,CATALOG_VISIBILITY TEXT NOT NULL,DESCRIPTION TEXT NOT NULL,
SHORT_DESCRIPTION TEXT NOT NULL,SKU TEXT NOT NULL,PRICE TEXT NOT NULL,REGULAR_PRICE TEXT NOT NULL,
SALE_PRICE TEXT NOT NULL,ON_SALE INTEGER,TOTAL_SALES INTEGER,PURCHASABLE INTEGER,
DATE_ON_SALE_FROM TEXT NOT NULL,DATE_ON_SALE_TO TEXT NOT NULL,DATE_ON_SALE_FROM_GMT TEXT NOT NULL,
DATE_ON_SALE_TO_GMT TEXT NOT NULL,VIRTUAL INTEGER,DOWNLOADABLE INTEGER,DOWNLOAD_LIMIT INTEGER,
DOWNLOAD_EXPIRY INTEGER,SOLD_INDIVIDUALLY INTEGER,EXTERNAL_URL TEXT NOT NULL,BUTTON_TEXT TEXT NOT NULL,
TAX_STATUS TEXT NOT NULL,TAX_CLASS TEXT NOT NULL,MANAGE_STOCK INTEGER,STOCK_QUANTITY REAL,
STOCK_STATUS TEXT NOT NULL,BACKORDERS TEXT NOT NULL,BACKORDERS_ALLOWED INTEGER,BACKORDERED INTEGER,
SHIPPING_REQUIRED INTEGER,SHIPPING_TAXABLE INTEGER,SHIPPING_CLASS TEXT NOT NULL,SHIPPING_CLASS_ID INTEGER,
REVIEWS_ALLOWED INTEGER,AVERAGE_RATING TEXT NOT NULL,RATING_COUNT INTEGER,PARENT_ID INTEGER,
PURCHASE_NOTE TEXT NOT NULL,MENU_ORDER INTEGER,CATEGORIES TEXT NOT NULL,TAGS TEXT NOT NULL,
IMAGES TEXT NOT NULL,ATTRIBUTES TEXT NOT NULL,VARIATIONS TEXT NOT NULL,DOWNLOADS TEXT NOT NULL,
RELATED_IDS TEXT NOT NULL,CROSS_SELL_IDS TEXT NOT NULL,UPSELL_IDS TEXT NOT NULL,
GROUPED_PRODUCT_IDS TEXT NOT NULL,WEIGHT TEXT NOT NULL,LENGTH TEXT NOT NULL,WIDTH TEXT NOT NULL,
HEIGHT TEXT NOT NULL,BUNDLED_ITEMS TEXT NOT NULL,COMPOSITE_COMPONENTS TEXT NOT NULL,
SPECIAL_STOCK_STATUS TEXT NOT NULL,BUNDLE_MIN_SIZE REAL,BUNDLE_MAX_SIZE REAL,
MIN_ALLOWED_QUANTITY INTEGER,MAX_ALLOWED_QUANTITY INTEGER,GROUP_OF_QUANTITY INTEGER,
COMBINE_VARIATION_QUANTITIES INTEGER,PASSWORD TEXT,IS_SAMPLE_PRODUCT INTEGER
)
""".trimIndent())

db.execSQL("""
INSERT INTO WCProductModel_temp (
_id, LOCAL_SITE_ID, REMOTE_PRODUCT_ID, NAME, SLUG, PERMALINK, DATE_CREATED, DATE_MODIFIED, TYPE, STATUS, FEATURED,
CATALOG_VISIBILITY, DESCRIPTION, SHORT_DESCRIPTION, SKU, PRICE, REGULAR_PRICE, SALE_PRICE, ON_SALE, TOTAL_SALES,
PURCHASABLE, DATE_ON_SALE_FROM, DATE_ON_SALE_TO, DATE_ON_SALE_FROM_GMT, DATE_ON_SALE_TO_GMT, VIRTUAL, DOWNLOADABLE,
DOWNLOAD_LIMIT, DOWNLOAD_EXPIRY, SOLD_INDIVIDUALLY, EXTERNAL_URL, BUTTON_TEXT, TAX_STATUS, TAX_CLASS, MANAGE_STOCK,
STOCK_QUANTITY, STOCK_STATUS, BACKORDERS, BACKORDERS_ALLOWED, BACKORDERED, SHIPPING_REQUIRED, SHIPPING_TAXABLE,
SHIPPING_CLASS, SHIPPING_CLASS_ID, REVIEWS_ALLOWED, AVERAGE_RATING, RATING_COUNT, PARENT_ID, PURCHASE_NOTE, MENU_ORDER,
CATEGORIES, TAGS, IMAGES, ATTRIBUTES, VARIATIONS, DOWNLOADS, RELATED_IDS, CROSS_SELL_IDS, UPSELL_IDS, GROUPED_PRODUCT_IDS,
WEIGHT, LENGTH, WIDTH, HEIGHT, BUNDLED_ITEMS, COMPOSITE_COMPONENTS, SPECIAL_STOCK_STATUS, BUNDLE_MIN_SIZE, BUNDLE_MAX_SIZE,
MIN_ALLOWED_QUANTITY, MAX_ALLOWED_QUANTITY, GROUP_OF_QUANTITY, COMBINE_VARIATION_QUANTITIES, PASSWORD, IS_SAMPLE_PRODUCT
)
SELECT
_id, LOCAL_SITE_ID, REMOTE_PRODUCT_ID, NAME, SLUG, PERMALINK, DATE_CREATED, DATE_MODIFIED, TYPE, STATUS, FEATURED,
CATALOG_VISIBILITY, DESCRIPTION, SHORT_DESCRIPTION, SKU, PRICE, REGULAR_PRICE, SALE_PRICE, ON_SALE, TOTAL_SALES,
PURCHASABLE, DATE_ON_SALE_FROM, DATE_ON_SALE_TO, DATE_ON_SALE_FROM_GMT, DATE_ON_SALE_TO_GMT, VIRTUAL, DOWNLOADABLE,
DOWNLOAD_LIMIT, DOWNLOAD_EXPIRY, SOLD_INDIVIDUALLY, EXTERNAL_URL, BUTTON_TEXT, TAX_STATUS, TAX_CLASS, MANAGE_STOCK,
STOCK_QUANTITY, STOCK_STATUS, BACKORDERS, BACKORDERS_ALLOWED, BACKORDERED, SHIPPING_REQUIRED, SHIPPING_TAXABLE,
SHIPPING_CLASS, SHIPPING_CLASS_ID, REVIEWS_ALLOWED, AVERAGE_RATING, RATING_COUNT, PARENT_ID, PURCHASE_NOTE, MENU_ORDER,
CATEGORIES, TAGS, IMAGES, ATTRIBUTES, VARIATIONS, DOWNLOADS, RELATED_IDS, CROSS_SELL_IDS, UPSELL_IDS, GROUPED_PRODUCT_IDS,
WEIGHT, LENGTH, WIDTH, HEIGHT, BUNDLED_ITEMS, COMPOSITE_COMPONENTS, SPECIAL_STOCK_STATUS, BUNDLE_MIN_SIZE, BUNDLE_MAX_SIZE,
MIN_ALLOWED_QUANTITY, MAX_ALLOWED_QUANTITY, GROUP_OF_QUANTITY, COMBINE_VARIATION_QUANTITIES, PASSWORD, IS_SAMPLE_PRODUCT
FROM WCProductModel
""".trimIndent())

db.execSQL("DROP TABLE WCProductModel")

db.execSQL("ALTER TABLE WCProductModel_temp RENAME TO WCProductModel")
}
}
}
db.setTransactionSuccessful()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,6 @@ data class WCProductModel(@PrimaryKey @Column private var id: Int = 0) : Identif
@Column var width = ""
@Column var height = ""

/**
* This holds just the subscription keys, for the rest of product's metadata please check [ProductWithMetaData]
*/
@Column var metadata = ""

@Column var bundledItems = ""
@Column var compositeComponents = ""
@Column var specialStockStatus = ""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package org.wordpress.android.fluxc.model.metadata

import com.google.gson.JsonArray
import com.google.gson.JsonNull
import com.google.gson.JsonObject

data class MetadataChanges(
val insertedMetadata: List<WCMetaData> = emptyList(),
val updatedMetadata: List<WCMetaData> = emptyList(),
val deletedMetadataIds: List<Long> = emptyList(),
) {
Comment on lines +7 to +11
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I extracted this bit to a separate class, this class can now be used in other places when needed (now it's used in WCProductStore, but can be used for orders as well), while the UpdateMetadataRequest will still be used for the MetadataStore operations.

My goal of extracting this is to not have to deal with unneeded information for the other usages (I mean parentItemId and parentItemType).

init {
// The ID of inserted metadata is ignored, so to ensure that there is no data loss here,
// we require that all inserted metadata have an ID of 0.
require(insertedMetadata.all { it.id == 0L }) {
"Inserted metadata must have an ID of 0"
}
}

internal fun toJsonArray() = JsonArray().apply {
insertedMetadata.forEach {
add(
JsonObject().apply {
addProperty(WCMetaData.KEY, it.key)
add(WCMetaData.VALUE, it.value.jsonValue)
}
)
}
updatedMetadata.forEach {
add(it.toJson())
}
deletedMetadataIds.forEach {
add(
JsonObject().apply {
addProperty(WCMetaData.ID, it)
add(WCMetaData.VALUE, JsonNull.INSTANCE)
}
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,26 @@ package org.wordpress.android.fluxc.model.metadata
data class UpdateMetadataRequest(
val parentItemId: Long,
val parentItemType: MetaDataParentItemType,
val insertedMetadata: List<WCMetaData> = emptyList(),
val updatedMetadata: List<WCMetaData> = emptyList(),
val deletedMetadataIds: List<Long> = emptyList(),
val metadataChanges: MetadataChanges,
) {
init {
// The ID of inserted metadata is ignored, so to ensure that there is no data loss here,
// we require that all inserted metadata have an ID of 0.
require(insertedMetadata.all { it.id == 0L }) {
"Inserted metadata must have an ID of 0"
}
}
val insertedMetadata: List<WCMetaData> get() = metadataChanges.insertedMetadata
val updatedMetadata: List<WCMetaData> get() = metadataChanges.updatedMetadata
val deletedMetadataIds: List<Long> get() = metadataChanges.deletedMetadataIds

constructor(
parentItemId: Long,
parentItemType: MetaDataParentItemType,
insertedMetadata: List<WCMetaData> = emptyList(),
updatedMetadata: List<WCMetaData> = emptyList(),
deletedMetadataIds: List<Long> = emptyList(),
) : this(
parentItemId,
parentItemType,
MetadataChanges(
insertedMetadata,
updatedMetadata,
deletedMetadataIds,
)
)
}

Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ data class WCMetaData(
val displayValue: WCMetaDataValue? = null
) {
constructor(id: Long, key: String, value: String) : this(id, key,
WCMetaDataValue.fromRawString(value)
WCMetaDataValue(value)
)

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,16 @@ sealed class WCMetaDataValue {
}

companion object {
operator fun invoke(value: String?): WCMetaDataValue {
if (value == null) return StringValue(null)

return runCatching { JsonParser().parse(value) }
.getOrElse { JsonPrimitive(value) }
.let { fromJsonElement(it) }
}
operator fun invoke(value: Number): WCMetaDataValue = NumberValue(value)
operator fun invoke(value: Boolean): WCMetaDataValue = BooleanValue(value)
Comment on lines +81 to +89
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nicer way to instantiate the values instead of the fromRawString version.


internal fun fromJsonElement(element: JsonElement): WCMetaDataValue {
return when {
element.isJsonPrimitive -> {
Expand All @@ -95,10 +105,5 @@ sealed class WCMetaDataValue {
else -> StringValue(element.toString())
}
}

fun fromRawString(value: String): WCMetaDataValue =
runCatching { JsonParser().parse(value) }
.getOrElse { JsonPrimitive(value) }
.let { fromJsonElement(it) }
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package org.wordpress.android.fluxc.network.rest.wpcom.wc.metadata

import com.google.gson.JsonArray
import com.google.gson.JsonNull
import com.google.gson.JsonObject
import org.wordpress.android.fluxc.generated.endpoint.WOOCOMMERCE
import org.wordpress.android.fluxc.model.SiteModel
Expand Down Expand Up @@ -47,32 +45,11 @@ internal class MetaDataRestClient @Inject internal constructor(
MetaDataParentItemType.PRODUCT -> WOOCOMMERCE.products.id(request.parentItemId).pathV3
}

val metaDataJson = JsonArray()
request.insertedMetadata.forEach {
metaDataJson.add(
JsonObject().apply {
addProperty(WCMetaData.KEY, it.key)
add(WCMetaData.VALUE, it.value.jsonValue)
}
)
}
request.updatedMetadata.forEach {
metaDataJson.add(it.toJson())
}
request.deletedMetadataIds.forEach {
metaDataJson.add(
JsonObject().apply {
addProperty(WCMetaData.ID, it)
add(WCMetaData.VALUE, JsonNull.INSTANCE)
}
)
}

val response = wooNetwork.executePostGsonRequest(
site = site,
path = path,
body = mapOf(
"meta_data" to metaDataJson,
"meta_data" to request.metadataChanges.toJsonArray(),
"_fields" to "meta_data"
),
clazz = JsonObject::class.java
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
package org.wordpress.android.fluxc.network.rest.wpcom.wc.product

import com.google.gson.Gson
import org.wordpress.android.fluxc.model.LocalOrRemoteId
import org.wordpress.android.fluxc.model.ProductWithMetaData
import org.wordpress.android.fluxc.model.StripProductMetaData
import org.wordpress.android.fluxc.model.metadata.WCMetaData
import org.wordpress.android.fluxc.model.WCProductModel
import org.wordpress.android.fluxc.model.metadata.WCMetaData
import org.wordpress.android.fluxc.network.utils.getString
import javax.inject.Inject

class ProductDtoMapper @Inject constructor(
private val gson: Gson,
private val stripProductMetaData: StripProductMetaData
) {
@Suppress("LongMethod", "ComplexMethod")
Expand Down Expand Up @@ -109,10 +107,6 @@ class ProductDtoMapper @Inject constructor(
it == "yes"
} ?: false

// Save only the subscription data, the rest of the metadata will be saved separately
metadata = metaData.filter { it.key in WCProductModel.SubscriptionMetadataKeys.ALL_KEYS }
.let { gson.toJson(it) }

password = dto.password

isSampleProduct = dto.metadata?.any {
Expand Down
Loading
Loading