Increment product quantity when the same product is scanned more than one time #9083

AnirudhBhat · 2023-05-22T10:42:21Z

Closes: #9081

Description

Before this PR
When you scan the same product more than once, the same product was being added to the list in a separate row instead of just incrementing the already existing product count.

After this PR
When you scan the same product more than once, the product quantity gets incremented as expected.

Testing instructions

Navigate to the Order details screen (orders -> "+")
Click on the barcode icon present under the products section
Scan the barcode (Make sure to generate the barcode with an valid SKU present in your store - example: https://barcode.tec-it.com/en/?data=AC-1)
Ensure the product gets added
Scan the same barcode again
Ensure the already added product's quantity gets incremented and no new product gets added.

Images/gif

Please ignore the scanner not displaying the camera session. It's something to do with Google play services. It's messed up on my device, again.

screen-20230522-174950.2.mp4

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

…ct is scanned more than once.

…en same product is scanned more than once.

…ue/9081-increment-product-quantity # Conflicts: # WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/creation/CreationFocusedOrderCreateEditViewModelTest.kt

wpmobilebot · 2023-05-22T11:11:06Z

You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code:

codecov-commenter · 2023-05-22T11:18:37Z

Codecov Report

Patch coverage: 90.47% and project coverage change: -0.06 ⚠️

Comparison is base (b6dc535) 43.59% compared to head (42c121f) 43.54%.

❗ Current head 42c121f differs from pull request most recent head ff6c60d. Consider uploading reports for the commit ff6c60d to get more accurate results

Additional details and impacted files

@@                                 Coverage Diff                                 @@
##             issue/9073-handle-error-case-barcode-scanning    #9083      +/-   ##
===================================================================================
- Coverage                                            43.59%   43.54%   -0.06%     
+ Complexity                                            4094     4041      -53     
===================================================================================
  Files                                                  834      833       -1     
  Lines                                                43985    43941      -44     
  Branches                                              5763     5741      -22     
===================================================================================
- Hits                                                 19176    19133      -43     
+ Misses                                               23137    23134       -3     
- Partials                                              1672     1674       +2

Impacted Files	Coverage Δ
...oid/ui/orders/creation/OrderCreateEditViewModel.kt	`88.02% <90.00%> (+1.02%)`	⬆️
...com/woocommerce/android/ui/products/ProductType.kt	`76.19% <100.00%> (ø)`

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

…sponse from the product search using SKU returns type as "variation" for variable product.

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/ProductType.kt

kidinov · 2023-05-23T08:00:57Z

...merce/src/main/kotlin/com/woocommerce/android/ui/orders/creation/OrderCreateEditViewModel.kt

        }
    }

