Skip to content

Conversation

@AnirudhBhat
Copy link
Contributor

@AnirudhBhat AnirudhBhat commented May 26, 2023

Closes: #9126

Description

This PR tracks the barcode format when the product search via SKU fails. This information gives us which barcode format is failing the most. Do we need to handle the checksum for that format ...etc

More context: p1685078975281749-slack-C025A8VV728

Testing instructions

  1. Navigate to the order creation screen (Orders tab -> "+")
  2. Tap on the barcode icon under the products section
  3. Scan any invalid SKU with a different barcode format from the online barcode generator website
  4. On product search via SKU failure, ensure product_search_via_sku_failure event is tracked
  5. Ensure the barcode_format property is proper.
  6. Repeat step 3 to 5 with different barcode formats.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@AnirudhBhat AnirudhBhat requested a review from kidinov May 26, 2023 09:05
@AnirudhBhat AnirudhBhat added feature: mobile payments Related to mobile payments / card present payments / Woo Payments. feature: analytics In-app store analytics labels May 26, 2023
@AnirudhBhat AnirudhBhat added this to the 13.8 milestone May 26, 2023
@peril-woocommerce
Copy link

peril-woocommerce bot commented May 26, 2023

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 AnirudhBhat marked this pull request as ready for review May 26, 2023 09:06
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented May 26, 2023

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

@kidinov kidinov self-assigned this May 26, 2023
@codecov-commenter
Copy link

codecov-commenter commented May 26, 2023

Codecov Report

Patch coverage: 93.15% and project coverage change: +0.06 🎉

Comparison is base (1cfe40c) 43.81% compared to head (7ce2146) 43.88%.

❗ Current head 7ce2146 differs from pull request most recent head 0429ad9. Consider uploading reports for the commit 0429ad9 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##              trunk    #9129      +/-   ##
============================================
+ Coverage     43.81%   43.88%   +0.06%     
- Complexity     4154     4169      +15     
============================================
  Files           843      844       +1     
  Lines         44514    44576      +62     
  Branches       5827     5827              
============================================
+ Hits          19505    19563      +58     
- Misses        23312    23314       +2     
- Partials       1697     1699       +2     
Impacted Files Coverage Δ
.../woocommerce/android/analytics/AnalyticsTracker.kt 20.76% <ø> (ø)
...in/com/woocommerce/android/ui/main/MainActivity.kt 0.00% <ø> (ø)
...om/woocommerce/android/ui/orders/OrderNavigator.kt 0.00% <0.00%> (ø)
...oid/ui/orders/creation/OrderCreateEditViewModel.kt 89.21% <90.00%> (-0.10%) ⬇️
...id/ui/orders/creation/GoogleBarcodeFormatMapper.kt 95.65% <95.65%> (ø)
...commerce/android/ui/orders/creation/CodeScanner.kt 97.10% <100.00%> (+0.17%) ⬆️
...merce/android/ui/orders/list/OrderListViewModel.kt 69.45% <100.00%> (+0.44%) ⬆️

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

@Parcelize
object FormatCodaBar : BarcodeFormat("codabar")
@Parcelize
object FormatCode128 : BarcodeFormat("code 128")
Copy link
Contributor

Choose a reason for hiding this comment

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

np: maybe to use code_128 etc as this is for tracking and we usually use this notation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done here: 0429ad9

android:name="sku"
app:argType="string"
app:nullable="true"/>
<argument
Copy link
Contributor

@kidinov kidinov May 26, 2023

Choose a reason for hiding this comment

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

np: you can consider putting Sku and BarcodeFormat in one structure, so there will be a compile time check that it should be either both SKU and BarcodeFormat or none of them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. I'll take this in a separate PR just so that we can merge and get the feature out to merchants.

@kidinov kidinov self-requested a review May 26, 2023 10:46
Copy link
Contributor

@kidinov kidinov left a comment

Choose a reason for hiding this comment

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

LGTM!

Tracked: product_search_via_sku_failure, Properties: {"source":"order_list","barcode_format":"qr code","reason":"Empty data response (no product found for the SKU)","blog_id":192152755,"is_wpcom_store":true,"is_debug":true}

@AnirudhBhat AnirudhBhat enabled auto-merge May 26, 2023 10:48
@AnirudhBhat AnirudhBhat merged commit 0b93fea into trunk May 26, 2023
@AnirudhBhat AnirudhBhat deleted the issue/9126-track-barcode-format branch May 26, 2023 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: analytics In-app store analytics feature: mobile payments Related to mobile payments / card present payments / Woo Payments.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add barcode format to the product search via SKU failure event

5 participants