Skip to content

[WellSQL Migration] Migrate WCProductVariationModel to Room #14027

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

Merged
merged 28 commits into from
May 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9f715a4
Migrate `WCProductVariationModel` to Room
wzieba Apr 24, 2025
5127a0c
Migrate `getVariations` and `observeVariations` to Room's DAO
wzieba Apr 24, 2025
149d6b4
Migrate `getVariation` to Room's DAO
wzieba Apr 24, 2025
8370dd4
Migrate `upsertVariation`(s) to Room's DAO
wzieba Apr 24, 2025
1df49aa
Migrate `deleteVariation` to Room's DAO
wzieba Apr 24, 2025
5bee63b
Remove unused observability wrappers for variations in `ProductSqlUtils`
wzieba Apr 24, 2025
9d12e9d
Adjust `WooCommerce` module to WCProductVariationModel changes: updat…
wzieba Apr 25, 2025
316ef5f
Fix operations on ProductVariationsDao by passing **local** site id
wzieba Apr 25, 2025
b45f030
Fix `ProductTestUtils` from testFixtures
wzieba Apr 25, 2025
1ce9504
Fix building `fluxc-plugin` tests
wzieba Apr 25, 2025
ac37d37
Ignore variations order in `test that variation insert emits a flow e…
wzieba Apr 25, 2025
d967324
Fix WCProductStore unit tests
wzieba Apr 28, 2025
0e9cf05
Apply code style
wzieba Apr 28, 2025
968135e
Make the `testJalapenoDebugUnitTest` run
wzieba Apr 28, 2025
636b9fa
Fix `WooCommerce` unit tests by mocking suspendable functions to retu…
wzieba Apr 28, 2025
4211e37
Drop `WCProductVariationModel` from WellSql
wzieba Apr 30, 2025
9b5949c
Make `ProductVariationsDao` internal
wzieba Apr 30, 2025
bfab067
Merge branch 'trunk' into migrate_product_variation_to_room
wzieba Apr 30, 2025
886bff1
Merge branch 'feature/product_variation_to_room' into migrate_product…
wzieba Apr 30, 2025
3e6c29c
Merge branch 'migrate_product_variation_to_room' into migrate_product…
wzieba Apr 30, 2025
4263681
Merge pull request #13972 from woocommerce/migrate_product_variation_…
ParaskP7 May 8, 2025
f40384f
Merge pull request #13982 from woocommerce/migrate_product_variation_…
ParaskP7 May 8, 2025
569ffff
Merge branch 'trunk' of github.com:woocommerce/woocommerce-android in…
ParaskP7 May 8, 2025
a440b6b
Fix: Replace payload with result on delete variations for product
ParaskP7 May 8, 2025
375d22a
Refactor: Use local/remote instead of int/long ids for product variation
ParaskP7 May 8, 2025
44c3436
Merge branch 'trunk' of github.com:woocommerce/woocommerce-android in…
ParaskP7 May 8, 2025
2d9650b
Merge branch 'trunk' of github.com:woocommerce/woocommerce-android in…
ParaskP7 May 9, 2025
7d02c49
Merge branch 'trunk' of github.com:woocommerce/woocommerce-android in…
ParaskP7 May 14, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import com.woocommerce.android.ui.products.ProductStatus.PRIVATE
import com.woocommerce.android.ui.products.ProductStatus.PUBLISH
import com.woocommerce.android.ui.products.ProductStockStatus
import kotlinx.parcelize.Parcelize
import org.wordpress.android.fluxc.model.LocalOrRemoteId.RemoteId
import org.wordpress.android.fluxc.model.WCProductVariationModel
import org.wordpress.android.util.DateTimeUtils
import java.math.BigDecimal
Expand Down Expand Up @@ -134,49 +135,71 @@ open class ProductVariation(
} ?: ""
}

return (cachedVariation ?: WCProductVariationModel()).also {
it.remoteProductId = remoteProductId
it.remoteVariationId = remoteVariationId
it.sku = sku
it.globalUniqueId = globalUniqueId
it.image = imageToJson()
it.regularPrice = if (regularPrice.isNotSet()) "" else regularPrice.toString()
it.salePrice = if (salePrice.isNotSet()) "" else salePrice.toString()
if (isSaleScheduled) {
saleStartDateGmt?.let { dateOnSaleFrom ->
it.dateOnSaleFromGmt = dateOnSaleFrom.formatToYYYYmmDDhhmmss()
}
it.dateOnSaleToGmt = saleEndDateGmt?.formatToYYYYmmDDhhmmss() ?: ""
fun getDateOnSaleFromGmt(): String {
return if (isSaleScheduled) {
saleStartDateGmt?.formatToYYYYmmDDhhmmss() ?: ""
} else {
it.dateOnSaleFromGmt = ""
it.dateOnSaleToGmt = ""
""
}
it.stockStatus = ProductStockStatus.fromStockStatus(stockStatus)
it.backorders = ProductBackorderStatus.fromBackorderStatus(backorderStatus)
it.stockQuantity = stockQuantity
it.purchasable = isPurchasable
it.virtual = isVirtual
it.downloadable = isDownloadable
it.manageStock = isStockManaged
it.description = description
it.status = if (isVisible) PUBLISH.value else PRIVATE.value
it.shippingClass = shippingClass
it.shippingClassId = shippingClassId.toInt()
it.menuOrder = menuOrder
it.attributes = JsonArray().toString()
attributes.takeIf { list -> list.isNotEmpty() }
?.forEach { variant -> it.addVariant(variant.asSourceModel()) }
it.length = if (length == 0f) "" else length.formatToString()
it.width = if (width == 0f) "" else width.formatToString()
it.weight = if (weight == 0f) "" else weight.formatToString()
it.height = if (height == 0f) "" else height.formatToString()
it.minAllowedQuantity = minAllowedQuantity ?: -1
it.maxAllowedQuantity = maxAllowedQuantity ?: -1
it.groupOfQuantity = groupOfQuantity ?: -1
it.overrideProductQuantities = overrideProductQuantities ?: false
if (this is SubscriptionProductVariation) {
}

fun getDateOnSaleToGmt(): String {
return if (isSaleScheduled) {
saleEndDateGmt?.formatToYYYYmmDDhhmmss() ?: ""
} else {
""
}
}

fun attributesToJson(): String {
val jsonArray = JsonArray()
attributes.forEach { variantOption ->
JsonObject().apply {
addProperty("id", variantOption.id)
addProperty("name", variantOption.name)
addProperty("option", variantOption.option)
}.also { jsonArray.add(it) }
}
return jsonArray.toString()
}

return (cachedVariation ?: WCProductVariationModel()).copy(
remoteProductId = RemoteId(remoteProductId),
remoteVariationId = RemoteId(remoteVariationId),
sku = sku,
globalUniqueId = globalUniqueId,
image = imageToJson(),
regularPrice = if (regularPrice.isNotSet()) "" else regularPrice.toString(),
salePrice = if (salePrice.isNotSet()) "" else salePrice.toString(),
dateOnSaleFromGmt = getDateOnSaleFromGmt(),
dateOnSaleToGmt = getDateOnSaleToGmt(),
stockStatus = ProductStockStatus.fromStockStatus(stockStatus),
backorders = ProductBackorderStatus.fromBackorderStatus(backorderStatus),
stockQuantity = stockQuantity,
purchasable = isPurchasable,
virtual = isVirtual,
downloadable = isDownloadable,
manageStock = isStockManaged,
description = description,
status = if (isVisible) PUBLISH.value else PRIVATE.value,
shippingClass = shippingClass,
shippingClassId = shippingClassId.toInt(),
menuOrder = menuOrder,
attributes = attributesToJson(),
length = if (length == 0f) "" else length.formatToString(),
width = if (width == 0f) "" else width.formatToString(),
weight = if (weight == 0f) "" else weight.formatToString(),
height = if (height == 0f) "" else height.formatToString(),
minAllowedQuantity = minAllowedQuantity ?: -1,
maxAllowedQuantity = maxAllowedQuantity ?: -1,
groupOfQuantity = groupOfQuantity ?: -1,
overrideProductQuantities = overrideProductQuantities ?: false,
).let {
if (this@ProductVariation is SubscriptionProductVariation) {
// Subscription details are currently the only editable metadata fields from the app.
it.metadata = subscriptionDetails?.toMetadataJson().toString()
it.copy(metadata = subscriptionDetails?.toMetadataJson().toString())
} else {
it
}
}
}
Expand Down Expand Up @@ -283,8 +306,8 @@ data class VariantOption(

fun WCProductVariationModel.toAppModel(): ProductVariation {
return ProductVariation(
remoteProductId = this.remoteProductId,
remoteVariationId = this.remoteVariationId,
remoteProductId = this.remoteProductId.value,
remoteVariationId = this.remoteVariationId.value,
sku = this.sku,
globalUniqueId = this.globalUniqueId,
image = this.getImageModel()?.let {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ class SubscriptionProductVariation(
constructor(model: WCProductVariationModel) :
this(
subscriptionDetails = model.metadata?.let { SubscriptionDetailsMapper.toAppModel(it) },
remoteProductId = model.remoteProductId,
remoteVariationId = model.remoteVariationId,
remoteProductId = model.remoteProductId.value,
remoteVariationId = model.remoteVariationId.value,
sku = model.sku,
globalUniqueId = model.globalUniqueId,
image = model.getImageModel()?.let {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class ProductDetailBottomSheetBuilder(
}
}

private fun ProductAggregate.getShipping(): ProductDetailBottomSheetUiItem? {
private suspend fun ProductAggregate.getShipping(): ProductDetailBottomSheetUiItem? {
return if (!product.isVirtual && !hasShipping) {
ProductDetailBottomSheetUiItem(
ProductDetailBottomSheetType.PRODUCT_SHIPPING,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ import com.woocommerce.android.util.CurrencyFormatter
import com.woocommerce.android.util.PriceUtils
import com.woocommerce.android.util.StringUtils
import com.woocommerce.android.viewmodel.ResourceProvider
import kotlinx.coroutines.runBlocking
import java.math.BigDecimal

@Suppress("LargeClass", "LongParameterList")
Expand Down Expand Up @@ -518,9 +519,11 @@ class ProductDetailCardBuilder(
subscription?.supportsOneTimeShipping ?: false
} else {
// For variable subscription products, we need to check against the variations
variationRepository.getProductVariationList(product.remoteId).all {
(it as? SubscriptionProductVariation)?.subscriptionDetails
?.supportsOneTimeShipping ?: false
runBlocking {
variationRepository.getProductVariationList(product.remoteId).all {
(it as? SubscriptionProductVariation)?.subscriptionDetails
?.supportsOneTimeShipping ?: false
}
}
}
)
Expand Down Expand Up @@ -951,7 +954,7 @@ class ProductDetailCardBuilder(
}
)

private fun Product.warning(): ProductProperty? {
private suspend fun Product.warning(): ProductProperty? {
val variations = variationRepository.getProductVariationList(this.remoteId)

val missingPriceVariation = variations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ class ProductDetailRepository @Inject constructor(

fun isSkuAvailableLocally(sku: String) = runBlocking { !productStore.isProductExists(selectedSite.get(), sku) }

fun getCachedVariationCount(remoteProductId: Long) =
suspend fun getCachedVariationCount(remoteProductId: Long) =
productStore.getVariationsForProduct(selectedSite.get(), remoteProductId).size

fun getTaxClassesForSite(): List<TaxClass> =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class VariationDetailRepository @Inject constructor(
}
}

private fun getCachedWCVariation(remoteProductId: Long, remoteVariationId: Long): WCProductVariationModel? =
private suspend fun getCachedWCVariation(remoteProductId: Long, remoteVariationId: Long): WCProductVariationModel? =
productStore.getVariationByRemoteId(selectedSite.get(), remoteProductId, remoteVariationId)

suspend fun getQuantityRules(remoteProductId: Long, remoteVariationId: Long): QuantityRules? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class VariationRepository @Inject constructor(
/**
* Returns all product variations for a product and current site that are in the database
*/
fun getProductVariationList(remoteProductId: Long): List<ProductVariation> {
suspend fun getProductVariationList(remoteProductId: Long): List<ProductVariation> {
val product = productStore.getProductByRemoteId(selectedSite.get(), remoteProductId)
return productStore.getVariationsForProduct(selectedSite.get(), remoteProductId)
.map {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import com.woocommerce.android.model.ProductVariation
import com.woocommerce.android.model.toAppModel
import com.woocommerce.android.ui.products.ProductStatus.DRAFT
import org.intellij.lang.annotations.Language
import org.wordpress.android.fluxc.model.LocalOrRemoteId
import org.wordpress.android.fluxc.model.LocalOrRemoteId.LocalId
import org.wordpress.android.fluxc.model.LocalOrRemoteId.RemoteId
import org.wordpress.android.fluxc.model.WCProductModel
import org.wordpress.android.fluxc.model.WCProductVariationModel
import java.sql.Date
Expand Down Expand Up @@ -62,8 +63,8 @@ object ProductTestUtils {
): Product {
return WCProductModel(
dateCreated = "2018-01-05T05:14:30Z",
localSiteId = LocalOrRemoteId.LocalId(2),
remoteId = LocalOrRemoteId.RemoteId(productId),
localSiteId = LocalId(2),
remoteId = RemoteId(productId),
parentId = parentID,
status = customStatus ?: "publish",
type = productType ?: if (isVariable) "variable" else "simple",
Expand Down Expand Up @@ -139,18 +140,18 @@ object ProductTestUtils {
isPurchasable: Boolean = true,
productAttributes: String = "",
): ProductVariation {
return WCProductVariationModel(2).apply {
dateCreated = "2018-01-05T05:14:30Z"
localSiteId = 1
remoteProductId = productId
remoteVariationId = variationId
price = amount
image = ""
attributes = productAttributes
virtual = isVirtual
downloadable = isDownloadable
purchasable = isPurchasable
}.toAppModel().also { it.priceWithCurrency = "$10.00" }
return WCProductVariationModel(LocalId(2)).copy(
dateCreated = "2018-01-05T05:14:30Z",
localSiteId = LocalId(1),
remoteProductId = RemoteId(productId),
remoteVariationId = RemoteId(variationId),
price = amount,
image = "",
attributes = productAttributes,
virtual = isVirtual,
downloadable = isDownloadable,
purchasable = isPurchasable,
).toAppModel().also { it.priceWithCurrency = "$10.00" }
}

fun generateProductVariationList(productId: Long = 1L): List<ProductVariation> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.woocommerce.android.viewmodel.BaseUnitTest
import com.woocommerce.android.viewmodel.MultiLiveEvent
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowSnackbar
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import org.assertj.core.api.Assertions.assertThat
import org.junit.Before
import org.junit.Test
Expand Down Expand Up @@ -81,7 +82,7 @@ class VariationListViewModelTest : BaseUnitTest() {
}

@Test
fun `Displays the product variation list view correctly`() {
fun `Displays the product variation list view correctly`() = runTest {
doReturn(variations).whenever(variationRepository).getProductVariationList(productRemoteId)

createViewModel()
Expand All @@ -97,6 +98,9 @@ class VariationListViewModelTest : BaseUnitTest() {
fun `Do not fetch product variations from api when not connected`() =
testBlocking {
doReturn(false).whenever(networkStatus).isConnected()
doReturn(
emptyList<ProductVariation>()
).whenever(variationRepository).getProductVariationList(productRemoteId)

createViewModel()

Expand Down Expand Up @@ -209,6 +213,7 @@ class VariationListViewModelTest : BaseUnitTest() {
@Test
fun `Refresh variations list and hide progress bar if variation generation is successful`() = testBlocking {
// given
doReturn(emptyList<ProductVariation>()).whenever(variationRepository).getProductVariationList(productRemoteId)
val variationCandidates = List(5) { id ->
listOf(VariantOption(id.toLong(), "Number", id.toString()))
}
Expand Down Expand Up @@ -247,6 +252,7 @@ class VariationListViewModelTest : BaseUnitTest() {
// given
variationRepository.stub {
onBlocking { bulkCreateVariations(any(), any()) } doReturn RequestResult.ERROR
onBlocking { getProductVariationList(productRemoteId) } doReturn emptyList()
}
val variationCandidates = List(5) { id ->
listOf(VariantOption(id.toLong(), "Number", id.toString()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.woocommerce.android.ui.products.ProductType
import com.woocommerce.android.ui.products.addons.AddonRepository
import com.woocommerce.android.ui.products.models.ProductProperty
import com.woocommerce.android.ui.products.models.ProductPropertyCard
import com.woocommerce.android.ui.products.variations.VariationRepository
import com.woocommerce.android.viewmodel.BaseUnitTest
import com.woocommerce.android.viewmodel.ResourceProvider
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand Down Expand Up @@ -61,14 +62,18 @@ class ProductDetailCardBuilderTest : BaseUnitTest() {
on { get() } doReturn SiteModel()
}

val variationRepository: VariationRepository = mock {
onBlocking { getProductVariationList(any()) } doReturn emptyList()
}

sut = ProductDetailCardBuilder(
viewModel = viewModel,
selectedSite = selectedSite,
resources = resources,
currencyFormatter = mock(),
parameters = mock(),
addonRepository = addonRepo,
variationRepository = mock(),
variationRepository = variationRepository,
appPrefsWrapper = mock(),
isBlazeEnabled = isBlazeEnabled,
isProductCurrentlyPromoted = mock(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ class ProductDetailViewModelTest : BaseUnitTest() {

private val wooCommerceStore: WooCommerceStore = mock()
private val networkStatus: NetworkStatus = mock()
private val productRepository: ProductDetailRepository = mock()
private val productRepository: ProductDetailRepository = mock {
onBlocking { getCachedVariationCount(any()) } doReturn 0
}
private val productCategoriesRepository: ProductCategoriesRepository = mock()
private val productTagsRepository: ProductTagsRepository = mock()
private val mediaFilesRepository: MediaFilesRepository = mock()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,9 +331,8 @@ class ScanToUpdateInventoryViewModelTest : BaseUnitTest() {
whenever(variationRepo.getVariationOrNull(productId, variationId)).thenReturn(originalVariation)
whenever(variationRepo.updateVariation(any())).thenReturn(
WCProductStore.OnVariationUpdated(
1,
1,
variationId
remoteProductId = 1,
remoteVariationId = variationId
)
)
whenever(
Expand Down Expand Up @@ -374,7 +373,9 @@ class ScanToUpdateInventoryViewModelTest : BaseUnitTest() {
ProductTestUtils.generateProductVariation(productId = 1, variationId = 2)
.copy(stockQuantity = 1.0, isStockManaged = true)
whenever(variationRepo.getVariationOrNull(1, 2)).thenReturn(originalVariation)
whenever(variationRepo.updateVariation(any())).thenReturn(WCProductStore.OnVariationUpdated(1, 1, 2))
whenever(
variationRepo.updateVariation(any())
).thenReturn(WCProductStore.OnVariationUpdated(remoteProductId = 1, remoteVariationId = 2))
whenever(
resourceProvider.getString(
R.string.scan_to_update_inventory_success_snackbar,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.emptyFlow
import kotlinx.coroutines.flow.flowOf
import org.assertj.core.api.Assertions
import org.assertj.core.api.Assertions.assertThat
import org.junit.Before
import org.junit.Test
Expand Down Expand Up @@ -168,7 +167,7 @@ class VariationDetailViewModelTest : BaseUnitTest() {
@Test
fun `Display error message on min-max quantities update product error`() = testBlocking {
val displayErrorMessage = "This is an error message"
var result = WCProductStore.OnVariationUpdated(1, 1, 2)
val result = WCProductStore.OnVariationUpdated(remoteProductId = 1, remoteVariationId = 2)
result.error = WCProductStore.ProductError(
type = WCProductStore.ProductErrorType.INVALID_MIN_MAX_QUANTITY,
message = displayErrorMessage
Expand All @@ -184,7 +183,7 @@ class VariationDetailViewModelTest : BaseUnitTest() {

sut.onUpdateButtonClicked()

Assertions.assertThat(showUpdateProductError?.message)
assertThat(showUpdateProductError?.message)
.isEqualTo(displayErrorMessage)
}

Expand Down
Loading