+    private fun getItemIdIfVariableProductIsAlreadySelected(product: com.woocommerce.android.model.Product): Long? {


Np: you may consider using

import com.woocommerce.android.model.Product as ModelProduct

To make it a bit more readable

Done here ff6c60d

kidinov · 2023-05-23T08:12:50Z

...merce/src/main/kotlin/com/woocommerce/android/ui/orders/creation/OrderCreateEditViewModel.kt

-                            selectedItems + SelectedItem.ProductVariation(
-                                productId = product.parentId,
-                                variationId = product.remoteId
+                        getItemIdIfVariableProductIsAlreadySelected(product)?.let { itemId ->


np: It took me some time to figure out this code. Maybe to put these a bit more streightforward:

when (val alreadySelectedItemId = getItemIdIfVariableProductIsAlreadySelected(product)) { null -> onProductsSelected(selectedItems + SelectedItem.ProductVariation( productId = product.parentId, variationId = product.remoteId ) ) else -> onIncreaseProductsQuantity(alreadySelectedItemId) }

Done here ff6c60d

kidinov · 2023-05-23T08:42:19Z

I notice that I can not add variable products via scanning. No error is shown. The issue comes from this code from my quick debugging

                val itemsToAdd = selectedItems.filter { selectedItem ->
                    if (selectedItem is SelectedItem.ProductVariation) {
                        none { it.variationId == selectedItem.variationId }
                    } else {
                        none { it.productId == selectedItem.id }
                    }

(check SKU: AC)

05-23--12-39.mp4

AnirudhBhat · 2023-05-23T11:08:50Z

Thanks for the review @kidinov

I notice that I can not add variable products via scanning

Hmm. I believe we cannot add the parent variable product as is right? We should select the specific variant of the product. Example: AC-1, AC-2.

Even with adding the products manually, we should select the specific variant and cannot add the parent variable product itself. Right?

…instead add a separate variation type.

…ue/9081-increment-product-quantity # Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/creation/OrderCreateEditViewModel.kt

kidinov · 2023-05-24T07:15:19Z

@AnirudhBhat

Hmm. I believe we cannot add the parent variable product as is right? We should select the specific variant of the product. Example: AC-1, AC-2.

Even with adding the products manually, we should select the specific variant and cannot add the parent variable product itself. Right?

It sounds about right. But still - this case is swollen, while we have all the info to handle that. We may want to show a message/snack bar that explains that the parent product can not be added to the order

peril-woocommerce · 2023-05-24T07:23:53Z

	Warnings
⚠️	This PR is assigned to a milestone which is closing in less than 2 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

AnirudhBhat · 2023-05-24T07:28:34Z

Thanks for the review @kidinov

It sounds about right. But still - this case is swollen, while we have all the info to handle that. We may want to show a message/snack bar that explains that the parent product can not be added to the order

I agree. Implemented here: ff6c60d

kidinov · 2023-05-24T07:41:54Z

...merce/src/main/kotlin/com/woocommerce/android/ui/orders/creation/OrderCreateEditViewModel.kt

+    }
+
+    private fun sendAddingProductsViaScanningFailedEvent(
+        @StringRes message: Int = string.order_creation_barcode_scanning_unable_to_add_product


Probably in the parent PR, you will use different strings for 2 different error handling, so I guess having a default value here doesn't make any good?

Also, np: we usually use R.string import

Yes. Done here ed8b7e8

kidinov · 2023-05-24T07:43:39Z

.../src/test/kotlin/com/woocommerce/android/ui/orders/creation/UnifiedOrderEditViewModelTest.kt

+                ?.takeIf { it.isNotEmpty() }
+                ?.find { it.productId == 10L }
+                ?.let { assertThat(it.quantity).isEqualTo(3f) }
+                ?: Assertions.fail("Expected an item with productId 10L with quantity as 3")


np: usually in test we use static imports for assertions

Done here ed8b7e8

kidinov · 2023-05-24T07:54:14Z

Not related to the PR, but we make the same architecture mistake here as with IPP/TTP when we tight application-scoped things to VM scope. Try to enable don't keep activity and see that it all falls apart. We are not subscribed to the emitter, and instead of using StateFlow, we use stateless flow so the result is lost

Maybe create a ticket and think if and how it should be addressed

05-24--11-52.mp4

kidinov · 2023-05-24T07:59:00Z

Is this expected that the snackbar stays forever?

05-24--11-57.mp4

kidinov

The PR looks good to me. I left a couple of non blocking comments, and a couple of not related to the PR but related to the feature itself

…ue/9081-increment-product-quantity # Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/creation/OrderCreateEditViewModel.kt # WooCommerce/src/main/res/values/strings.xml

AnirudhBhat · 2023-05-24T08:24:39Z

Not related to the PR, but we make the same architecture mistake here as with IPP/TTP when we tight application-scoped things to VM scope. Try to enable don't keep activity and see that it all falls apart. We are not subscribed to the emitter, and instead of using StateFlow, we use stateless flow so the result is lost

Yup. This is a known issue and I'll take care of this as soon as the first release is out.

These are the things that I'll be taking care of after the first release

Let me know if I've missed anything here.

AnirudhBhat · 2023-05-24T08:25:18Z

Is this expected that the snackbar stays forever?

Yes. In order to dismiss it. You need to swipe right.

kidinov · 2023-05-24T08:33:17Z

@AnirudhBhat

Yes. In order to dismiss it. You need to swipe right.

Hm, feels not really Android way of showing errors then. I'd vote for using dismissive snackbars for this purpose then

AnirudhBhat added 6 commits May 22, 2023 11:49

Implement the logic to increment the product quantity when same produ…

67b5f24

…ct is scanned more than once.

Implement the logic to increment the non-variable product quantity wh…

1e9c3e0

…en same product is scanned more than once.

Refactor tests

c459ce1

Refactor product quantity increment logic after fetching product by SKU.

8f519ad

Merge branch 'issue/9073-handle-error-case-barcode-scanning' into iss…

261cc7d

…ue/9081-increment-product-quantity # Conflicts: # WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/creation/CreationFocusedOrderCreateEditViewModelTest.kt

Fix indentation.

832a31f

Consider "variation" type as variable product as well. The Product re…

42c121f

…sponse from the product search using SKU returns type as "variation" for variable product.

AnirudhBhat requested a review from kidinov May 22, 2023 12:42

AnirudhBhat added the feature: mobile payments Related to mobile payments / card present payments / Woo Payments. label May 22, 2023

AnirudhBhat added this to the 13.8 milestone May 22, 2023

AnirudhBhat marked this pull request as ready for review May 22, 2023 12:42

kidinov self-assigned this May 23, 2023

kidinov reviewed May 23, 2023

View reviewed changes

AnirudhBhat added 4 commits May 24, 2023 10:32

Remove the "variation" mapping to the VARIABLE in ProductType.kt and …

1fde175

…instead add a separate variation type.

Use import alias for making code readable.

fecd0c0

Code refactor

9bf16b9

Merge branch 'issue/9073-handle-error-case-barcode-scanning' into iss…

0827912

…ue/9081-increment-product-quantity # Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/creation/OrderCreateEditViewModel.kt

Display an error message when we try to add a parent variable product.

ff6c60d

kidinov reviewed May 24, 2023

View reviewed changes

kidinov self-requested a review May 24, 2023 07:54

kidinov approved these changes May 24, 2023

View reviewed changes

AnirudhBhat added 2 commits May 24, 2023 13:39

Merge branch 'issue/9073-handle-error-case-barcode-scanning' into iss…

7fb0372

…ue/9081-increment-product-quantity # Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/creation/OrderCreateEditViewModel.kt # WooCommerce/src/main/res/values/strings.xml

Refactor code

ed8b7e8

Base automatically changed from issue/9073-handle-error-case-barcode-scanning to trunk May 24, 2023 08:32

AnirudhBhat enabled auto-merge May 24, 2023 08:33

AnirudhBhat merged commit 4b62b61 into trunk May 24, 2023

AnirudhBhat deleted the issue/9081-increment-product-quantity branch May 24, 2023 08:42

AnirudhBhat mentioned this pull request May 25, 2023

Scanned and deliver final PR to turn off the feature flag #9125

Merged

1 task

Increment product quantity when the same product is scanned more than one time #9083

Increment product quantity when the same product is scanned more than one time #9083

Uh oh!

Conversation

AnirudhBhat commented May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing instructions

Images/gif

Uh oh!

wpmobilebot commented May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kidinov commented May 23, 2023

Uh oh!

AnirudhBhat commented May 23, 2023

Uh oh!

kidinov commented May 24, 2023

Uh oh!

peril-woocommerce bot commented May 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AnirudhBhat commented May 24, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kidinov commented May 24, 2023

Uh oh!

kidinov commented May 24, 2023

Uh oh!

kidinov left a comment

Choose a reason for hiding this comment

Uh oh!

AnirudhBhat commented May 24, 2023

Uh oh!

AnirudhBhat commented May 24, 2023

Uh oh!

kidinov commented May 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AnirudhBhat commented May 22, 2023 •

edited

Loading

wpmobilebot commented May 22, 2023 •

edited

Loading

codecov-commenter commented May 22, 2023 •

edited

Loading

peril-woocommerce bot commented May 24, 2023 •

edited

Loading