Skip to content

Commit

Permalink
Merge pull request #13312 from woocommerce/issue/13293-remove-null-pr…
Browse files Browse the repository at this point in the history 
…ice-check

[Woo POS][Non-Simple Products] Remove null price check on Products and Variations screen
  • Loading branch information
@AnirudhBhat
AnirudhBhat authored Jan 16, 2025
2 parents a938c73 + a953166 commit 07de8ee
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 172 deletions.
12 changes: 2 additions & 10 deletions .../kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsDataSource.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package com.woocommerce.android.ui.woopos.home.items.products

import com.woocommerce.android.model.Product
import com.woocommerce.android.ui.products.ProductStatus
import com.woocommerce.android.ui.products.ProductType.VARIABLE
import com.woocommerce.android.ui.products.selector.ProductListHandler
import com.woocommerce.android.util.WooLog
import kotlinx.coroutines.Dispatchers
Expand Down Expand Up @@ -46,7 +45,7 @@ class WooPosProductsDataSource @Inject constructor(
)

if (result.isSuccess) {
val remoteProducts = handler.productsFlow.first().applyPosProductFilter()
val remoteProducts = handler.productsFlow.first()
updateProductCache(remoteProducts)
emit(ProductsResult.Remote(Result.success(productCache)))
} else {
Expand All @@ -64,7 +63,7 @@ class WooPosProductsDataSource @Inject constructor(
suspend fun loadMore(): Result<List<Product>> = withContext(Dispatchers.IO) {
val result = handler.loadMore()
if (result.isSuccess) {
val moreProducts = handler.productsFlow.first().applyPosProductFilter()
val moreProducts = handler.productsFlow.first()
updateProductCache(moreProducts)
Result.success(productCache)
} else {
Expand All @@ -83,13 +82,6 @@ class WooPosProductsDataSource @Inject constructor(
WooLog.e(WooLog.T.POS, "Loading products failed - $errorMessage", error)
}

private fun List<Product>.applyPosProductFilter() = this.filter { product ->
isProductHasAPrice(product) || product.productType == VARIABLE
}

private fun isProductHasAPrice(product: Product) =
(product.price != null)

sealed class ProductsResult {
data class Cached(val products: List<Product>) : ProductsResult()
data class Remote(val productsResult: Result<List<Product>>) : ProductsResult()
Expand Down
2 changes: 1 addition & 1 deletion ...lin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsDataSource.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,5 +105,5 @@ sealed class FetchResult {
}

private fun List<ProductVariation>.applyFilter(): List<ProductVariation> {
return filter { it.price != null && !it.isDownloadable }
return filter { !it.isDownloadable }
}
71 changes: 49 additions & 22 deletions ...merce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,26 +151,28 @@ private fun TotalsLoaded(
horizontalAlignment = Alignment.CenterHorizontally,
verticalArrangement = Arrangement.Center,
) {
Column(
modifier = Modifier
.fillMaxWidth()
.weight(1.1f)
.background(WooPosTheme.colors.totalsErrorBackground),
horizontalAlignment = Alignment.CenterHorizontally,
verticalArrangement = Arrangement.Center,
) {
when (val readerStatus = state.readerStatus) {
is WooPosTotalsViewState.ReaderStatus.Disconnected -> {
ReaderDisconnected(modifier = Modifier, status = readerStatus, onUIEvent = onUIEvent)
}

is WooPosTotalsViewState.ReaderStatus.Preparing,
is WooPosTotalsViewState.ReaderStatus.CheckingOrder -> {
PreparingReader(readerStatus)
}

is WooPosTotalsViewState.ReaderStatus.ReadyForPayment -> {
ReaderReadyForPayment(readerStatus)
if (!state.isFreeOrder) {
Column(
modifier = Modifier
.fillMaxWidth()
.weight(1.1f)
.background(WooPosTheme.colors.totalsErrorBackground),
horizontalAlignment = Alignment.CenterHorizontally,
verticalArrangement = Arrangement.Center,
) {
when (val readerStatus = state.readerStatus) {
is WooPosTotalsViewState.ReaderStatus.Disconnected -> {
ReaderDisconnected(modifier = Modifier, status = readerStatus, onUIEvent = onUIEvent)
}

is WooPosTotalsViewState.ReaderStatus.Preparing,
is WooPosTotalsViewState.ReaderStatus.CheckingOrder -> {
PreparingReader(readerStatus)
}

is WooPosTotalsViewState.ReaderStatus.ReadyForPayment -> {
ReaderReadyForPayment(readerStatus)
}
}
}
}
Expand Down Expand Up @@ -418,6 +420,7 @@ fun WooPosTotalsScreenPreview(modifier: Modifier = Modifier) {
title = "Ready for payment",
subtitle = "Tap, swipe or insert card"
),
isFreeOrder = false
),
onUIEvent = {},
)
Expand All @@ -438,7 +441,8 @@ fun WooPosTotalsScreenPreviewReaderNotConnected(modifier: Modifier = Modifier) {
title = "Reader not connected",
subtitle = "To process this payment, please connect your reader.",
actionButtonLabel = "Connect to a reader",
)
),
isFreeOrder = false
),
onUIEvent = {},
)
Expand All @@ -459,7 +463,30 @@ fun WooPosTotalsScreenPreviewWithCashPaymentAvailable() {
title = "Reader not connected",
subtitle = "To process this payment, please connect your reader.",
actionButtonLabel = "Connect to a reader",
)
),
isFreeOrder = false
),
onUIEvent = {},
)
}
}

