Skip to content

Commit b8da934

Browse files
authored
Merge pull request #14981 from woocommerce/woomob-1692-woo-poslocal-catalog-update-and-remove-products-from-db-in
[Woo POS][Local Catalog] When reconciling items in cart, delete products from Local Catalog DB as well
2 parents 20db21d + ebfd0f0 commit b8da934

File tree

4 files changed

+126
-5
lines changed

4 files changed

+126
-5
lines changed

RELEASE-NOTES.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
23.8
66
-----
7-
7+
- [Internal][Woo POS] Improve detection and handling of deleted products during checkout [https://github.com/woocommerce/woocommerce-android/pull/14981]
88

99
23.7
1010
-----

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartItemsUpdater.kt

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
package com.woocommerce.android.ui.woopos.home.cart
22

33
import com.automattic.android.tracks.crashlogging.CrashLogging
4+
import com.woocommerce.android.tools.SelectedSite
45
import com.woocommerce.android.ui.woopos.common.data.WooPosProductsCache
56
import com.woocommerce.android.ui.woopos.common.data.models.WooPosProductModel
67
import com.woocommerce.android.ui.woopos.common.util.WooPosLogWrapper
78
import com.woocommerce.android.ui.woopos.home.ParentToChildrenEvent
89
import com.woocommerce.android.ui.woopos.home.cart.WooPosCartItemViewState.Coupon.CouponValidationState
910
import com.woocommerce.android.ui.woopos.util.format.WooPosFormatPrice
11+
import org.wordpress.android.fluxc.model.LocalOrRemoteId
12+
import org.wordpress.android.fluxc.store.pos.localcatalog.WooPosLocalCatalogStore
1013
import javax.inject.Inject
1114

1215
class WooPosCartItemsUpdater @Inject constructor(
1316
private val formatPrice: WooPosFormatPrice,
1417
private val productsCache: WooPosProductsCache,
18+
private val localCatalogStore: WooPosLocalCatalogStore,
19+
private val selectedSite: SelectedSite,
1520
private val wooPosLogWrapper: WooPosLogWrapper,
1621
private val crashLogger: CrashLogging,
1722
) {
@@ -84,7 +89,7 @@ class WooPosCartItemsUpdater @Inject constructor(
8489
newItem.productDoesNotExist != item.productDoesNotExist
8590

8691
if (itemChanged) {
87-
deleteProductFromCache(newItem.id)
92+
deleteProductFromCache(newItem)
8893
}
8994

9095
changed = itemChanged
@@ -134,8 +139,32 @@ class WooPosCartItemsUpdater @Inject constructor(
134139
}
135140
}
136141

137-
private suspend fun deleteProductFromCache(productId: Long) {
138-
productsCache.deleteProduct(productId)
142+
private suspend fun deleteProductFromCache(item: WooPosCartItemViewState.Product) {
143+
when (item) {
144+
is WooPosCartItemViewState.Product.Simple -> {
145+
productsCache.deleteProduct(item.id)
146+
selectedSite.getOrNull()?.let { site ->
147+
val siteId = LocalOrRemoteId.LocalId(site.id)
148+
val remoteId = LocalOrRemoteId.RemoteId(item.id)
149+
localCatalogStore.deleteProducts(siteId, listOf(remoteId))
150+
.onFailure { error ->
151+
wooPosLogWrapper.e("Failed to delete product from Local Catalog DB: ${error.message}")
152+
}
153+
}
154+
}
155+
is WooPosCartItemViewState.Product.Variation -> {
156+
productsCache.deleteProduct(item.id)
157+
selectedSite.getOrNull()?.let { site ->
158+
val siteId = LocalOrRemoteId.LocalId(site.id)
159+
val productId = LocalOrRemoteId.RemoteId(item.id)
160+
val variationId = LocalOrRemoteId.RemoteId(item.variationId)
161+
localCatalogStore.deleteVariations(siteId, listOf(productId to variationId))
162+
.onFailure { error ->
163+
wooPosLogWrapper.e("Failed to delete variation from Local Catalog DB: ${error.message}")
164+
}
165+
}
166+
}
167+
}
139168
}
140169

141170
private fun getProductKey(item: WooPosCartItemViewState.Product): String {

WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartItemsUpdaterTest.kt

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package com.woocommerce.android.ui.woopos.home.cart
22

33
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
44
import com.automattic.android.tracks.crashlogging.CrashLogging
5+
import com.woocommerce.android.tools.SelectedSite
56
import com.woocommerce.android.ui.woopos.common.data.WooPosProductsCache
67
import com.woocommerce.android.ui.woopos.common.data.models.WooPosProductModel
78
import com.woocommerce.android.ui.woopos.common.util.WooPosLogWrapper
@@ -21,6 +22,8 @@ import org.mockito.kotlin.mock
2122
import org.mockito.kotlin.never
2223
import org.mockito.kotlin.verify
2324
import org.mockito.kotlin.whenever
25+
import org.wordpress.android.fluxc.model.SiteModel
26+
import org.wordpress.android.fluxc.store.pos.localcatalog.WooPosLocalCatalogStore
2427
import java.math.BigDecimal
2528
import kotlin.test.Test
2629

@@ -39,15 +42,23 @@ class WooPosCartItemsUpdaterTest {
3942
onBlocking { invoke(argThat { this == BigDecimal("5.0") }) }.thenReturn("5.0$")
4043
}
4144
private val productsCache: WooPosProductsCache = mock()
45+
private val localCatalogStore: WooPosLocalCatalogStore = mock {
46+
onBlocking { deleteProducts(any(), any()) }.thenReturn(Result.success(Unit))
47+
onBlocking { deleteVariations(any(), any()) }.thenReturn(Result.success(Unit))
48+
}
49+
private val selectedSite: SelectedSite = mock {
50+
onBlocking { getOrNull() }.thenReturn(SiteModel().apply { id = 1 })
51+
}
4252
private val crashLogger: CrashLogging = mock()
4353
private val logger: WooPosLogWrapper = mock()
4454

4555
private val updater = WooPosCartItemsUpdater(
4656
formatPrice = formatPrice,
4757
productsCache = productsCache,
58+
localCatalogStore = localCatalogStore,
59+
selectedSite = selectedSite,
4860
wooPosLogWrapper = logger,
4961
crashLogger = crashLogger,
50-
5162
)
5263

5364
@Test
@@ -303,6 +314,62 @@ class WooPosCartItemsUpdaterTest {
303314
verify(productsCache).deleteProduct(1L)
304315
}
305316

317+
@Test
318+
fun `given simple product not existing, when deleted, then also deletes from Local Catalog DB`() = runTest {
319+
// GIVEN
320+
val simpleProduct = WooPosCartItemViewState.Product.Simple(
321+
itemNumber = 1,
322+
id = 1L,
323+
name = "Product Name",
324+
price = "9.0$",
325+
imageUrl = "url",
326+
description = null
327+
)
328+
val itemsInCart = listOf(simpleProduct)
329+
330+
// WHEN
331+
updater.invoke(itemsInCart, emptyList(), emptyList())
332+
333+
// THEN
334+
verify(productsCache).deleteProduct(1L)
335+
verify(localCatalogStore).deleteProducts(
336+
argThat { siteId -> siteId.value == 1 },
337+
argThat { productIds ->
338+
productIds.size == 1 && productIds[0].value == 1L
339+
}
340+
)
341+
}
342+
343+
@Test
344+
fun `given variation product not existing, when deleted, then deletes only variation from Local Catalog DB`() =
345+
runTest {
346+
// GIVEN
347+
val variationProduct = WooPosCartItemViewState.Product.Variation(
348+
itemNumber = 1,
349+
id = 1L,
350+
variationId = 2L,
351+
name = "Variation Name",
352+
price = "9.0$",
353+
imageUrl = "url",
354+
description = null
355+
)
356+
val itemsInCart = listOf(variationProduct)
357+
358+
// WHEN
359+
updater.invoke(itemsInCart, emptyList(), emptyList())
360+
361+
// THEN
362+
verify(productsCache).deleteProduct(1L)
363+
verify(localCatalogStore).deleteVariations(
364+
argThat { siteId -> siteId.value == 1 },
365+
argThat { variations ->
366+
variations.size == 1 &&
367+
variations[0].first.value == 1L &&
368+
variations[0].second.value == 2L
369+
}
370+
)
371+
}
372+
306373
@Test
307374
fun `given product updated, when called, then cache is updated correctly`() = runTest {
308375
// GIVEN

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/pos/localcatalog/WooPosLocalCatalogStore.kt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,31 @@ class WooPosLocalCatalogStore @Inject constructor(
275275
): Result<Unit> =
276276
runCatching { posProductDao.deleteAllProductsForSite(siteId) }
277277

278+
suspend fun deleteProducts(
279+
siteId: LocalOrRemoteId.LocalId,
280+
productIds: List<LocalOrRemoteId.RemoteId>
281+
): Result<Unit> =
282+
runCatching {
283+
database.executeInTransaction {
284+
productIds.forEach { remoteId ->
285+
posProductDao.deleteProduct(siteId, remoteId)
286+
posVariationsDao.deleteVariationsForProduct(siteId, remoteId)
287+
}
288+
}
289+
}
290+
291+
suspend fun deleteVariations(
292+
siteId: LocalOrRemoteId.LocalId,
293+
variations: List<Pair<LocalOrRemoteId.RemoteId, LocalOrRemoteId.RemoteId>>
294+
): Result<Unit> =
295+
runCatching {
296+
database.executeInTransaction {
297+
variations.forEach { (productId, variationId) ->
298+
posVariationsDao.deleteVariation(siteId, productId, variationId)
299+
}
300+
}
301+
}
302+
278303
suspend fun upsertVariations(variations: List<WooPosVariationEntity>): Result<Unit> =
279304
runCatching { posVariationsDao.upsertVariations(variations) }
280305

0 commit comments

Comments
 (0)