Skip to content

[1/2] WCProductVariationModel to Room: working app #13972

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

Open
wants to merge 21 commits into
base: feature/product_variation_to_room
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 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
076f143
Fix missing buildkite test collector support when JaCoCo fails
wzieba Apr 29, 2025
9d416db
Run `jacocoTestReport` on the same Gradle invocation. Upload code cov…
wzieba Apr 29, 2025
8bec355
Add missing modules test tasks to JacocoReport dependencies
wzieba Apr 29, 2025
6c958c1
Print "Uploading code coverage" in a new line
wzieba Apr 29, 2025
ca437a0
Open `Testing` by default
wzieba Apr 29, 2025
6ff44ee
echo report tests status group in a new line
wzieba Apr 29, 2025
a5cc82b
Actually print some section headers in a new line
wzieba Apr 29, 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
1e156ea
Merge pull request #13986 from woocommerce/fix_tests_collection_on_ja…
wzieba Apr 30, 2025
886bff1
Merge branch 'feature/product_variation_to_room' into migrate_product…
wzieba Apr 30, 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
18 changes: 6 additions & 12 deletions .buildkite/commands/run-unit-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,18 @@ install_gems
echo "--- :closed_lock_with_key: Installing Secrets"
bundle exec fastlane run configure_apply

echo "--- 🧪 Testing"
echo "+++ 🧪 Testing"
set +e
./gradlew testJalapenoDebugUnitTest testDebugUnitTest
./gradlew testJalapenoDebugUnitTest testDebugUnitTest jacocoTestReport
TESTS_EXIT_STATUS=$?
set -e

if [[ "$TESTS_EXIT_STATUS" -ne 0 ]]; then
# Keep the (otherwise collapsed) current "Testing" section open in Buildkite logs on error. See https://buildkite.com/docs/pipelines/managing-log-output#collapsing-output
echo "^^^ +++"
echo "Unit Tests failed!"
if [[ "$TESTS_EXIT_STATUS" -eq 0 ]]; then
echo -e "\n--- ⚒️ Uploading code coverage"
.buildkite/commands/upload-code-coverage.sh
fi


echo "--- 🚦 Report Tests Status"
echo -e "\n--- 🚦 Report Tests Status"
results_file="WooCommerce/build/test-results/merged-test-results.xml"
# Merge JUnit results into a single file (for performance reasons with reporting)
# See https://github.com/woocommerce/woocommerce-android/pull/12064
Expand All @@ -33,10 +31,6 @@ else
annotate_test_failures "$results_file"
fi

echo "--- ⚒️ Generating and uploading code coverage"
./gradlew jacocoTestReport
.buildkite/commands/upload-code-coverage.sh

echo "--- 🧪 Copying test logs for test collector"
mkdir WooCommerce/build/buildkite-test-analytics && cp "$results_file" WooCommerce/build/buildkite-test-analytics

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,49 +134,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()
}
Comment on lines +153 to +163
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this replaces addVariant method. I like it doesn't need gson object (we shouldn't use gson inside models, nor create instance of gson per model), but we also loose synchronization of fields with ProductVariantOption. I think it's acceptable, because I think it's unlikely this model will change.

https://github.com/woocommerce/woocommerce-android/pull/13972/files#diff-a2f8ea1eb59520371a927db507f69e8a3da9de51a52c6cb1665a1bb2b56cbe17L108-L115


return (cachedVariation ?: WCProductVariationModel()).copy(
remoteProductId = remoteProductId,
remoteVariationId = 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
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
2 changes: 2 additions & 0 deletions config/gradle/jacoco.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ rootProject.afterEvaluate {
':libs:fluxc-plugin:testDebugUnitTest',
':libs:fluxc-tests:testDebugUnitTest',
':libs:login:testDebugUnitTest',
':libs:apifaker:testDebugUnitTest',
':libs:commons:testDebugUnitTest',
)

group = "Reporting"
Expand Down
Loading