@Composable
@WooPosPreview
fun WooPosTotalsScreenPreviewForFreeOrders() {
WooPosTheme {
WooPosTotalsScreen(
modifier = Modifier,
state = WooPosTotalsViewState.Totals(
orderSubtotalText = "$420.00",
orderTotalText = "$462.00",
orderTaxText = "$42.00",
readerStatus = WooPosTotalsViewState.ReaderStatus.Disconnected(
title = "Reader not connected",
subtitle = "To process this payment, please connect your reader.",
actionButtonLabel = "Connect to a reader",
),
isFreeOrder = true
),
onUIEvent = {},
)
Expand Down
7 changes: 6 additions & 1 deletion ...ce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ class WooPosTotalsViewModel @Inject constructor(
uiState.value = state.copy(readerStatus = buildTotalsReaderNotConnectedError())
cancelPaymentAction()
}

is Connected -> {
val state = uiState.value
if (state !is WooPosTotalsViewState.Totals) return@collect
Expand Down Expand Up @@ -254,7 +255,10 @@ class WooPosTotalsViewModel @Inject constructor(
} else {
val orderId = dataState.value.orderId
check(orderId != EMPTY_ORDER_ID)
if (cardReaderFacade.readerStatus.value is Connected) {
if (
cardReaderFacade.readerStatus.value is Connected &&
dataState.value.orderTotal?.compareTo(BigDecimal.ZERO) == 1
) {
val state = uiState.value
check(state is WooPosTotalsViewState.Totals)
check(uiState.value is WooPosTotalsViewState.Totals)
Expand Down Expand Up @@ -464,6 +468,7 @@ class WooPosTotalsViewModel @Inject constructor(
orderTaxText = priceFormat(taxAmount),
orderTotalText = priceFormat(totalAmount),
readerStatus = readerStatus,
isFreeOrder = totalAmount.compareTo(BigDecimal.ZERO) == 0
)
}

Expand Down
1 change: 1 addition & 0 deletions ...ce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewState.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ sealed class WooPosTotalsViewState : Parcelable {
val orderTaxText: String,
val orderTotalText: String,
val readerStatus: ReaderStatus,
val isFreeOrder: Boolean,
) : WooPosTotalsViewState()

data class PaymentSuccess(val orderTotalText: String) : WooPosTotalsViewState()
Expand Down
36 changes: 0 additions & 36 deletions .../test/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosProductsDataSourceTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,42 +297,6 @@ class WooPosProductsDataSourceTest {
assertFalse(cachedResult.products.any { it.remoteId == 1L })
}

@Test
fun `given remote products, when loadSimpleProducts called, then filter in only products that has price`() =
runTest {
// GIVEN
whenever(handler.canLoadMore).thenReturn(AtomicBoolean(true))
whenever(handler.productsFlow).thenReturn(
flowOf(
listOf(
ProductTestUtils.generateProduct(
productId = 1,
productName = "Product 1",
amount = "",
productType = "simple",
isDownloadable = false,
),
ProductTestUtils.generateProduct(
productId = 2,
productName = "Product 2",
amount = "20.0",
productType = "simple",
isDownloadable = false
).copy(firstImageUrl = "https://test.com")
)
)
)
whenever(handler.loadFromCacheAndFetch(any(), any(), any(), any(), any())).thenReturn(Result.success(Unit))
val sut = WooPosProductsDataSource(handler)

// WHEN
val flow = sut.loadSimpleProducts(forceRefreshProducts = true).toList()

// THEN
val remoteResult = flow[1] as WooPosProductsDataSource.ProductsResult.Remote
assertThat(remoteResult.productsResult.getOrNull()?.any { it.remoteId == 1L }).isFalse()
}

@Test
fun `given cached products, when loadSimpleProducts called, then filter out downloadable products`() =
runTest {
Expand Down
117 changes: 117 additions & 0 deletions ...rc/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import org.wordpress.android.fluxc.store.WooCommerceStore
import java.math.BigDecimal
import java.util.Date
import kotlin.test.Test
import kotlin.test.assertFalse

@ExperimentalCoroutinesApi
class WooPosTotalsViewModelTest {
Expand Down Expand Up @@ -1125,6 +1126,122 @@ class WooPosTotalsViewModelTest {
assertThat(successState.orderTotalText).isEqualTo("Paid 5.00$ in Cash")
}

@Test
fun `given checkout started and order contains only free products, when vm created, then totals state correctly calculated`() =
runTest {
// GIVEN
whenever(resourceProvider.getString(R.string.woopos_totals_reader_getting_ready))
.thenReturn("Getting ready")
whenever(resourceProvider.getString(R.string.woopos_totals_reader_checking_order))
.thenReturn("Checking order")
whenever(resourceProvider.getString(R.string.woopos_no_internet_message))
.thenReturn("No internet")
val itemClickedData = listOf(
WooPosItemsViewModel.ItemClickedData.SimpleProduct(id = 1L)
)
val parentToChildrenEventFlow = MutableStateFlow(ParentToChildrenEvent.CheckoutClicked(itemClickedData))
val parentToChildrenEventReceiver: WooPosParentToChildrenEventReceiver = mock {
on { events }.thenReturn(parentToChildrenEventFlow)
}
val order = Order.getEmptyOrder(
dateCreated = Date(),
dateModified = Date()
).copy(
totalTax = BigDecimal("0.00"),
items = listOf(
Order.Item.EMPTY.copy(
subtotal = BigDecimal("0.00"),
),
Order.Item.EMPTY.copy(
subtotal = BigDecimal("0.00"),
),
Order.Item.EMPTY.copy(
subtotal = BigDecimal("0.00"),
)
),
total = BigDecimal("0.00"),
productsTotal = BigDecimal("0.00"),
)
val totalsRepository: WooPosTotalsRepository = mock {
onBlocking { createOrderWithProducts(itemClickedData) }.thenReturn(
Result.success(order)
)
}
val priceFormat: WooPosFormatPrice = mock {
onBlocking { invoke(BigDecimal("0.00")) }.thenReturn("0.00$")
}

// WHEN
val viewModel = createViewModel(
parentToChildrenEventReceiver = parentToChildrenEventReceiver,
totalsRepository = totalsRepository,
priceFormat = priceFormat,
)

// THEN
val totals = viewModel.state.value as WooPosTotalsViewState.Totals
assertTrue(totals.isFreeOrder)
}

@Test
fun `given checkout started and order contains non-free products, when vm created, then totals state correctly calculated`() =
runTest {
// GIVEN
whenever(resourceProvider.getString(R.string.woopos_totals_reader_getting_ready))
.thenReturn("Getting ready")
whenever(resourceProvider.getString(R.string.woopos_totals_reader_checking_order))
.thenReturn("Checking order")
whenever(resourceProvider.getString(R.string.woopos_no_internet_message))
.thenReturn("No internet")
val itemClickedData = listOf(
WooPosItemsViewModel.ItemClickedData.SimpleProduct(id = 1L)
)
val parentToChildrenEventFlow = MutableStateFlow(ParentToChildrenEvent.CheckoutClicked(itemClickedData))
val parentToChildrenEventReceiver: WooPosParentToChildrenEventReceiver = mock {
on { events }.thenReturn(parentToChildrenEventFlow)
}
val order = Order.getEmptyOrder(
dateCreated = Date(),
dateModified = Date()
).copy(
totalTax = BigDecimal("2.00"),
items = listOf(
Order.Item.EMPTY.copy(
subtotal = BigDecimal("1.00"),
),
Order.Item.EMPTY.copy(
subtotal = BigDecimal("1.00"),
),
Order.Item.EMPTY.copy(
subtotal = BigDecimal("1.00"),
)
),
total = BigDecimal("5.00"),
productsTotal = BigDecimal("3.00"),
)
val totalsRepository: WooPosTotalsRepository = mock {
onBlocking { createOrderWithProducts(itemClickedData) }.thenReturn(
Result.success(order)
)
}
val priceFormat: WooPosFormatPrice = mock {
onBlocking { invoke(BigDecimal("2.00")) }.thenReturn("2.00$")
onBlocking { invoke(BigDecimal("3.00")) }.thenReturn("3.00$")
onBlocking { invoke(BigDecimal("5.00")) }.thenReturn("5.00$")
}

// WHEN
val viewModel = createViewModel(
parentToChildrenEventReceiver = parentToChildrenEventReceiver,
totalsRepository = totalsRepository,
priceFormat = priceFormat,
)

// THEN
val totals = viewModel.state.value as WooPosTotalsViewState.Totals
assertFalse(totals.isFreeOrder)
}

private fun createNonEmptyOrder() = Order.getEmptyOrder(
dateCreated = Date(),
dateModified = Date()
Expand Down
Loading

0 comments on commit 07de8ee

Please sign in to comment.