From 5ac86713d7c06848b7ae2d745ff2ae656396e555 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Wed, 3 Sep 2025 23:25:47 +0200 Subject: [PATCH 01/19] Create POS-specific variation model --- .../ui/woopos/common/data/WooPosVariation.kt | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt new file mode 100644 index 000000000000..4dde216a2664 --- /dev/null +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt @@ -0,0 +1,137 @@ +package com.woocommerce.android.ui.woopos.common.data + +import com.google.gson.JsonSyntaxException +import com.google.gson.reflect.TypeToken +import com.woocommerce.android.R +import com.woocommerce.android.model.Product +import com.woocommerce.android.model.ProductVariation +import com.woocommerce.android.viewmodel.ResourceProvider +import org.wordpress.android.fluxc.persistence.entity.pos.WCPosVariationModel +import java.math.BigDecimal + +/** + * POS-specific product variation model containing only the fields needed for POS functionality. + * This model provides better performance and cleaner separation compared to the general ProductVariation model. + */ +data class WooPosVariation( + val remoteVariationId: Long, + val remoteProductId: Long, + val globalUniqueId: String, + val price: BigDecimal?, + val image: WooPosVariationImage?, + val attributes: List, + val isVisible: Boolean, + val isDownloadable: Boolean +) { + + data class WooPosVariationImage( + val source: String + ) + + data class WooPosVariationAttribute( + val id: Long?, + val name: String?, + val option: String? + ) +} + + +fun ProductVariation.toWooPosVariation(): WooPosVariation { + return WooPosVariation( + remoteVariationId = remoteVariationId, + remoteProductId = remoteProductId, + globalUniqueId = globalUniqueId, + price = price, + image = image?.let { WooPosVariation.WooPosVariationImage(it.source) }, + attributes = attributes.map { + WooPosVariation.WooPosVariationAttribute( + id = it.id, + name = it.name, + option = it.option + ) + }, + isVisible = isVisible, + isDownloadable = isDownloadable + ) +} + +@Suppress("SwallowedException") +fun WCPosVariationModel.toWooPosVariation(): WooPosVariation { + val attributesList = try { + if (attributesJson.isNotEmpty()) { + parseAttributesJson(attributesJson) + } else { + emptyList() + } + } catch (e: JsonSyntaxException) { + emptyList() + } + + return WooPosVariation( + remoteVariationId = remoteVariationId.value, + remoteProductId = remoteProductId.value, + globalUniqueId = globalUniqueId, + price = price.toBigDecimalOrNull(), + image = if (imageUrl.isNotEmpty()) WooPosVariation.WooPosVariationImage(imageUrl) else null, + attributes = attributesList, + isVisible = status == "publish", // Convert status to visibility + isDownloadable = downloadable + ) +} + +@Suppress("SwallowedException") +private fun parseAttributesJson(attributesJson: String): List { + return try { + val gson = com.google.gson.Gson() + val type = object : TypeToken>() {}.type + val items: List = gson.fromJson(attributesJson, type) + items.map { item -> + WooPosVariation.WooPosVariationAttribute( + id = item.id, + name = item.name, + option = item.option + ) + } + } catch (e: JsonSyntaxException) { + emptyList() + } +} + +private data class AttributeJsonItem( + val id: Long?, + val name: String?, + val option: String? +) + +fun WooPosVariation.getNameForPOS( + parentProduct: Product? = null, + resourceProvider: ResourceProvider, +): String { + return parentProduct?.variationEnabledAttributes?.joinToString(", ") { attribute -> + val option = attributes.firstOrNull { it.name == attribute.name } + if (option?.option != null) { + "${attribute.name}: ${option.option}" + } else { + resourceProvider.getString( + R.string.woopos_variations_any_variation, + attribute.name + ) + } + } ?: attributes.joinToString(", ") { attribute -> + if (attribute.option != null) { + "${attribute.name}: ${attribute.option}" + } else { + resourceProvider.getString( + R.string.woopos_variations_any_variation, + attribute.name!! + ) + } + } +} + +fun WooPosVariation.getName(parentProduct: Product? = null): String { + return parentProduct?.variationEnabledAttributes?.joinToString(" - ") { attribute -> + val option = attributes.firstOrNull { it.name == attribute.name } + option?.option ?: "Any ${attribute.name}" + } ?: attributes.filter { it.option != null }.joinToString(" - ") { o -> o.option!! } +} From 24f635020a194e288c5b6f4240868b46c312339f Mon Sep 17 00:00:00 2001 From: samiuelson Date: Wed, 3 Sep 2025 23:26:20 +0200 Subject: [PATCH 02/19] Migrate POS to WooPosVariation --- .../common/data/WooPosGetVariationById.kt | 19 +++++++---- .../WooPosSearchByIdentifier.kt | 4 +-- .../WooPosSearchByIdentifierResult.kt | 4 +-- .../WooPosSearchByIdentifierVariationFetch.kt | 7 ++-- .../woopos/home/cart/WooPosCartViewModel.kt | 6 ++-- .../variations/WooPosVariationsDataSource.kt | 20 ++++++++---- .../variations/WooPosVariationsLRUCache.kt | 12 +++---- .../variations/WooPosVariationsViewModel.kt | 32 +++---------------- .../home/totals/WooPosTotalsRepository.kt | 3 +- 9 files changed, 49 insertions(+), 58 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt index f87745e11823..f200eeb0c3bc 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt @@ -1,18 +1,25 @@ package com.woocommerce.android.ui.woopos.common.data -import com.woocommerce.android.model.ProductVariation -import com.woocommerce.android.model.toAppModel import com.woocommerce.android.tools.SelectedSite import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.withContext -import org.wordpress.android.fluxc.store.WCProductStore +import org.wordpress.android.fluxc.model.LocalOrRemoteId.LocalId +import org.wordpress.android.fluxc.model.LocalOrRemoteId.RemoteId +import org.wordpress.android.fluxc.store.pos.localcatalog.WooPosLocalCatalogStore import javax.inject.Inject class WooPosGetVariationById @Inject constructor( - private val store: WCProductStore, + private val posStore: WooPosLocalCatalogStore, private val site: SelectedSite, ) { - suspend operator fun invoke(productId: Long, variationId: Long): ProductVariation? = withContext(IO) { - store.getVariationByRemoteId(site.getOrNull()!!, productId, variationId)?.toAppModel() + suspend operator fun invoke(productId: Long, variationId: Long): WooPosVariation? = withContext(IO) { + val siteModel = site.getOrNull() ?: return@withContext null + val result = posStore.getVariation( + siteId = LocalId(siteModel.id), + productId = RemoteId(productId), + variationId = RemoteId(variationId) + ) + + result.getOrNull()?.toWooPosVariation() } } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifier.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifier.kt index 215c35242b9b..e97c6a87b996 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifier.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifier.kt @@ -1,8 +1,8 @@ package com.woocommerce.android.ui.woopos.common.data.searchbyidentifier import com.woocommerce.android.model.Product -import com.woocommerce.android.model.ProductVariation import com.woocommerce.android.ui.woopos.common.data.WooPosProductsTypesFilterConfig +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation import com.woocommerce.android.ui.woopos.common.data.WooPosVariationsTypesFilterConfig import com.woocommerce.android.ui.woopos.common.util.WooPosLogWrapper import org.wordpress.android.fluxc.store.WCProductStore @@ -75,7 +75,7 @@ class WooPosSearchByIdentifier @Inject constructor( } } - private fun meetsVariationFilterRequirements(variation: ProductVariation): Boolean { + private fun meetsVariationFilterRequirements(variation: WooPosVariation): Boolean { val requiredStatus = variationFilterConfig.filters[VariationFilterOption.STATUS] val hasValidStatus = when (requiredStatus) { WooPosVariationsTypesFilterConfig.VARIATION_STATUS_PUBLISH -> variation.isVisible diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResult.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResult.kt index 7d93cdf49b85..3246cecaeee0 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResult.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResult.kt @@ -1,11 +1,11 @@ package com.woocommerce.android.ui.woopos.common.data.searchbyidentifier import com.woocommerce.android.model.Product -import com.woocommerce.android.model.ProductVariation +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation sealed class WooPosSearchByIdentifierResult { data class Success(val product: Product) : WooPosSearchByIdentifierResult() - data class VariationSuccess(val variation: ProductVariation, val parentProduct: Product) : + data class VariationSuccess(val variation: WooPosVariation, val parentProduct: Product) : WooPosSearchByIdentifierResult() data class Failure(val error: Error) : WooPosSearchByIdentifierResult() diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationFetch.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationFetch.kt index e8361c2d7740..889a5b65f90b 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationFetch.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationFetch.kt @@ -1,8 +1,9 @@ package com.woocommerce.android.ui.woopos.common.data.searchbyidentifier -import com.woocommerce.android.model.ProductVariation import com.woocommerce.android.model.toAppModel import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation +import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsLRUCache import org.wordpress.android.fluxc.store.WCProductStore import javax.inject.Inject @@ -14,7 +15,7 @@ class WooPosSearchByIdentifierVariationFetch @Inject constructor( private val errorMapper: WooPosSearchByIdentifierProductErrorMapper ) { sealed class VariationFetchResult { - data class Success(val variation: ProductVariation) : VariationFetchResult() + data class Success(val variation: WooPosVariation) : VariationFetchResult() data class Failure(val error: WooPosSearchByIdentifierResult.Error) : VariationFetchResult() } @@ -34,7 +35,7 @@ class WooPosSearchByIdentifierVariationFetch @Inject constructor( selectedSite.get(), parentId, variationId - )?.toAppModel() + )?.toAppModel()?.toWooPosVariation() return if (variation != null) { variationsCache.add(parentId, variation) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModel.kt index e2553a454f00..1721187ce553 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModel.kt @@ -8,11 +8,12 @@ import androidx.lifecycle.map import androidx.lifecycle.viewModelScope import com.woocommerce.android.R import com.woocommerce.android.model.Product -import com.woocommerce.android.model.ProductVariation import com.woocommerce.android.ui.woopos.common.composeui.modifier.BarcodeInputDetector import com.woocommerce.android.ui.woopos.common.data.WooPosGetCouponById import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById import com.woocommerce.android.ui.woopos.common.data.WooPosGetVariationById +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation +import com.woocommerce.android.ui.woopos.common.data.getNameForPOS import com.woocommerce.android.ui.woopos.common.data.searchbyidentifier.WooPosSearchByIdentifier import com.woocommerce.android.ui.woopos.common.data.searchbyidentifier.WooPosSearchByIdentifierResult import com.woocommerce.android.ui.woopos.common.util.WooPosLogWrapper @@ -28,7 +29,6 @@ import com.woocommerce.android.ui.woopos.home.cart.WooPosCartStatus.CHECKOUT import com.woocommerce.android.ui.woopos.home.cart.WooPosCartStatus.EDITABLE import com.woocommerce.android.ui.woopos.home.cart.WooPosCartStatus.EMPTY import com.woocommerce.android.ui.woopos.home.items.WooPosItemsViewModel -import com.woocommerce.android.ui.woopos.home.items.variations.getNameForPOS import com.woocommerce.android.ui.woopos.util.WooPosGetCachedStoreCurrency import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent.Event.BackToCartTapped @@ -589,7 +589,7 @@ class WooPosCartViewModel @Inject constructor( imageUrl = firstImageUrl, ) - private suspend fun ProductVariation.toCartListItem( + private suspend fun WooPosVariation.toCartListItem( itemNumber: Int, product: Product ): WooPosCartItemViewState.Product.Variation = diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt index 3a42eec27204..8faf1e06b009 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt @@ -2,7 +2,9 @@ package com.woocommerce.android.ui.woopos.home.items.variations import com.woocommerce.android.model.ProductVariation import com.woocommerce.android.ui.products.variations.selector.VariationListHandler +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation import com.woocommerce.android.ui.woopos.common.data.WooPosVariationsTypesFilterConfig +import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.util.WooLog import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow @@ -20,11 +22,11 @@ class WooPosVariationsDataSource @Inject constructor( private val variationCache: WooPosVariationsLRUCache, private val variationFilterConfig: WooPosVariationsTypesFilterConfig ) { - private suspend fun getCachedVariations(productId: Long): List { + private suspend fun getCachedVariations(productId: Long): List { return variationCache.get(productId) ?: emptyList() } - private suspend fun updateCache(productId: Long, variations: List) { + private suspend fun updateCache(productId: Long, variations: List) { variationCache.put(productId, variations) } @@ -55,7 +57,9 @@ class WooPosVariationsDataSource @Inject constructor( filterOptions = variationFilterConfig.filters ) if (result.isSuccess) { - val remoteVariations = handler.getVariationsFlow(productId).firstOrNull()?.applyFilter() ?: emptyList() + val remoteVariations = handler.getVariationsFlow(productId).firstOrNull()?.applyFilter()?.map { + it.toWooPosVariation() + } ?: emptyList() updateCache(productId, remoteVariations) emit(FetchResult.Remote(Result.success(remoteVariations))) } else { @@ -69,13 +73,15 @@ class WooPosVariationsDataSource @Inject constructor( } }.flowOn(Dispatchers.IO) - suspend fun loadMore(productId: Long): Result> = withContext(Dispatchers.IO) { + suspend fun loadMore(productId: Long): Result> = withContext(Dispatchers.IO) { val result = handler.loadMore( productId, filterOptions = variationFilterConfig.filters ) if (result.isSuccess) { - val fetchedVariations = handler.getVariationsFlow(productId).first().applyFilter() + val fetchedVariations = handler.getVariationsFlow( + productId + ).first().applyFilter().map { it.toWooPosVariation() } Result.success(fetchedVariations) } else { result.logFailure() @@ -93,8 +99,8 @@ private fun Result.logFailure() { } sealed class FetchResult { - data class Cached(val data: List) : FetchResult() - data class Remote(val result: Result>) : FetchResult() + data class Cached(val data: List) : FetchResult() + data class Remote(val result: Result>) : FetchResult() } private fun List.applyFilter(): List { diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsLRUCache.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsLRUCache.kt index bbf261208872..355b3a466ce1 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsLRUCache.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsLRUCache.kt @@ -1,7 +1,7 @@ package com.woocommerce.android.ui.woopos.home.items.variations import android.util.LruCache -import com.woocommerce.android.model.ProductVariation +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import javax.inject.Inject @@ -13,22 +13,22 @@ class WooPosVariationsLRUCache @Inject constructor() { private const val VARIATION_CACHE_MAX_SIZE = 50 } - private val cache = LruCache>(VARIATION_CACHE_MAX_SIZE) + private val cache = LruCache>(VARIATION_CACHE_MAX_SIZE) private val mutex = Mutex() - suspend fun get(key: Long): List? { + suspend fun get(key: Long): List? { return mutex.withLock { cache.get(key) } } - suspend fun put(key: Long, value: List) { + suspend fun put(key: Long, value: List) { mutex.withLock { cache.put(key, value) } } - suspend fun add(key: Long, value: ProductVariation) { + suspend fun add(key: Long, value: WooPosVariation) { mutex.withLock { val list = cache.get(key) if (list != null) { @@ -48,7 +48,7 @@ class WooPosVariationsLRUCache @Inject constructor() { } } - suspend fun getAll(): List { + suspend fun getAll(): List { return mutex.withLock { cache.snapshot().values.flatten() } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt index fdeeacaf0db7..158e603919af 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt @@ -3,10 +3,10 @@ package com.woocommerce.android.ui.woopos.home.items.variations import androidx.annotation.VisibleForTesting import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import com.woocommerce.android.R import com.woocommerce.android.model.Product -import com.woocommerce.android.model.ProductVariation import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation +import com.woocommerce.android.ui.woopos.common.data.getNameForPOS import com.woocommerce.android.ui.woopos.home.ChildToParentEvent import com.woocommerce.android.ui.woopos.home.WooPosChildrenToParentEventSender import com.woocommerce.android.ui.woopos.home.items.WooPosItemSelectionViewState @@ -124,7 +124,7 @@ class WooPosVariationsViewModel @Inject constructor( } } - private suspend fun updateViewStateWithVariations(variations: List, productId: Long) { + private suspend fun updateViewStateWithVariations(variations: List, productId: Long) { if (variations.isEmpty()) { _viewState.value = WooPosVariationsViewState.Empty() } else { @@ -255,28 +255,4 @@ class WooPosVariationsViewModel @Inject constructor( } } -fun ProductVariation.getNameForPOS( - parentProduct: Product? = null, - resourceProvider: ResourceProvider, -): String { - return parentProduct?.variationEnabledAttributes?.joinToString(", ") { attribute -> - val option = attributes.firstOrNull { it.name == attribute.name } - if (option?.option != null) { - "${attribute.name}: ${option.option}" - } else { - resourceProvider.getString( - R.string.woopos_variations_any_variation, - attribute.name - ) - } - } ?: attributes.joinToString(", ") { attribute -> - if (attribute.option != null) { - "${attribute.name}: ${attribute.option}" - } else { - resourceProvider.getString( - R.string.woopos_variations_any_variation, - attribute.name!! - ) - } - } -} +// Extension function moved to WooPosVariation.kt for consistency diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsRepository.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsRepository.kt index 10a3b0da0814..eec6cb28e0a5 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsRepository.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsRepository.kt @@ -7,8 +7,9 @@ import com.woocommerce.android.ui.orders.creation.OrderCreateEditRepository import com.woocommerce.android.ui.orders.creation.OrderCreationSource import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById import com.woocommerce.android.ui.woopos.common.data.WooPosGetVariationById +import com.woocommerce.android.ui.woopos.common.data.getName +import com.woocommerce.android.ui.woopos.common.data.getNameForPOS import com.woocommerce.android.ui.woopos.home.items.WooPosItemsViewModel -import com.woocommerce.android.ui.woopos.home.items.variations.getNameForPOS import com.woocommerce.android.util.DateUtils import com.woocommerce.android.viewmodel.ResourceProvider import kotlinx.coroutines.Deferred From f10a0796b025b8e5700d3ec73187ee8e6636384a Mon Sep 17 00:00:00 2001 From: samiuelson Date: Wed, 3 Sep 2025 23:32:13 +0200 Subject: [PATCH 03/19] Satisfy detekt's complaints --- .../woocommerce/android/ui/woopos/common/data/WooPosVariation.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt index 4dde216a2664..c24cbf190a07 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt @@ -35,7 +35,6 @@ data class WooPosVariation( ) } - fun ProductVariation.toWooPosVariation(): WooPosVariation { return WooPosVariation( remoteVariationId = remoteVariationId, From 62474bf1236b7083fdfc51c0e4486ebbea68a0ab Mon Sep 17 00:00:00 2001 From: samiuelson Date: Thu, 4 Sep 2025 11:15:49 +0200 Subject: [PATCH 04/19] Remove comment --- .../woopos/home/items/variations/WooPosVariationsViewModel.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt index 158e603919af..0560121ea9b3 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt @@ -254,5 +254,3 @@ class WooPosVariationsViewModel @Inject constructor( loadMore(productId, numOfVariations) } } - -// Extension function moved to WooPosVariation.kt for consistency From f5af9f9ebaf3dd80b99f437cf03390d74f7d5c2e Mon Sep 17 00:00:00 2001 From: samiuelson Date: Thu, 4 Sep 2025 20:44:14 +0200 Subject: [PATCH 05/19] Handle null attributes --- .../ui/woopos/common/data/WooPosVariation.kt | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt index c24cbf190a07..c45ab1956088 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt @@ -117,20 +117,21 @@ fun WooPosVariation.getNameForPOS( ) } } ?: attributes.joinToString(", ") { attribute -> - if (attribute.option != null) { - "${attribute.name}: ${attribute.option}" - } else { - resourceProvider.getString( + when { + attribute.option != null && attribute.name != null -> "${attribute.name}: ${attribute.option}" + attribute.option != null -> attribute.option + attribute.name != null -> resourceProvider.getString( R.string.woopos_variations_any_variation, - attribute.name!! + attribute.name ) + else -> "" } - } + }.ifEmpty { "" } } fun WooPosVariation.getName(parentProduct: Product? = null): String { return parentProduct?.variationEnabledAttributes?.joinToString(" - ") { attribute -> val option = attributes.firstOrNull { it.name == attribute.name } option?.option ?: "Any ${attribute.name}" - } ?: attributes.filter { it.option != null }.joinToString(" - ") { o -> o.option!! } + } ?: attributes.mapNotNull { it.option }.joinToString(" - ").ifEmpty { "" } } From bf1f27a9b63722ee878020ee05a28ed30b3bf723 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Thu, 4 Sep 2025 20:50:52 +0200 Subject: [PATCH 06/19] Update tests --- .../WooPosSearchByIdentifierLocalTest.kt | 88 ++++++++++++------- 1 file changed, 54 insertions(+), 34 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierLocalTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierLocalTest.kt index 8cdd5eef5950..c851e18b5593 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierLocalTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierLocalTest.kt @@ -1,8 +1,8 @@ package com.woocommerce.android.ui.woopos.common.data.searchbyidentifier import com.woocommerce.android.model.Product -import com.woocommerce.android.model.ProductVariation import com.woocommerce.android.model.toAppModel +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation import com.woocommerce.android.ui.products.ProductBackorderStatus import com.woocommerce.android.ui.products.ProductStatus import com.woocommerce.android.ui.products.ProductStockStatus @@ -42,7 +42,7 @@ class WooPosSearchByIdentifierLocalTest { val identifier = "1234567890123" val product = createProduct(globalUniqueId = identifier) whenever(productsCache.getAll()).thenReturn(listOf(product)) - whenever(variationsCache.getAll()).thenReturn(emptyList()) + whenever(variationsCache.getAll()).thenReturn(emptyList()) // WHEN val result = sut(identifier) @@ -56,7 +56,7 @@ class WooPosSearchByIdentifierLocalTest { // GIVEN val identifier = "NOTFOUND" whenever(productsCache.getAll()).thenReturn(emptyList()) - whenever(variationsCache.getAll()).thenReturn(emptyList()) + whenever(variationsCache.getAll()).thenReturn(emptyList()) // WHEN val result = sut(identifier) @@ -71,7 +71,7 @@ class WooPosSearchByIdentifierLocalTest { val identifier = "ABC123" val product = createProduct(globalUniqueId = "abc123") whenever(productsCache.getAll()).thenReturn(listOf(product)) - whenever(variationsCache.getAll()).thenReturn(emptyList()) + whenever(variationsCache.getAll()).thenReturn(emptyList()) // WHEN val result = sut(identifier) @@ -87,12 +87,16 @@ class WooPosSearchByIdentifierLocalTest { val productId = 1L val variationId = 10L val product = createProduct(remoteId = productId).copy(type = ProductType.VARIABLE.value) - val variation: ProductVariation = mock { - on { remoteVariationId }.thenReturn(variationId) - on { remoteProductId }.thenReturn(productId) - on { globalUniqueId }.thenReturn(identifier) - on { sku }.thenReturn("") - } + val variation = WooPosVariation( + remoteVariationId = variationId, + remoteProductId = productId, + globalUniqueId = identifier, + price = BigDecimal.TEN, + image = null, + attributes = emptyList(), + isVisible = true, + isDownloadable = false + ) whenever(productsCache.getAll()).thenReturn(listOf(product)) whenever(productsCache.getProductById(productId)).thenReturn(product) whenever(variationsCache.getAll()).thenReturn(listOf(variation)) @@ -112,18 +116,26 @@ class WooPosSearchByIdentifierLocalTest { val identifier = "MATCH-VAR" val productId = 1L val product = createProduct(remoteId = productId).copy(type = ProductType.VARIABLE.value) - val variation1: ProductVariation = mock { - on { remoteVariationId }.thenReturn(10L) - on { remoteProductId }.thenReturn(productId) - on { globalUniqueId }.thenReturn("OTHER-VAR") - on { sku }.thenReturn("OTHER-SKU") - } - val variation2: ProductVariation = mock { - on { remoteVariationId }.thenReturn(20L) - on { remoteProductId }.thenReturn(productId) - on { globalUniqueId }.thenReturn(identifier) - on { sku }.thenReturn("") - } + val variation1 = WooPosVariation( + remoteVariationId = 10L, + remoteProductId = productId, + globalUniqueId = "OTHER-VAR", + price = BigDecimal.TEN, + image = null, + attributes = emptyList(), + isVisible = true, + isDownloadable = false + ) + val variation2 = WooPosVariation( + remoteVariationId = 20L, + remoteProductId = productId, + globalUniqueId = identifier, + price = BigDecimal.TEN, + image = null, + attributes = emptyList(), + isVisible = true, + isDownloadable = false + ) whenever(productsCache.getAll()).thenReturn(listOf(product)) whenever(productsCache.getProductById(productId)).thenReturn(product) whenever(variationsCache.getAll()).thenReturn(listOf(variation1, variation2)) @@ -143,12 +155,16 @@ class WooPosSearchByIdentifierLocalTest { val identifier = "VAR-UPPER" val productId = 1L val product = createProduct(remoteId = productId).copy(type = ProductType.VARIABLE.value) - val variation: ProductVariation = mock { - on { remoteVariationId }.thenReturn(10L) - on { remoteProductId }.thenReturn(productId) - on { globalUniqueId }.thenReturn("var-upper") - on { sku }.thenReturn("") - } + val variation = WooPosVariation( + remoteVariationId = 10L, + remoteProductId = productId, + globalUniqueId = "var-upper", + price = BigDecimal.TEN, + image = null, + attributes = emptyList(), + isVisible = true, + isDownloadable = false + ) whenever(productsCache.getAll()).thenReturn(listOf(product)) whenever(productsCache.getProductById(productId)).thenReturn(product) whenever(variationsCache.getAll()).thenReturn(listOf(variation)) @@ -167,12 +183,16 @@ class WooPosSearchByIdentifierLocalTest { // GIVEN val identifier = "VAR123456" val productId = 1L - val variation: ProductVariation = mock { - on { remoteVariationId }.thenReturn(10L) - on { remoteProductId }.thenReturn(productId) - on { globalUniqueId }.thenReturn(identifier) - on { sku }.thenReturn("") - } + val variation = WooPosVariation( + remoteVariationId = 10L, + remoteProductId = productId, + globalUniqueId = identifier, + price = BigDecimal.TEN, + image = null, + attributes = emptyList(), + isVisible = true, + isDownloadable = false + ) whenever(productsCache.getAll()).thenReturn(emptyList()) whenever(variationsCache.getAll()).thenReturn(listOf(variation)) From 74ad241b1762d5529f8381eec8f92deba61c7147 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Thu, 4 Sep 2025 21:16:00 +0200 Subject: [PATCH 07/19] Clean up code --- .../android/ui/woopos/common/data/WooPosVariation.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt index c45ab1956088..f0dbc29d6e5b 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt @@ -126,12 +126,12 @@ fun WooPosVariation.getNameForPOS( ) else -> "" } - }.ifEmpty { "" } + } } fun WooPosVariation.getName(parentProduct: Product? = null): String { return parentProduct?.variationEnabledAttributes?.joinToString(" - ") { attribute -> val option = attributes.firstOrNull { it.name == attribute.name } option?.option ?: "Any ${attribute.name}" - } ?: attributes.mapNotNull { it.option }.joinToString(" - ").ifEmpty { "" } + } ?: attributes.mapNotNull { it.option }.joinToString(" - ") } From 5095fb6b6da38f3c81c7b4bdf69b232b8e023f35 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Thu, 4 Sep 2025 21:24:25 +0200 Subject: [PATCH 08/19] Revert WooPosGetVariationById --- .../woopos/common/data/WooPosGetVariationById.kt | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt index f200eeb0c3bc..0fe912a7b48f 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt @@ -1,25 +1,18 @@ package com.woocommerce.android.ui.woopos.common.data +import com.woocommerce.android.model.toAppModel import com.woocommerce.android.tools.SelectedSite import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.withContext -import org.wordpress.android.fluxc.model.LocalOrRemoteId.LocalId -import org.wordpress.android.fluxc.model.LocalOrRemoteId.RemoteId -import org.wordpress.android.fluxc.store.pos.localcatalog.WooPosLocalCatalogStore +import org.wordpress.android.fluxc.store.WCProductStore import javax.inject.Inject class WooPosGetVariationById @Inject constructor( - private val posStore: WooPosLocalCatalogStore, + private val store: WCProductStore, private val site: SelectedSite, ) { suspend operator fun invoke(productId: Long, variationId: Long): WooPosVariation? = withContext(IO) { val siteModel = site.getOrNull() ?: return@withContext null - val result = posStore.getVariation( - siteId = LocalId(siteModel.id), - productId = RemoteId(productId), - variationId = RemoteId(variationId) - ) - - result.getOrNull()?.toWooPosVariation() + store.getVariationByRemoteId(siteModel, productId, variationId)?.toAppModel()?.toWooPosVariation() } } From abc822188e28a64eaea9172f46dc65c057121c51 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Thu, 4 Sep 2025 21:34:54 +0200 Subject: [PATCH 09/19] =?UTF-8?q?Map=20WCProductVariationModel=20=E2=86=92?= =?UTF-8?q?=20WooPosVariation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../common/data/WooPosGetVariationById.kt | 3 +- .../ui/woopos/common/data/WooPosVariation.kt | 35 ++++++++++++++++++- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt index 0fe912a7b48f..955c14a4aac6 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt @@ -1,6 +1,5 @@ package com.woocommerce.android.ui.woopos.common.data -import com.woocommerce.android.model.toAppModel import com.woocommerce.android.tools.SelectedSite import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.withContext @@ -13,6 +12,6 @@ class WooPosGetVariationById @Inject constructor( ) { suspend operator fun invoke(productId: Long, variationId: Long): WooPosVariation? = withContext(IO) { val siteModel = site.getOrNull() ?: return@withContext null - store.getVariationByRemoteId(siteModel, productId, variationId)?.toAppModel()?.toWooPosVariation() + store.getVariationByRemoteId(siteModel, productId, variationId)?.toWooPosVariation() } } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt index f0dbc29d6e5b..9a4d24183715 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt @@ -6,6 +6,7 @@ import com.woocommerce.android.R import com.woocommerce.android.model.Product import com.woocommerce.android.model.ProductVariation import com.woocommerce.android.viewmodel.ResourceProvider +import org.wordpress.android.fluxc.model.WCProductVariationModel import org.wordpress.android.fluxc.persistence.entity.pos.WCPosVariationModel import java.math.BigDecimal @@ -54,6 +55,38 @@ fun ProductVariation.toWooPosVariation(): WooPosVariation { ) } +@Suppress("SwallowedException") +fun WCProductVariationModel.toWooPosVariation(): WooPosVariation { + val attributesList = try { + attributeList?.map { attribute -> + WooPosVariation.WooPosVariationAttribute( + id = attribute.id, + name = attribute.name, + option = attribute.option + ) + } ?: emptyList() + } catch (e: Exception) { + emptyList() + } + + val imageModel = try { + if (image.isNotEmpty()) getImageModel() else null + } catch (e: JsonSyntaxException) { + null + } + + return WooPosVariation( + remoteVariationId = remoteVariationId.value, + remoteProductId = remoteProductId.value, + globalUniqueId = globalUniqueId, + price = price.toBigDecimalOrNull(), + image = imageModel?.src?.let { WooPosVariation.WooPosVariationImage(it) }, + attributes = attributesList, + isVisible = status == "publish", + isDownloadable = downloadable + ) +} + @Suppress("SwallowedException") fun WCPosVariationModel.toWooPosVariation(): WooPosVariation { val attributesList = try { @@ -73,7 +106,7 @@ fun WCPosVariationModel.toWooPosVariation(): WooPosVariation { price = price.toBigDecimalOrNull(), image = if (imageUrl.isNotEmpty()) WooPosVariation.WooPosVariationImage(imageUrl) else null, attributes = attributesList, - isVisible = status == "publish", // Convert status to visibility + isVisible = status == "publish", isDownloadable = downloadable ) } From 64a0510c8635516f63c536a0ac8b7b0324385573 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 5 Sep 2025 07:16:20 +0200 Subject: [PATCH 10/19] Remove unnecessary try-catch --- .../ui/woopos/common/data/WooPosVariation.kt | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt index 9a4d24183715..f0111324fdf9 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt @@ -57,17 +57,13 @@ fun ProductVariation.toWooPosVariation(): WooPosVariation { @Suppress("SwallowedException") fun WCProductVariationModel.toWooPosVariation(): WooPosVariation { - val attributesList = try { - attributeList?.map { attribute -> - WooPosVariation.WooPosVariationAttribute( - id = attribute.id, - name = attribute.name, - option = attribute.option - ) - } ?: emptyList() - } catch (e: Exception) { - emptyList() - } + val attributesList = attributeList?.map { attribute -> + WooPosVariation.WooPosVariationAttribute( + id = attribute.id, + name = attribute.name, + option = attribute.option + ) + } ?: emptyList() val imageModel = try { if (image.isNotEmpty()) getImageModel() else null From b4997b7e301cb833e13c8951ef514e22256c4f9a Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 5 Sep 2025 07:16:28 +0200 Subject: [PATCH 11/19] Satisfy detekt's complaints --- .../searchbyidentifier/WooPosSearchByIdentifierLocalTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierLocalTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierLocalTest.kt index c851e18b5593..13af12bbe859 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierLocalTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierLocalTest.kt @@ -2,7 +2,6 @@ package com.woocommerce.android.ui.woopos.common.data.searchbyidentifier import com.woocommerce.android.model.Product import com.woocommerce.android.model.toAppModel -import com.woocommerce.android.ui.woopos.common.data.WooPosVariation import com.woocommerce.android.ui.products.ProductBackorderStatus import com.woocommerce.android.ui.products.ProductStatus import com.woocommerce.android.ui.products.ProductStockStatus @@ -11,6 +10,7 @@ import com.woocommerce.android.ui.products.ProductTestUtils import com.woocommerce.android.ui.products.ProductType import com.woocommerce.android.ui.products.settings.ProductCatalogVisibility import com.woocommerce.android.ui.woopos.common.data.WooPosProductsCache +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsLRUCache import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals From 2c8b8fbcad0b9b37429c2530e5492163f9a76136 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 5 Sep 2025 07:21:08 +0200 Subject: [PATCH 12/19] Make Gson instance a singleton --- .../android/ui/woopos/common/data/WooPosVariation.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt index f0111324fdf9..fc32febf19ca 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt @@ -1,5 +1,6 @@ package com.woocommerce.android.ui.woopos.common.data +import com.google.gson.Gson import com.google.gson.JsonSyntaxException import com.google.gson.reflect.TypeToken import com.woocommerce.android.R @@ -107,10 +108,10 @@ fun WCPosVariationModel.toWooPosVariation(): WooPosVariation { ) } +private val gson by lazy { Gson() } @Suppress("SwallowedException") private fun parseAttributesJson(attributesJson: String): List { return try { - val gson = com.google.gson.Gson() val type = object : TypeToken>() {}.type val items: List = gson.fromJson(attributesJson, type) items.map { item -> From e2dc6d0a59dbba2ddbc6437f1d3e6cd614eb401a Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 5 Sep 2025 07:27:28 +0200 Subject: [PATCH 13/19] Satisfy detekt's complaints --- .../woocommerce/android/ui/woopos/common/data/WooPosVariation.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt index fc32febf19ca..017b504882f6 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt @@ -109,6 +109,7 @@ fun WCPosVariationModel.toWooPosVariation(): WooPosVariation { } private val gson by lazy { Gson() } + @Suppress("SwallowedException") private fun parseAttributesJson(attributesJson: String): List { return try { From ed12b161e8641a480340850bd96e1ba767d19f28 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 5 Sep 2025 08:28:39 +0200 Subject: [PATCH 14/19] Fix tests --- .../WooPosVariationsDataSourceTest.kt | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsDataSourceTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsDataSourceTest.kt index f0f4985e9d5b..281b94b10b15 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsDataSourceTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsDataSourceTest.kt @@ -2,7 +2,9 @@ package com.woocommerce.android.ui.woopos.home.variations import com.woocommerce.android.ui.products.ProductTestUtils import com.woocommerce.android.ui.products.variations.selector.VariationListHandler +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation import com.woocommerce.android.ui.woopos.common.data.WooPosVariationsTypesFilterConfig +import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.home.items.variations.FetchResult import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsDataSource import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsLRUCache @@ -26,7 +28,7 @@ class WooPosVariationsDataSourceTest { @JvmField val coroutinesTestRule = WooPosCoroutineTestRule() - private val sampleProducts = listOf( + private val sampleProductVariations = listOf( ProductTestUtils.generateProductVariation( productId = 1, variationId = 2, @@ -46,8 +48,10 @@ class WooPosVariationsDataSourceTest { isDownloadable = false, ) ) + + private val sampleProducts = sampleProductVariations.map { it.toWooPosVariation() } - private val additionalProducts = listOf( + private val additionalProductVariations = listOf( ProductTestUtils.generateProductVariation( productId = 4, variationId = 5, @@ -61,6 +65,8 @@ class WooPosVariationsDataSourceTest { isDownloadable = false, ), ) + + private val additionalProducts = additionalProductVariations.map { it.toWooPosVariation() } private val handler: VariationListHandler = mock() private val variationsCache: WooPosVariationsLRUCache = mock() @@ -70,7 +76,7 @@ class WooPosVariationsDataSourceTest { // GIVEN val productId = 1L whenever(handler.canLoadMore(5)).thenReturn(true) - whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProducts)) + whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProductVariations)) whenever(variationsCache.get(productId)).thenReturn(sampleProducts) val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) @@ -95,7 +101,7 @@ class WooPosVariationsDataSourceTest { // GIVEN val productId = 1L whenever(handler.canLoadMore(5)).thenReturn(true) - whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProducts)) + whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProductVariations)) whenever(variationsCache.get(productId)).thenReturn(sampleProducts) val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) @@ -116,7 +122,7 @@ class WooPosVariationsDataSourceTest { // GIVEN val productId = 1L whenever(handler.canLoadMore(5)).thenReturn(true) - whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProducts)) + whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProductVariations)) whenever(variationsCache.get(productId)).thenReturn(sampleProducts) val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) @@ -140,7 +146,7 @@ class WooPosVariationsDataSourceTest { // GIVEN val productId = 1L whenever(handler.canLoadMore(5)).thenReturn(true) - whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProducts)) + whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProductVariations)) whenever(variationsCache.get(productId)).thenReturn(sampleProducts) val exception = Exception("Remote load failed") val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) @@ -177,8 +183,8 @@ class WooPosVariationsDataSourceTest { val productId = 1L whenever(handler.canLoadMore(5)).thenReturn(true) whenever(handler.getVariationsFlow(productId)).thenReturn( - flowOf(sampleProducts), - flowOf(sampleProducts + additionalProducts) + flowOf(sampleProductVariations), + flowOf(sampleProductVariations + additionalProductVariations) ) whenever(variationsCache.get(productId)).thenReturn(sampleProducts + additionalProducts) whenever(handler.loadMore(productId)).thenReturn(Result.success(Unit)) @@ -204,7 +210,7 @@ class WooPosVariationsDataSourceTest { // GIVEN val productId = 1L whenever(handler.canLoadMore(5)).thenReturn(true) - whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProducts)) + whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProductVariations)) val exception = Exception("Load more failed") whenever( handler.loadMore( From d12819ba6049548392a777a28c5167540c437be2 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 5 Sep 2025 09:03:20 +0200 Subject: [PATCH 15/19] Fix tests --- .../WooPosVariationsViewModelTest.kt | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt index cec3cfcbfd5d..2eac4a69b740 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt @@ -4,6 +4,8 @@ import app.cash.turbine.test import com.woocommerce.android.R import com.woocommerce.android.ui.products.ProductTestUtils import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById +import com.woocommerce.android.ui.woopos.common.data.getNameForPOS +import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.home.ChildToParentEvent import com.woocommerce.android.ui.woopos.home.WooPosChildrenToParentEventSender import com.woocommerce.android.ui.woopos.home.items.WooPosItemsViewModel @@ -13,7 +15,6 @@ import com.woocommerce.android.ui.woopos.home.items.variations.FetchResult import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsDataSource import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsUIEvents import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsViewModel -import com.woocommerce.android.ui.woopos.home.items.variations.getNameForPOS import com.woocommerce.android.ui.woopos.util.WooPosCoroutineTestRule import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEventConstant @@ -60,8 +61,8 @@ class WooPosVariationsViewModelTest { fun `given variations from data source, when view model created, then view state updated correctly`() = runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0"), - ProductTestUtils.generateProductVariation(2, 1, "20.0") + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), + ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation() ) whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( flowOf(FetchResult.Remote(Result.success(variations))) @@ -83,7 +84,7 @@ class WooPosVariationsViewModelTest { fun `given variation has zero price, when view model created, then view state updated correctly`() = runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "0.00"), + ProductTestUtils.generateProductVariation(1, 1, "0.00").toWooPosVariation(), ) whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( flowOf(FetchResult.Remote(Result.success(variations))) @@ -105,7 +106,7 @@ class WooPosVariationsViewModelTest { fun `given variation has no price set, when view model created, then view state updated correctly`() = runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, ""), + ProductTestUtils.generateProductVariation(1, 1, "").toWooPosVariation(), ) whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( flowOf(FetchResult.Remote(Result.success(variations))) @@ -158,8 +159,8 @@ class WooPosVariationsViewModelTest { fun `given variations, when pull to refresh triggered, then fetchFirstPage is called`() = runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0"), - ProductTestUtils.generateProductVariation(2, 1, "20.0") + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), + ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation() ) whenever(variationsDataSource.fetchFirstPage(any(), eq(false))).thenReturn( flowOf(FetchResult.Remote(Result.success(emptyList()))) @@ -200,8 +201,8 @@ class WooPosVariationsViewModelTest { fun `given no more variations, when end of list reached, then pagination state is none`() = runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0"), - ProductTestUtils.generateProductVariation(2, 1, "20.0") + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), + ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation() ) whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( flowOf(FetchResult.Remote(Result.success(variations))) @@ -226,7 +227,7 @@ class WooPosVariationsViewModelTest { fun `given variations, when load more succeeds, then pagination state is updated`() = runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0"), + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), ) whenever(variationsDataSource.loadMore(any())).thenReturn(Result.success(variations)) whenever(variationsDataSource.canLoadMore(any())).thenReturn(true) @@ -236,8 +237,8 @@ class WooPosVariationsViewModelTest { FetchResult.Remote( Result.success( listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0"), - ProductTestUtils.generateProductVariation(2, 1, "20.0") + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), + ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation() ) ) ) @@ -267,8 +268,8 @@ class WooPosVariationsViewModelTest { FetchResult.Remote( Result.success( listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0"), - ProductTestUtils.generateProductVariation(2, 1, "20.0") + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), + ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation() ) ) ) @@ -293,8 +294,8 @@ class WooPosVariationsViewModelTest { runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0"), - ProductTestUtils.generateProductVariation(2, 1, "20.0") + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), + ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation() ) whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( flowOf(FetchResult.Remote(Result.success(variations))) @@ -396,7 +397,7 @@ class WooPosVariationsViewModelTest { ) // WHEN - val attributeName = variableProduct.getNameForPOS(parentProduct, resourceProvider) + val attributeName = variableProduct.toWooPosVariation().getNameForPOS(parentProduct, resourceProvider) // Then assertThat(attributeName).isEqualTo("Color: Blue") @@ -424,7 +425,7 @@ class WooPosVariationsViewModelTest { ) // WHEN - val attributeName = variableProduct.getNameForPOS(null, resourceProvider) + val attributeName = variableProduct.toWooPosVariation().getNameForPOS(null, resourceProvider) // THEN assertThat(attributeName).isEqualTo("Color: Blue, Size: M") @@ -467,7 +468,7 @@ class WooPosVariationsViewModelTest { ).thenReturn("Any Material") // WHEN - val attributeName = variableProduct.getNameForPOS(parentProduct, resourceProvider) + val attributeName = variableProduct.toWooPosVariation().getNameForPOS(parentProduct, resourceProvider) // THEN assertThat(attributeName).isEqualTo("Any Material") @@ -513,7 +514,7 @@ class WooPosVariationsViewModelTest { ) // WHEN - val attributeName = variableProduct.getNameForPOS(parentProduct, resourceProvider) + val attributeName = variableProduct.toWooPosVariation().getNameForPOS(parentProduct, resourceProvider) // THEN assertThat(attributeName).isEqualTo("Color: Blue, Size: M") @@ -565,7 +566,7 @@ class WooPosVariationsViewModelTest { ).thenReturn("Any Size") // WHEN - val attributeName = variableProduct.getNameForPOS(parentProduct, resourceProvider) + val attributeName = variableProduct.toWooPosVariation().getNameForPOS(parentProduct, resourceProvider) // THEN assertThat(attributeName).isEqualTo("Any Color, Any Size") @@ -597,7 +598,7 @@ class WooPosVariationsViewModelTest { ).thenReturn("Any Color") // WHEN - val attributeName = variableProduct.getNameForPOS(parentProduct, resourceProvider) + val attributeName = variableProduct.toWooPosVariation().getNameForPOS(parentProduct, resourceProvider) // THEN assertThat(attributeName).isEqualTo("Any Color") @@ -643,7 +644,7 @@ class WooPosVariationsViewModelTest { ) // WHEN - val attributeName = variableProduct.getNameForPOS(parentProduct, resourceProvider) + val attributeName = variableProduct.toWooPosVariation().getNameForPOS(parentProduct, resourceProvider) // THEN assertThat(attributeName).isEqualTo("Color: Blue") From a4c390381c91f6ecfd18301bca9a64aff7170fd1 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 5 Sep 2025 10:03:39 +0200 Subject: [PATCH 16/19] Fix tests --- ...hByIdentifierProcessVariationResultTest.kt | 24 ++++++++++++++++--- ...osSearchByIdentifierResultConverterTest.kt | 3 ++- ...archByIdentifierVariationGetOrFetchTest.kt | 21 ++++++++++------ .../WooPosVariationsDataSourceTest.kt | 5 ++-- 4 files changed, 39 insertions(+), 14 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierProcessVariationResultTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierProcessVariationResultTest.kt index e68482d7f569..5028f1feb8f1 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierProcessVariationResultTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierProcessVariationResultTest.kt @@ -1,7 +1,7 @@ package com.woocommerce.android.ui.woopos.common.data.searchbyidentifier import com.woocommerce.android.model.Product -import com.woocommerce.android.model.ProductVariation +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals @@ -30,7 +30,16 @@ class WooPosSearchByIdentifierProcessVariationResultTest { on { remoteId }.thenReturn(variationId) } val parentProduct: Product = mock() - val variation: ProductVariation = mock() + val variation = WooPosVariation( + remoteVariationId = variationId, + remoteProductId = parentId, + globalUniqueId = "test", + price = null, + image = null, + attributes = emptyList(), + isVisible = true, + isDownloadable = false + ) runBlocking { whenever( @@ -108,7 +117,16 @@ class WooPosSearchByIdentifierProcessVariationResultTest { on { this.parentId }.thenReturn(parentId) on { remoteId }.thenReturn(variationId) } - val variation: ProductVariation = mock() + val variation = WooPosVariation( + remoteVariationId = variationId, + remoteProductId = parentId, + globalUniqueId = "test", + price = null, + image = null, + attributes = emptyList(), + isVisible = true, + isDownloadable = false + ) val expectedFailure = WooPosSearchByIdentifierResult.Failure(WooPosSearchByIdentifierResult.Error.NetworkError) runBlocking { diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResultConverterTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResultConverterTest.kt index d0b14fc4ac46..c4116acf2720 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResultConverterTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResultConverterTest.kt @@ -2,6 +2,7 @@ package com.woocommerce.android.ui.woopos.common.data.searchbyidentifier import com.woocommerce.android.ui.products.ProductTestUtils import com.woocommerce.android.ui.woopos.common.data.WooPosProductsCache +import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.util.WooPosCoroutineTestRule import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.UnconfinedTestDispatcher @@ -26,7 +27,7 @@ class WooPosSearchByIdentifierResultConverterTest { private val variationProcess: WooPosSearchByIdentifierProcessVariationResult = mock() private val testProduct = ProductTestUtils.generateProduct() - private val testVariation = ProductTestUtils.generateProductVariation() + private val testVariation = ProductTestUtils.generateProductVariation().toWooPosVariation() @Before fun setup() { diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationGetOrFetchTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationGetOrFetchTest.kt index d81f14c9d5f1..a849e3113369 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationGetOrFetchTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationGetOrFetchTest.kt @@ -1,8 +1,8 @@ package com.woocommerce.android.ui.woopos.common.data.searchbyidentifier -import com.woocommerce.android.model.ProductVariation -import com.woocommerce.android.model.toAppModel import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation +import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsLRUCache import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals @@ -43,7 +43,7 @@ class WooPosSearchByIdentifierVariationGetOrFetchTest { remoteVariationId = RemoteId(variationId), remoteProductId = RemoteId(parentId) ) - val variation = wcVariation.toAppModel() + val variation = wcVariation.toWooPosVariation() val fetchResult: WCProductStore.OnVariationChanged = mock { on { isError }.thenReturn(false) } @@ -118,14 +118,21 @@ class WooPosSearchByIdentifierVariationGetOrFetchTest { // GIVEN val variationId = 456L val parentId = 123L - val differentVariation: ProductVariation = mock { - on { remoteVariationId }.thenReturn(999L) - } + val differentVariation = WooPosVariation( + remoteVariationId = 999L, + remoteProductId = parentId, + globalUniqueId = "different", + price = null, + image = null, + attributes = emptyList(), + isVisible = true, + isDownloadable = false + ) val wcVariation = WCProductVariationModel(LocalId(1)).copy( remoteVariationId = RemoteId(variationId), remoteProductId = RemoteId(parentId) ) - val variation = wcVariation.toAppModel() + val variation = wcVariation.toWooPosVariation() val fetchResult: WCProductStore.OnVariationChanged = mock { on { isError }.thenReturn(false) } diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsDataSourceTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsDataSourceTest.kt index 281b94b10b15..65104271b1f9 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsDataSourceTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsDataSourceTest.kt @@ -2,7 +2,6 @@ package com.woocommerce.android.ui.woopos.home.variations import com.woocommerce.android.ui.products.ProductTestUtils import com.woocommerce.android.ui.products.variations.selector.VariationListHandler -import com.woocommerce.android.ui.woopos.common.data.WooPosVariation import com.woocommerce.android.ui.woopos.common.data.WooPosVariationsTypesFilterConfig import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.home.items.variations.FetchResult @@ -48,7 +47,7 @@ class WooPosVariationsDataSourceTest { isDownloadable = false, ) ) - + private val sampleProducts = sampleProductVariations.map { it.toWooPosVariation() } private val additionalProductVariations = listOf( @@ -65,7 +64,7 @@ class WooPosVariationsDataSourceTest { isDownloadable = false, ), ) - + private val additionalProducts = additionalProductVariations.map { it.toWooPosVariation() } private val handler: VariationListHandler = mock() From cf5cecd2540757f8aced8101d0d632a0fa36fe5b Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 5 Sep 2025 11:39:48 +0200 Subject: [PATCH 17/19] Log JsonSyntaxException --- .../android/ui/woopos/common/data/WooPosVariation.kt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt index 017b504882f6..d4bebd0c57f8 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt @@ -6,6 +6,7 @@ import com.google.gson.reflect.TypeToken import com.woocommerce.android.R import com.woocommerce.android.model.Product import com.woocommerce.android.model.ProductVariation +import com.woocommerce.android.util.WooLog import com.woocommerce.android.viewmodel.ResourceProvider import org.wordpress.android.fluxc.model.WCProductVariationModel import org.wordpress.android.fluxc.persistence.entity.pos.WCPosVariationModel @@ -69,6 +70,10 @@ fun WCProductVariationModel.toWooPosVariation(): WooPosVariation { val imageModel = try { if (image.isNotEmpty()) getImageModel() else null } catch (e: JsonSyntaxException) { + WooLog.w( + WooLog.T.POS, + "Failed to parse image from JSON attributes for variation ${remoteVariationId.value}: ${e.message}" + ) null } @@ -93,6 +98,7 @@ fun WCPosVariationModel.toWooPosVariation(): WooPosVariation { emptyList() } } catch (e: JsonSyntaxException) { + WooLog.w(WooLog.T.POS, "Failed to parse attributes JSON for variation ${remoteVariationId.value}: ${e.message}") emptyList() } @@ -123,6 +129,7 @@ private fun parseAttributesJson(attributesJson: String): List Date: Fri, 5 Sep 2025 12:38:19 +0200 Subject: [PATCH 18/19] Extract JSON parsing and data mapping to the mapper class --- .../common/data/WooPosGetVariationById.kt | 3 +- .../ui/woopos/common/data/WooPosVariation.kt | 146 --------------- .../common/data/WooPosVariationMapper.kt | 175 ++++++++++++++++++ .../WooPosSearchByIdentifierVariationFetch.kt | 7 +- .../woopos/home/cart/WooPosCartViewModel.kt | 6 +- .../variations/WooPosVariationsDataSource.kt | 8 +- .../variations/WooPosVariationsViewModel.kt | 21 ++- .../home/totals/WooPosTotalsRepository.kt | 6 +- ...osSearchByIdentifierResultConverterTest.kt | 4 +- .../home/totals/WooPosTotalsRepositoryTest.kt | 3 + 10 files changed, 217 insertions(+), 162 deletions(-) create mode 100644 WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariationMapper.kt diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt index 955c14a4aac6..d99b752693da 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetVariationById.kt @@ -9,9 +9,10 @@ import javax.inject.Inject class WooPosGetVariationById @Inject constructor( private val store: WCProductStore, private val site: SelectedSite, + private val mapper: WooPosVariationMapper, ) { suspend operator fun invoke(productId: Long, variationId: Long): WooPosVariation? = withContext(IO) { val siteModel = site.getOrNull() ?: return@withContext null - store.getVariationByRemoteId(siteModel, productId, variationId)?.toWooPosVariation() + store.getVariationByRemoteId(siteModel, productId, variationId)?.toWooPosVariation(mapper) } } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt index d4bebd0c57f8..17fd06d06f4a 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt @@ -1,15 +1,5 @@ package com.woocommerce.android.ui.woopos.common.data -import com.google.gson.Gson -import com.google.gson.JsonSyntaxException -import com.google.gson.reflect.TypeToken -import com.woocommerce.android.R -import com.woocommerce.android.model.Product -import com.woocommerce.android.model.ProductVariation -import com.woocommerce.android.util.WooLog -import com.woocommerce.android.viewmodel.ResourceProvider -import org.wordpress.android.fluxc.model.WCProductVariationModel -import org.wordpress.android.fluxc.persistence.entity.pos.WCPosVariationModel import java.math.BigDecimal /** @@ -37,139 +27,3 @@ data class WooPosVariation( val option: String? ) } - -fun ProductVariation.toWooPosVariation(): WooPosVariation { - return WooPosVariation( - remoteVariationId = remoteVariationId, - remoteProductId = remoteProductId, - globalUniqueId = globalUniqueId, - price = price, - image = image?.let { WooPosVariation.WooPosVariationImage(it.source) }, - attributes = attributes.map { - WooPosVariation.WooPosVariationAttribute( - id = it.id, - name = it.name, - option = it.option - ) - }, - isVisible = isVisible, - isDownloadable = isDownloadable - ) -} - -@Suppress("SwallowedException") -fun WCProductVariationModel.toWooPosVariation(): WooPosVariation { - val attributesList = attributeList?.map { attribute -> - WooPosVariation.WooPosVariationAttribute( - id = attribute.id, - name = attribute.name, - option = attribute.option - ) - } ?: emptyList() - - val imageModel = try { - if (image.isNotEmpty()) getImageModel() else null - } catch (e: JsonSyntaxException) { - WooLog.w( - WooLog.T.POS, - "Failed to parse image from JSON attributes for variation ${remoteVariationId.value}: ${e.message}" - ) - null - } - - return WooPosVariation( - remoteVariationId = remoteVariationId.value, - remoteProductId = remoteProductId.value, - globalUniqueId = globalUniqueId, - price = price.toBigDecimalOrNull(), - image = imageModel?.src?.let { WooPosVariation.WooPosVariationImage(it) }, - attributes = attributesList, - isVisible = status == "publish", - isDownloadable = downloadable - ) -} - -@Suppress("SwallowedException") -fun WCPosVariationModel.toWooPosVariation(): WooPosVariation { - val attributesList = try { - if (attributesJson.isNotEmpty()) { - parseAttributesJson(attributesJson) - } else { - emptyList() - } - } catch (e: JsonSyntaxException) { - WooLog.w(WooLog.T.POS, "Failed to parse attributes JSON for variation ${remoteVariationId.value}: ${e.message}") - emptyList() - } - - return WooPosVariation( - remoteVariationId = remoteVariationId.value, - remoteProductId = remoteProductId.value, - globalUniqueId = globalUniqueId, - price = price.toBigDecimalOrNull(), - image = if (imageUrl.isNotEmpty()) WooPosVariation.WooPosVariationImage(imageUrl) else null, - attributes = attributesList, - isVisible = status == "publish", - isDownloadable = downloadable - ) -} - -private val gson by lazy { Gson() } - -@Suppress("SwallowedException") -private fun parseAttributesJson(attributesJson: String): List { - return try { - val type = object : TypeToken>() {}.type - val items: List = gson.fromJson(attributesJson, type) - items.map { item -> - WooPosVariation.WooPosVariationAttribute( - id = item.id, - name = item.name, - option = item.option - ) - } - } catch (e: JsonSyntaxException) { - WooLog.w(WooLog.T.POS, "Failed to parse attributes JSON: ${e.message}") - emptyList() - } -} - -private data class AttributeJsonItem( - val id: Long?, - val name: String?, - val option: String? -) - -fun WooPosVariation.getNameForPOS( - parentProduct: Product? = null, - resourceProvider: ResourceProvider, -): String { - return parentProduct?.variationEnabledAttributes?.joinToString(", ") { attribute -> - val option = attributes.firstOrNull { it.name == attribute.name } - if (option?.option != null) { - "${attribute.name}: ${option.option}" - } else { - resourceProvider.getString( - R.string.woopos_variations_any_variation, - attribute.name - ) - } - } ?: attributes.joinToString(", ") { attribute -> - when { - attribute.option != null && attribute.name != null -> "${attribute.name}: ${attribute.option}" - attribute.option != null -> attribute.option - attribute.name != null -> resourceProvider.getString( - R.string.woopos_variations_any_variation, - attribute.name - ) - else -> "" - } - } -} - -fun WooPosVariation.getName(parentProduct: Product? = null): String { - return parentProduct?.variationEnabledAttributes?.joinToString(" - ") { attribute -> - val option = attributes.firstOrNull { it.name == attribute.name } - option?.option ?: "Any ${attribute.name}" - } ?: attributes.mapNotNull { it.option }.joinToString(" - ") -} diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariationMapper.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariationMapper.kt new file mode 100644 index 000000000000..44e68e88a4ec --- /dev/null +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariationMapper.kt @@ -0,0 +1,175 @@ +package com.woocommerce.android.ui.woopos.common.data + +import com.google.gson.Gson +import com.google.gson.JsonSyntaxException +import com.google.gson.reflect.TypeToken +import com.woocommerce.android.R +import com.woocommerce.android.model.Product +import com.woocommerce.android.model.ProductVariation +import com.woocommerce.android.util.WooLog +import com.woocommerce.android.viewmodel.ResourceProvider +import org.wordpress.android.fluxc.model.WCProductVariationModel +import org.wordpress.android.fluxc.persistence.entity.pos.WCPosVariationModel +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Mapper class responsible for all WooPosVariation conversions and transformations. + * This centralizes all variation mapping logic, JSON parsing, and name generation. + */ +@Singleton +class WooPosVariationMapper @Inject constructor( + private val gson: Gson +) { + fun fromProductVariation(productVariation: ProductVariation): WooPosVariation { + return WooPosVariation( + remoteVariationId = productVariation.remoteVariationId, + remoteProductId = productVariation.remoteProductId, + globalUniqueId = productVariation.globalUniqueId, + price = productVariation.price, + image = productVariation.image?.let { WooPosVariation.WooPosVariationImage(it.source) }, + attributes = productVariation.attributes.map { + WooPosVariation.WooPosVariationAttribute( + id = it.id, + name = it.name, + option = it.option + ) + }, + isVisible = productVariation.isVisible, + isDownloadable = productVariation.isDownloadable + ) + } + + @Suppress("SwallowedException") + fun fromWCProductVariationModel(model: WCProductVariationModel): WooPosVariation { + val attributesList = model.attributeList?.map { attribute -> + WooPosVariation.WooPosVariationAttribute( + id = attribute.id, + name = attribute.name, + option = attribute.option + ) + } ?: emptyList() + + val imageModel = try { + if (model.image.isNotEmpty()) model.getImageModel() else null + } catch (e: JsonSyntaxException) { + WooLog.w( + WooLog.T.POS, + "Failed to parse image from JSON attributes for variation " + + "${model.remoteVariationId.value}: ${e.message}" + ) + null + } + + return WooPosVariation( + remoteVariationId = model.remoteVariationId.value, + remoteProductId = model.remoteProductId.value, + globalUniqueId = model.globalUniqueId, + price = model.price.toBigDecimalOrNull(), + image = imageModel?.src?.let { WooPosVariation.WooPosVariationImage(it) }, + attributes = attributesList, + isVisible = model.status == "publish", + isDownloadable = model.downloadable + ) + } + + @Suppress("SwallowedException") + fun fromWCPosVariationModel(model: WCPosVariationModel): WooPosVariation { + val attributesList = parseAttributesJson(model.attributesJson) + + return WooPosVariation( + remoteVariationId = model.remoteVariationId.value, + remoteProductId = model.remoteProductId.value, + globalUniqueId = model.globalUniqueId, + price = model.price.toBigDecimalOrNull(), + image = if (model.imageUrl.isNotEmpty()) WooPosVariation.WooPosVariationImage(model.imageUrl) else null, + attributes = attributesList, + isVisible = model.status == "publish", + isDownloadable = model.downloadable + ) + } + + fun getNameForPOS( + variation: WooPosVariation, + parentProduct: Product? = null, + resourceProvider: ResourceProvider, + ): String { + return parentProduct?.variationEnabledAttributes?.joinToString(", ") { attribute -> + val option = variation.attributes.firstOrNull { it.name == attribute.name } + if (option?.option != null) { + "${attribute.name}: ${option.option}" + } else { + resourceProvider.getString( + R.string.woopos_variations_any_variation, + attribute.name + ) + } + } ?: variation.attributes.joinToString(", ") { attribute -> + when { + attribute.option != null && attribute.name != null -> "${attribute.name}: ${attribute.option}" + attribute.option != null -> attribute.option + attribute.name != null -> resourceProvider.getString( + R.string.woopos_variations_any_variation, + attribute.name + ) + else -> "" + } + } + } + + fun getName(variation: WooPosVariation, parentProduct: Product? = null): String { + return parentProduct?.variationEnabledAttributes?.joinToString(" - ") { attribute -> + val option = variation.attributes.firstOrNull { it.name == attribute.name } + option?.option ?: "Any ${attribute.name}" + } ?: variation.attributes.mapNotNull { it.option }.joinToString(" - ") + } + + /** + * Parses JSON string containing variation attributes into a list of WooPosVariationAttribute objects. + * + * @param attributesJson The JSON string to parse + * @return List of parsed attributes, or empty list if parsing fails + */ + private fun parseAttributesJson(attributesJson: String): List { + if (attributesJson.isEmpty()) return emptyList() + + return try { + val type = object : TypeToken>() {}.type + val items: List = gson.fromJson(attributesJson, type) + items.map { item -> + WooPosVariation.WooPosVariationAttribute( + id = item.id, + name = item.name, + option = item.option + ) + } + } catch (e: JsonSyntaxException) { + WooLog.w(WooLog.T.POS, "Failed to parse attributes JSON: ${e.message}") + emptyList() + } + } + + private data class AttributeJsonItem( + val id: Long?, + val name: String?, + val option: String? + ) +} + +fun ProductVariation.toWooPosVariation(mapper: WooPosVariationMapper): WooPosVariation = + mapper.fromProductVariation(this) + +fun WCProductVariationModel.toWooPosVariation(mapper: WooPosVariationMapper): WooPosVariation = + mapper.fromWCProductVariationModel(this) + +fun WCPosVariationModel.toWooPosVariation(mapper: WooPosVariationMapper): WooPosVariation = + mapper.fromWCPosVariationModel(this) + +fun WooPosVariation.getNameForPOS( + mapper: WooPosVariationMapper, + parentProduct: Product? = null, + resourceProvider: ResourceProvider, +): String = mapper.getNameForPOS(this, parentProduct, resourceProvider) + +fun WooPosVariation.getName(mapper: WooPosVariationMapper, parentProduct: Product? = null): String = + mapper.getName(this, parentProduct) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationFetch.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationFetch.kt index 889a5b65f90b..98e233b9e670 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationFetch.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationFetch.kt @@ -1,8 +1,8 @@ package com.woocommerce.android.ui.woopos.common.data.searchbyidentifier -import com.woocommerce.android.model.toAppModel import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.ui.woopos.common.data.WooPosVariation +import com.woocommerce.android.ui.woopos.common.data.WooPosVariationMapper import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsLRUCache import org.wordpress.android.fluxc.store.WCProductStore @@ -12,7 +12,8 @@ class WooPosSearchByIdentifierVariationFetch @Inject constructor( private val selectedSite: SelectedSite, private val productStore: WCProductStore, private val variationsCache: WooPosVariationsLRUCache, - private val errorMapper: WooPosSearchByIdentifierProductErrorMapper + private val errorMapper: WooPosSearchByIdentifierProductErrorMapper, + private val mapper: WooPosVariationMapper ) { sealed class VariationFetchResult { data class Success(val variation: WooPosVariation) : VariationFetchResult() @@ -35,7 +36,7 @@ class WooPosSearchByIdentifierVariationFetch @Inject constructor( selectedSite.get(), parentId, variationId - )?.toAppModel()?.toWooPosVariation() + )?.toWooPosVariation(mapper) return if (variation != null) { variationsCache.add(parentId, variation) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModel.kt index 1721187ce553..a8593490db11 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModel.kt @@ -13,6 +13,7 @@ import com.woocommerce.android.ui.woopos.common.data.WooPosGetCouponById import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById import com.woocommerce.android.ui.woopos.common.data.WooPosGetVariationById import com.woocommerce.android.ui.woopos.common.data.WooPosVariation +import com.woocommerce.android.ui.woopos.common.data.WooPosVariationMapper import com.woocommerce.android.ui.woopos.common.data.getNameForPOS import com.woocommerce.android.ui.woopos.common.data.searchbyidentifier.WooPosSearchByIdentifier import com.woocommerce.android.ui.woopos.common.data.searchbyidentifier.WooPosSearchByIdentifierResult @@ -69,6 +70,7 @@ class WooPosCartViewModel @Inject constructor( private val wooPosLogWrapper: WooPosLogWrapper, private val soundHelper: WooPosSoundHelper, private val barcodeEventTracker: WooPosBarcodeEventTracker, + private val variationMapper: WooPosVariationMapper, savedState: SavedStateHandle, ) : ViewModel() { private val _state = savedState.getStateFlow( @@ -598,7 +600,7 @@ class WooPosCartViewModel @Inject constructor( id = product.remoteId, variationId = this.remoteVariationId, name = product.name, - description = getNameForPOS(product, resourceProvider), + description = getNameForPOS(variationMapper, product, resourceProvider), price = formatPrice(price), imageUrl = image?.source, ) @@ -635,7 +637,7 @@ class WooPosCartViewModel @Inject constructor( id = variation.remoteProductId, variationId = variation.remoteVariationId, name = this.parentProduct.name, - description = variation.getNameForPOS(this.parentProduct, resourceProvider), + description = variation.getNameForPOS(variationMapper, this.parentProduct, resourceProvider), price = formatPrice(variation.price), imageUrl = variation.image?.source ) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt index 8faf1e06b009..722d3fb36d31 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt @@ -3,6 +3,7 @@ package com.woocommerce.android.ui.woopos.home.items.variations import com.woocommerce.android.model.ProductVariation import com.woocommerce.android.ui.products.variations.selector.VariationListHandler import com.woocommerce.android.ui.woopos.common.data.WooPosVariation +import com.woocommerce.android.ui.woopos.common.data.WooPosVariationMapper import com.woocommerce.android.ui.woopos.common.data.WooPosVariationsTypesFilterConfig import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.util.WooLog @@ -20,7 +21,8 @@ import javax.inject.Singleton class WooPosVariationsDataSource @Inject constructor( private val handler: VariationListHandler, private val variationCache: WooPosVariationsLRUCache, - private val variationFilterConfig: WooPosVariationsTypesFilterConfig + private val variationFilterConfig: WooPosVariationsTypesFilterConfig, + private val mapper: WooPosVariationMapper ) { private suspend fun getCachedVariations(productId: Long): List { return variationCache.get(productId) ?: emptyList() @@ -58,7 +60,7 @@ class WooPosVariationsDataSource @Inject constructor( ) if (result.isSuccess) { val remoteVariations = handler.getVariationsFlow(productId).firstOrNull()?.applyFilter()?.map { - it.toWooPosVariation() + it.toWooPosVariation(mapper) } ?: emptyList() updateCache(productId, remoteVariations) emit(FetchResult.Remote(Result.success(remoteVariations))) @@ -81,7 +83,7 @@ class WooPosVariationsDataSource @Inject constructor( if (result.isSuccess) { val fetchedVariations = handler.getVariationsFlow( productId - ).first().applyFilter().map { it.toWooPosVariation() } + ).first().applyFilter().map { it.toWooPosVariation(mapper) } Result.success(fetchedVariations) } else { result.logFailure() diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt index 0560121ea9b3..3cff41e69cc6 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt @@ -3,9 +3,9 @@ package com.woocommerce.android.ui.woopos.home.items.variations import androidx.annotation.VisibleForTesting import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import com.woocommerce.android.model.Product import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById import com.woocommerce.android.ui.woopos.common.data.WooPosVariation +import com.woocommerce.android.ui.woopos.common.data.WooPosVariationMapper import com.woocommerce.android.ui.woopos.common.data.getNameForPOS import com.woocommerce.android.ui.woopos.home.ChildToParentEvent import com.woocommerce.android.ui.woopos.home.WooPosChildrenToParentEventSender @@ -37,6 +37,7 @@ class WooPosVariationsViewModel @Inject constructor( private val priceFormat: WooPosFormatPrice, private val resourceProvider: ResourceProvider, private val analyticsTracker: WooPosAnalyticsTracker, + private val mapper: WooPosVariationMapper, ) : ViewModel() { private val _viewState = @@ -99,7 +100,11 @@ class WooPosVariationsViewModel @Inject constructor( items = variations.map { WooPosItemSelectionViewState.Product.Variation( id = it.remoteVariationId, - name = it.getNameForPOS(getProductById(productId), resourceProvider), + name = it.getNameForPOS( + mapper, + getProductById(productId), + resourceProvider + ), productId = it.remoteProductId, price = priceFormat(it.price), imageUrl = it.image?.source @@ -132,7 +137,11 @@ class WooPosVariationsViewModel @Inject constructor( items = variations.map { WooPosItemSelectionViewState.Product.Variation( id = it.remoteVariationId, - name = it.getNameForPOS(getProductById(productId), resourceProvider), + name = it.getNameForPOS( + mapper, + getProductById(productId), + resourceProvider + ), productId = it.remoteProductId, price = priceFormat(it.price), imageUrl = it.image?.source @@ -182,7 +191,11 @@ class WooPosVariationsViewModel @Inject constructor( items = result.getOrThrow().map { WooPosItemSelectionViewState.Product.Variation( id = it.remoteVariationId, - name = it.getNameForPOS(getProductById(productId), resourceProvider), + name = it.getNameForPOS( + mapper, + getProductById(productId), + resourceProvider + ), productId = it.remoteProductId, price = priceFormat(it.price), imageUrl = it.image?.source diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsRepository.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsRepository.kt index eec6cb28e0a5..95b17586d4b0 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsRepository.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsRepository.kt @@ -7,6 +7,7 @@ import com.woocommerce.android.ui.orders.creation.OrderCreateEditRepository import com.woocommerce.android.ui.orders.creation.OrderCreationSource import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById import com.woocommerce.android.ui.woopos.common.data.WooPosGetVariationById +import com.woocommerce.android.ui.woopos.common.data.WooPosVariationMapper import com.woocommerce.android.ui.woopos.common.data.getName import com.woocommerce.android.ui.woopos.common.data.getNameForPOS import com.woocommerce.android.ui.woopos.home.items.WooPosItemsViewModel @@ -29,6 +30,7 @@ class WooPosTotalsRepository @Inject constructor( private val selectedSite: SelectedSite, private val orderMapper: OrderMapper, private val resourceProvider: ResourceProvider, + private val variationMapper: WooPosVariationMapper, ) { private var orderCreationJob: Deferred>? = null @@ -118,7 +120,7 @@ class WooPosTotalsRepository @Inject constructor( productId = itemData.productId, variationId = itemData.id )!! - variationResult.getNameForPOS(productResult, resourceProvider) + variationResult.getNameForPOS(variationMapper, productResult, resourceProvider) return Order.Item.EMPTY.copy( itemId = 0L, productId = itemData.productId, @@ -129,7 +131,7 @@ class WooPosTotalsRepository @Inject constructor( attributesList = variationResult.attributes .filterNot { it.name.isNullOrEmpty() || it.option.isNullOrEmpty() } .map { Order.Item.Attribute(it.name!!, it.option!!) }, - name = variationResult.getName(productResult), + name = variationResult.getName(variationMapper, productResult), ) } diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResultConverterTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResultConverterTest.kt index c4116acf2720..1dcc5aedb9e5 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResultConverterTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResultConverterTest.kt @@ -2,6 +2,7 @@ package com.woocommerce.android.ui.woopos.common.data.searchbyidentifier import com.woocommerce.android.ui.products.ProductTestUtils import com.woocommerce.android.ui.woopos.common.data.WooPosProductsCache +import com.woocommerce.android.ui.woopos.common.data.WooPosVariationMapper import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.util.WooPosCoroutineTestRule import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -25,9 +26,10 @@ class WooPosSearchByIdentifierResultConverterTest { private lateinit var sut: WooPosSearchByIdentifierResultConverter private val productsCache: WooPosProductsCache = mock() private val variationProcess: WooPosSearchByIdentifierProcessVariationResult = mock() + private val variationMapper: WooPosVariationMapper = mock() private val testProduct = ProductTestUtils.generateProduct() - private val testVariation = ProductTestUtils.generateProductVariation().toWooPosVariation() + private val testVariation = ProductTestUtils.generateProductVariation().toWooPosVariation(variationMapper) @Before fun setup() { diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsRepositoryTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsRepositoryTest.kt index d741b8a79f66..fd57b3f37b3d 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsRepositoryTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsRepositoryTest.kt @@ -9,6 +9,7 @@ import com.woocommerce.android.ui.products.ProductHelper import com.woocommerce.android.ui.products.ProductType import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById import com.woocommerce.android.ui.woopos.common.data.WooPosGetVariationById +import com.woocommerce.android.ui.woopos.common.data.WooPosVariationMapper import com.woocommerce.android.ui.woopos.home.items.WooPosItemsViewModel import com.woocommerce.android.util.DateUtils import com.woocommerce.android.viewmodel.ResourceProvider @@ -33,6 +34,7 @@ class WooPosTotalsRepositoryTest { private val selectedSite: SelectedSite = mock() private val orderMapper: OrderMapper = mock() private val resourceProvider: ResourceProvider = mock() + private val variationMapper: WooPosVariationMapper = mock() private lateinit var repository: WooPosTotalsRepository @@ -233,5 +235,6 @@ class WooPosTotalsRepositoryTest { selectedSite, orderMapper, resourceProvider, + variationMapper, ) } From 2dc7ce91f3306c4faffa437877bf2a4f05a7be91 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 5 Sep 2025 16:53:35 +0200 Subject: [PATCH 19/19] Update tests --- ...osSearchByIdentifierResultConverterTest.kt | 17 ++- ...archByIdentifierVariationGetOrFetchTest.kt | 23 +++- .../home/cart/WooPosCartViewModelTest.kt | 44 +++++-- .../WooPosVariationsDataSourceTest.kt | 100 +++++++++++++-- .../WooPosVariationsViewModelTest.kt | 118 ++++++++++++++---- 5 files changed, 253 insertions(+), 49 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResultConverterTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResultConverterTest.kt index 1dcc5aedb9e5..9a39a5b6d63c 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResultConverterTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierResultConverterTest.kt @@ -2,6 +2,7 @@ package com.woocommerce.android.ui.woopos.common.data.searchbyidentifier import com.woocommerce.android.ui.products.ProductTestUtils import com.woocommerce.android.ui.woopos.common.data.WooPosProductsCache +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation import com.woocommerce.android.ui.woopos.common.data.WooPosVariationMapper import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.util.WooPosCoroutineTestRule @@ -12,6 +13,7 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.Before import org.junit.Rule import org.junit.Test +import org.mockito.kotlin.any import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -29,10 +31,23 @@ class WooPosSearchByIdentifierResultConverterTest { private val variationMapper: WooPosVariationMapper = mock() private val testProduct = ProductTestUtils.generateProduct() - private val testVariation = ProductTestUtils.generateProductVariation().toWooPosVariation(variationMapper) + private val testProductVariation = ProductTestUtils.generateProductVariation() + private val testVariation by lazy { testProductVariation.toWooPosVariation(variationMapper) } @Before fun setup() { + whenever(variationMapper.fromProductVariation(any())).thenReturn( + WooPosVariation( + remoteVariationId = 1L, + remoteProductId = 1L, + globalUniqueId = "test-unique-id", + price = java.math.BigDecimal("10.0"), + image = null, + attributes = emptyList(), + isVisible = true, + isDownloadable = false + ) + ) sut = WooPosSearchByIdentifierResultConverter(productsCache, variationProcess) } diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationGetOrFetchTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationGetOrFetchTest.kt index a849e3113369..b29151b3a764 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationGetOrFetchTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationGetOrFetchTest.kt @@ -2,12 +2,14 @@ package com.woocommerce.android.ui.woopos.common.data.searchbyidentifier import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.ui.woopos.common.data.WooPosVariation +import com.woocommerce.android.ui.woopos.common.data.WooPosVariationMapper import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsLRUCache import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test +import org.mockito.kotlin.any import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -27,11 +29,26 @@ class WooPosSearchByIdentifierVariationGetOrFetchTest { private val variationsCache: WooPosVariationsLRUCache = mock() private val site: SiteModel = mock() private val errorMapper: WooPosSearchByIdentifierProductErrorMapper = WooPosSearchByIdentifierProductErrorMapper() + private val mapper: WooPosVariationMapper = mock() @Before fun setup() { - sut = WooPosSearchByIdentifierVariationFetch(selectedSite, productStore, variationsCache, errorMapper) + sut = WooPosSearchByIdentifierVariationFetch(selectedSite, productStore, variationsCache, errorMapper, mapper) whenever(selectedSite.get()).thenReturn(site) + + // Mock the mapper to return a valid WooPosVariation + whenever(mapper.fromWCProductVariationModel(any())).thenReturn( + WooPosVariation( + remoteVariationId = 456L, + remoteProductId = 123L, + globalUniqueId = "test-global-id", + price = java.math.BigDecimal("10.0"), + image = null, + attributes = emptyList(), + isVisible = true, + isDownloadable = false + ) + ) } @Test @@ -43,7 +60,7 @@ class WooPosSearchByIdentifierVariationGetOrFetchTest { remoteVariationId = RemoteId(variationId), remoteProductId = RemoteId(parentId) ) - val variation = wcVariation.toWooPosVariation() + val variation = wcVariation.toWooPosVariation(mapper) val fetchResult: WCProductStore.OnVariationChanged = mock { on { isError }.thenReturn(false) } @@ -132,7 +149,7 @@ class WooPosSearchByIdentifierVariationGetOrFetchTest { remoteVariationId = RemoteId(variationId), remoteProductId = RemoteId(parentId) ) - val variation = wcVariation.toWooPosVariation() + val variation = wcVariation.toWooPosVariation(mapper) val fetchResult: WCProductStore.OnVariationChanged = mock { on { isError }.thenReturn(false) } diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModelTest.kt index bfcec181e00d..7bca931a9b23 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModelTest.kt @@ -5,14 +5,16 @@ import androidx.lifecycle.SavedStateHandle import com.woocommerce.android.R import com.woocommerce.android.model.Coupon import com.woocommerce.android.model.ProductVariation -import com.woocommerce.android.ui.products.ProductStockStatus import com.woocommerce.android.ui.products.ProductTestUtils import com.woocommerce.android.ui.woopos.common.composeui.modifier.BarcodeInputDetector import com.woocommerce.android.ui.woopos.common.data.WooPosGetCouponById import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById import com.woocommerce.android.ui.woopos.common.data.WooPosGetVariationById +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation +import com.woocommerce.android.ui.woopos.common.data.WooPosVariationMapper import com.woocommerce.android.ui.woopos.common.data.searchbyidentifier.WooPosSearchByIdentifier import com.woocommerce.android.ui.woopos.common.data.searchbyidentifier.WooPosSearchByIdentifierResult +import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.common.util.WooPosLogWrapper import com.woocommerce.android.ui.woopos.common.util.WooPosSoundHelper import com.woocommerce.android.ui.woopos.home.ChildToParentEvent @@ -45,6 +47,7 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.Rule import org.mockito.kotlin.any import org.mockito.kotlin.clearInvocations +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.doSuspendableAnswer import org.mockito.kotlin.eq import org.mockito.kotlin.mock @@ -107,6 +110,28 @@ class WooPosCartViewModelTest { private val analyticsTracker: WooPosAnalyticsTracker = mock() + private val variationMapper: WooPosVariationMapper = mock { + on { fromProductVariation(any()) } doAnswer { invocation -> + val productVariation = invocation.arguments[0] as ProductVariation + WooPosVariation( + remoteVariationId = productVariation.remoteVariationId, + remoteProductId = productVariation.remoteProductId, + globalUniqueId = productVariation.globalUniqueId, + price = productVariation.price, + image = productVariation.image?.let { WooPosVariation.WooPosVariationImage(it.source) }, + attributes = productVariation.attributes.map { + WooPosVariation.WooPosVariationAttribute( + id = it.id, + name = it.name, + option = it.option + ) + }, + isVisible = productVariation.isVisible, + isDownloadable = productVariation.isDownloadable + ) + } + } + private val savedState: SavedStateHandle = SavedStateHandle() private val trackerData: WooPosAnalyticsTrackingDataKeeper = WooPosAnalyticsTrackingDataKeeper() private val cartItemsUpdater: WooPosCartItemsUpdater = mock() @@ -150,7 +175,7 @@ class WooPosCartViewModelTest { productId = 23L, variationId = 24L, amount = "10.0" - ) + ).toWooPosVariation(variationMapper) val product = ProductTestUtils.generateProduct( productId = 23L, productName = "title", @@ -1348,15 +1373,11 @@ class WooPosCartViewModelTest { // GIVEN val productId = 100L val variationId = 200L - val variation: ProductVariation = mock { - on { remoteVariationId }.thenReturn(variationId) - on { remoteProductId }.thenReturn(productId) - on { getName() }.thenReturn("Red Hoodie - Size L") - on { price }.thenReturn(BigDecimal("45.0")) - on { stockStatus }.thenReturn(ProductStockStatus.InStock) - on { image }.thenReturn(null) - on { attributes }.thenReturn(emptyArray()) - } + val variation = ProductTestUtils.generateProductVariation( + productId = productId, + variationId = variationId, + amount = "45.0" + ).toWooPosVariation(variationMapper) val product = ProductTestUtils.generateProduct( productId = productId, @@ -1460,6 +1481,7 @@ class WooPosCartViewModelTest { wooPosLogWrapper, soundHelper, barcodeEventTracker, + variationMapper, savedState, ) } diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsDataSourceTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsDataSourceTest.kt index 65104271b1f9..4e92062dcbf7 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsDataSourceTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsDataSourceTest.kt @@ -2,6 +2,8 @@ package com.woocommerce.android.ui.woopos.home.variations import com.woocommerce.android.ui.products.ProductTestUtils import com.woocommerce.android.ui.products.variations.selector.VariationListHandler +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation +import com.woocommerce.android.ui.woopos.common.data.WooPosVariationMapper import com.woocommerce.android.ui.woopos.common.data.WooPosVariationsTypesFilterConfig import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.home.items.variations.FetchResult @@ -16,6 +18,8 @@ import kotlinx.coroutines.flow.toList import kotlinx.coroutines.test.runTest import org.assertj.core.api.Assertions.assertThat import org.junit.Rule +import org.mockito.kotlin.any +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.whenever import org.wordpress.android.fluxc.store.WCProductStore @@ -27,6 +31,28 @@ class WooPosVariationsDataSourceTest { @JvmField val coroutinesTestRule = WooPosCoroutineTestRule() + private val variationMapper: WooPosVariationMapper = mock { + on { fromProductVariation(any()) } doAnswer { invocation -> + val productVariation = invocation.arguments[0] as com.woocommerce.android.model.ProductVariation + WooPosVariation( + remoteVariationId = productVariation.remoteVariationId, + remoteProductId = productVariation.remoteProductId, + globalUniqueId = productVariation.globalUniqueId, + price = productVariation.price, + image = productVariation.image?.let { WooPosVariation.WooPosVariationImage(it.source) }, + attributes = productVariation.attributes.map { + WooPosVariation.WooPosVariationAttribute( + id = it.id, + name = it.name, + option = it.option + ) + }, + isVisible = productVariation.isVisible, + isDownloadable = productVariation.isDownloadable + ) + } + } + private val sampleProductVariations = listOf( ProductTestUtils.generateProductVariation( productId = 1, @@ -48,7 +74,7 @@ class WooPosVariationsDataSourceTest { ) ) - private val sampleProducts = sampleProductVariations.map { it.toWooPosVariation() } + private val sampleProducts = sampleProductVariations.map { it.toWooPosVariation(variationMapper) } private val additionalProductVariations = listOf( ProductTestUtils.generateProductVariation( @@ -65,7 +91,7 @@ class WooPosVariationsDataSourceTest { ), ) - private val additionalProducts = additionalProductVariations.map { it.toWooPosVariation() } + private val additionalProducts = additionalProductVariations.map { it.toWooPosVariation(variationMapper) } private val handler: VariationListHandler = mock() private val variationsCache: WooPosVariationsLRUCache = mock() @@ -77,7 +103,12 @@ class WooPosVariationsDataSourceTest { whenever(handler.canLoadMore(5)).thenReturn(true) whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProductVariations)) whenever(variationsCache.get(productId)).thenReturn(sampleProducts) - val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) + val sut = WooPosVariationsDataSource( + handler, + variationsCache, + WooPosVariationsTypesFilterConfig(), + variationMapper + ) sut.fetchFirstPage(productId, forceRefresh = true).first() assertThat( @@ -102,7 +133,12 @@ class WooPosVariationsDataSourceTest { whenever(handler.canLoadMore(5)).thenReturn(true) whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProductVariations)) whenever(variationsCache.get(productId)).thenReturn(sampleProducts) - val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) + val sut = WooPosVariationsDataSource( + handler, + variationsCache, + WooPosVariationsTypesFilterConfig(), + variationMapper + ) sut.fetchFirstPage(productId, forceRefresh = true).first() @@ -123,7 +159,12 @@ class WooPosVariationsDataSourceTest { whenever(handler.canLoadMore(5)).thenReturn(true) whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProductVariations)) whenever(variationsCache.get(productId)).thenReturn(sampleProducts) - val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) + val sut = WooPosVariationsDataSource( + handler, + variationsCache, + WooPosVariationsTypesFilterConfig(), + variationMapper + ) sut.fetchFirstPage(productId, forceRefresh = true).first() @@ -148,7 +189,12 @@ class WooPosVariationsDataSourceTest { whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(sampleProductVariations)) whenever(variationsCache.get(productId)).thenReturn(sampleProducts) val exception = Exception("Remote load failed") - val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) + val sut = WooPosVariationsDataSource( + handler, + variationsCache, + WooPosVariationsTypesFilterConfig(), + variationMapper + ) sut.fetchFirstPage(productId, forceRefresh = true).first() @@ -187,7 +233,12 @@ class WooPosVariationsDataSourceTest { ) whenever(variationsCache.get(productId)).thenReturn(sampleProducts + additionalProducts) whenever(handler.loadMore(productId)).thenReturn(Result.success(Unit)) - val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) + val sut = WooPosVariationsDataSource( + handler, + variationsCache, + WooPosVariationsTypesFilterConfig(), + variationMapper + ) sut.fetchFirstPage(productId, forceRefresh = false).first() @@ -221,7 +272,12 @@ class WooPosVariationsDataSourceTest { ), ).thenReturn(Result.failure(exception)) whenever(variationsCache.get(productId)).thenReturn(sampleProducts) - val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) + val sut = WooPosVariationsDataSource( + handler, + variationsCache, + WooPosVariationsTypesFilterConfig(), + variationMapper + ) sut.fetchFirstPage(productId, forceRefresh = false).first() @@ -257,7 +313,12 @@ class WooPosVariationsDataSourceTest { ).thenReturn(Result.failure(exception)) whenever(variationsCache.get(productId)).thenReturn(emptyList()) - val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) + val sut = WooPosVariationsDataSource( + handler, + variationsCache, + WooPosVariationsTypesFilterConfig(), + variationMapper + ) // WHEN val flow = sut.fetchFirstPage(productId, forceRefresh = false).toList() @@ -276,7 +337,12 @@ class WooPosVariationsDataSourceTest { whenever(handler.getVariationsFlow(productId)).thenReturn(flowOf(emptyList())) whenever(handler.fetchVariations(productId, forceRefresh = false)).thenReturn(Result.success(Unit)) whenever(variationsCache.get(productId)).thenReturn(emptyList()) - val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) + val sut = WooPosVariationsDataSource( + handler, + variationsCache, + WooPosVariationsTypesFilterConfig(), + variationMapper + ) // WHEN val flow = sut.fetchFirstPage(productId, forceRefresh = false).toList() @@ -308,7 +374,12 @@ class WooPosVariationsDataSourceTest { ) whenever(handler.fetchVariations(productId, forceRefresh = true)).thenReturn(Result.success(Unit)) whenever(variationsCache.get(productId)).thenReturn(sampleProducts) - val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) + val sut = WooPosVariationsDataSource( + handler, + variationsCache, + WooPosVariationsTypesFilterConfig(), + variationMapper + ) // WHEN val flow = sut.fetchFirstPage(productId, forceRefresh = false).toList() @@ -342,7 +413,12 @@ class WooPosVariationsDataSourceTest { ) whenever(handler.fetchVariations(productId, forceRefresh = true)).thenReturn(Result.success(Unit)) whenever(variationsCache.get(productId)).thenReturn(sampleProducts) - val sut = WooPosVariationsDataSource(handler, variationsCache, WooPosVariationsTypesFilterConfig()) + val sut = WooPosVariationsDataSource( + handler, + variationsCache, + WooPosVariationsTypesFilterConfig(), + variationMapper + ) // WHEN val flow = sut.fetchFirstPage(productId, forceRefresh = false).toList() diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt index 2eac4a69b740..681ed04b042e 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt @@ -4,6 +4,8 @@ import app.cash.turbine.test import com.woocommerce.android.R import com.woocommerce.android.ui.products.ProductTestUtils import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById +import com.woocommerce.android.ui.woopos.common.data.WooPosVariation +import com.woocommerce.android.ui.woopos.common.data.WooPosVariationMapper import com.woocommerce.android.ui.woopos.common.data.getNameForPOS import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation import com.woocommerce.android.ui.woopos.home.ChildToParentEvent @@ -31,6 +33,7 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.Rule import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.verify @@ -56,13 +59,59 @@ class WooPosVariationsViewModelTest { onBlocking { invoke(null) }.thenReturn("$0.00") } private val analyticsTracker: WooPosAnalyticsTracker = mock() + private val mapper: WooPosVariationMapper = mock { + on { fromProductVariation(any()) } doAnswer { invocation -> + val productVariation = invocation.arguments[0] as com.woocommerce.android.model.ProductVariation + WooPosVariation( + remoteVariationId = productVariation.remoteVariationId, + remoteProductId = productVariation.remoteProductId, + globalUniqueId = productVariation.globalUniqueId, + price = productVariation.price, + image = productVariation.image?.let { WooPosVariation.WooPosVariationImage(it.source) }, + attributes = productVariation.attributes.map { + WooPosVariation.WooPosVariationAttribute( + id = it.id, + name = it.name, + option = it.option + ) + }, + isVisible = productVariation.isVisible, + isDownloadable = productVariation.isDownloadable + ) + } + on { getNameForPOS(any(), anyOrNull(), any()) } doAnswer { invocation -> + val variation = invocation.arguments[0] as WooPosVariation + val parentProduct = invocation.arguments[1] as? com.woocommerce.android.model.Product + // Mock the basic behavior for tests + if (parentProduct != null) { + // Use parent product's variation enabled attributes + parentProduct.variationEnabledAttributes?.joinToString(", ") { attribute -> + val option = variation.attributes.firstOrNull { it.name == attribute.name } + if (option?.option != null) { + "${attribute.name}: ${option.option}" + } else { + "Any ${attribute.name}" + } + } ?: "Any variation" + } else { + // No parent product, use all variation attributes + variation.attributes.mapNotNull { attribute -> + if (attribute.name != null && attribute.option != null) { + "${attribute.name}: ${attribute.option}" + } else { + null + } + }.joinToString(", ").takeIf { it.isNotEmpty() } ?: "Any variation" + } + } + } @Test fun `given variations from data source, when view model created, then view state updated correctly`() = runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), - ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation() + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(mapper), + ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation(mapper) ) whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( flowOf(FetchResult.Remote(Result.success(variations))) @@ -70,6 +119,7 @@ class WooPosVariationsViewModelTest { // WHEN val viewModel = createViewModel() + advanceUntilIdle() viewModel.viewState.test { // THEN @@ -84,7 +134,7 @@ class WooPosVariationsViewModelTest { fun `given variation has zero price, when view model created, then view state updated correctly`() = runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "0.00").toWooPosVariation(), + ProductTestUtils.generateProductVariation(1, 1, "0.00").toWooPosVariation(mapper), ) whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( flowOf(FetchResult.Remote(Result.success(variations))) @@ -92,6 +142,7 @@ class WooPosVariationsViewModelTest { // WHEN val viewModel = createViewModel() + advanceUntilIdle() viewModel.viewState.test { // THEN @@ -106,7 +157,7 @@ class WooPosVariationsViewModelTest { fun `given variation has no price set, when view model created, then view state updated correctly`() = runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "").toWooPosVariation(), + ProductTestUtils.generateProductVariation(1, 1, "").toWooPosVariation(mapper), ) whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( flowOf(FetchResult.Remote(Result.success(variations))) @@ -114,6 +165,7 @@ class WooPosVariationsViewModelTest { // WHEN val viewModel = createViewModel() + advanceUntilIdle() viewModel.viewState.test { // THEN @@ -159,8 +211,8 @@ class WooPosVariationsViewModelTest { fun `given variations, when pull to refresh triggered, then fetchFirstPage is called`() = runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), - ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation() + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(mapper), + ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation(mapper) ) whenever(variationsDataSource.fetchFirstPage(any(), eq(false))).thenReturn( flowOf(FetchResult.Remote(Result.success(emptyList()))) @@ -185,6 +237,7 @@ class WooPosVariationsViewModelTest { // WHEN val viewModel = createViewModel() + advanceUntilIdle() viewModel.onUIEvent(WooPosVariationsUIEvents.PullToRefreshTriggered(123L)) // THEN @@ -201,8 +254,8 @@ class WooPosVariationsViewModelTest { fun `given no more variations, when end of list reached, then pagination state is none`() = runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), - ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation() + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(mapper), + ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation(mapper) ) whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( flowOf(FetchResult.Remote(Result.success(variations))) @@ -227,7 +280,7 @@ class WooPosVariationsViewModelTest { fun `given variations, when load more succeeds, then pagination state is updated`() = runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(mapper), ) whenever(variationsDataSource.loadMore(any())).thenReturn(Result.success(variations)) whenever(variationsDataSource.canLoadMore(any())).thenReturn(true) @@ -237,8 +290,8 @@ class WooPosVariationsViewModelTest { FetchResult.Remote( Result.success( listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), - ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation() + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(mapper), + ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation(mapper) ) ) ) @@ -247,7 +300,9 @@ class WooPosVariationsViewModelTest { ) val viewModel = createViewModel() + advanceUntilIdle() viewModel.onUIEvent(WooPosVariationsUIEvents.EndOfItemsListReached(123L, 10)) + advanceUntilIdle() // THEN viewModel.viewState.test { @@ -268,8 +323,8 @@ class WooPosVariationsViewModelTest { FetchResult.Remote( Result.success( listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), - ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation() + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(mapper), + ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation(mapper) ) ) ) @@ -294,8 +349,8 @@ class WooPosVariationsViewModelTest { runTest { // GIVEN val variations = listOf( - ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(), - ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation() + ProductTestUtils.generateProductVariation(1, 1, "10.0").toWooPosVariation(mapper), + ProductTestUtils.generateProductVariation(2, 1, "20.0").toWooPosVariation(mapper) ) whenever(variationsDataSource.fetchFirstPage(any(), any())).thenReturn( flowOf(FetchResult.Remote(Result.success(variations))) @@ -303,7 +358,9 @@ class WooPosVariationsViewModelTest { // WHEN val viewModel = createViewModel() + advanceUntilIdle() viewModel.init(1L, sourceType = WooPosAnalyticsEventConstant.ItemsListSourceType.LIST) + advanceUntilIdle() viewModel.viewState.test { // THEN @@ -321,7 +378,9 @@ class WooPosVariationsViewModelTest { ) val viewModel = createViewModel() + advanceUntilIdle() viewModel.init(123L, sourceType = ItemsListSourceType.LIST) + advanceUntilIdle() // WHEN viewModel.onUIEvent(WooPosVariationsUIEvents.OnItemClicked(123L, 1L)) @@ -347,6 +406,7 @@ class WooPosVariationsViewModelTest { flowOf(FetchResult.Remote(Result.success(emptyList()))) ) val viewModel = createViewModel(123L, sourceType = ItemsListSourceType.SEARCH_RESULT) + advanceUntilIdle() // WHEN viewModel.onUIEvent(WooPosVariationsUIEvents.OnItemClicked(123L, 1L)) @@ -397,7 +457,9 @@ class WooPosVariationsViewModelTest { ) // WHEN - val attributeName = variableProduct.toWooPosVariation().getNameForPOS(parentProduct, resourceProvider) + val attributeName = variableProduct.toWooPosVariation( + mapper + ).getNameForPOS(mapper, parentProduct, resourceProvider) // Then assertThat(attributeName).isEqualTo("Color: Blue") @@ -425,7 +487,8 @@ class WooPosVariationsViewModelTest { ) // WHEN - val attributeName = variableProduct.toWooPosVariation().getNameForPOS(null, resourceProvider) + val wooPosVariation = variableProduct.toWooPosVariation(mapper) + val attributeName = mapper.getNameForPOS(wooPosVariation, null, resourceProvider) // THEN assertThat(attributeName).isEqualTo("Color: Blue, Size: M") @@ -468,7 +531,9 @@ class WooPosVariationsViewModelTest { ).thenReturn("Any Material") // WHEN - val attributeName = variableProduct.toWooPosVariation().getNameForPOS(parentProduct, resourceProvider) + val attributeName = variableProduct.toWooPosVariation( + mapper + ).getNameForPOS(mapper, parentProduct, resourceProvider) // THEN assertThat(attributeName).isEqualTo("Any Material") @@ -514,7 +579,9 @@ class WooPosVariationsViewModelTest { ) // WHEN - val attributeName = variableProduct.toWooPosVariation().getNameForPOS(parentProduct, resourceProvider) + val attributeName = variableProduct.toWooPosVariation( + mapper + ).getNameForPOS(mapper, parentProduct, resourceProvider) // THEN assertThat(attributeName).isEqualTo("Color: Blue, Size: M") @@ -566,7 +633,9 @@ class WooPosVariationsViewModelTest { ).thenReturn("Any Size") // WHEN - val attributeName = variableProduct.toWooPosVariation().getNameForPOS(parentProduct, resourceProvider) + val attributeName = variableProduct.toWooPosVariation( + mapper + ).getNameForPOS(mapper, parentProduct, resourceProvider) // THEN assertThat(attributeName).isEqualTo("Any Color, Any Size") @@ -598,7 +667,9 @@ class WooPosVariationsViewModelTest { ).thenReturn("Any Color") // WHEN - val attributeName = variableProduct.toWooPosVariation().getNameForPOS(parentProduct, resourceProvider) + val attributeName = variableProduct.toWooPosVariation( + mapper + ).getNameForPOS(mapper, parentProduct, resourceProvider) // THEN assertThat(attributeName).isEqualTo("Any Color") @@ -644,7 +715,9 @@ class WooPosVariationsViewModelTest { ) // WHEN - val attributeName = variableProduct.toWooPosVariation().getNameForPOS(parentProduct, resourceProvider) + val attributeName = variableProduct.toWooPosVariation( + mapper + ).getNameForPOS(mapper, parentProduct, resourceProvider) // THEN assertThat(attributeName).isEqualTo("Color: Blue") @@ -661,6 +734,7 @@ class WooPosVariationsViewModelTest { priceFormat, resourceProvider, analyticsTracker, + mapper, ).let { it.init(productId, sourceType = sourceType) it