[POS][Local Catalog] WooPosVariation #14564

samiuelson · 2025-09-03T21:33:59Z

Description

This PR introduces POS-specific domain data model for Product Variation to replace direct usage of the ProductVariation.

Steps to reproduce

N/A

Testing information

Ensure that POS works without any visible change

Variations are loading
Variations pagination works
It's possible to add variation to cart and create order

The tests that have been performed

Above

Images/gif

N/A

I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

dangermattic · 2025-09-03T21:34:59Z

	1 Warning
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

wpmobilebot · 2025-09-03T21:39:51Z

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce-Wear Android
	Platform	⌚️ Wear OS
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`4ae74f9`
	Direct Download	`woocommerce-wear-prototype-build-pr14564-4ae74f9.apk`

wpmobilebot · 2025-09-03T21:52:24Z

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce Android
	Platform	📱 Mobile
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`4ae74f9`
	Direct Download	`woocommerce-prototype-build-pr14564-4ae74f9.apk`

…s-specific-variation-model-in

Copilot

Pull Request Overview

This PR introduces a POS-specific domain data model WooPosVariation to replace direct usage of the general ProductVariation model within the WooCommerce POS system. The change provides better performance and cleaner separation of concerns for POS functionality.

Introduces WooPosVariation data class containing only POS-relevant fields
Replaces ProductVariation usage across POS-related classes with WooPosVariation
Moves and adapts the getNameForPOS extension function to work with the new model

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
WooPosVariation.kt	Defines the new POS-specific variation model with conversion functions
WooPosGetVariationById.kt	Updates return type to use `WooPosVariation`
WooPosVariationsViewModel.kt	Removes extension function and updates to use new model
WooPosVariationsDataSource.kt	Converts remote variations to `WooPosVariation` type
WooPosVariationsLRUCache.kt	Updates cache type parameters to store `WooPosVariation`
WooPosCartViewModel.kt	Updates variation handling to use new model
WooPosSearchByIdentifier*.kt	Updates search functionality to work with new variation model
WooPosTotalsRepository.kt	Updates import statements for moved extension function
WooPosSearchByIdentifierLocalTest.kt	Updates test mocks to use actual `WooPosVariation` instances

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt

…s-specific-variation-model-in

malinajirka · 2025-09-05T08:49:01Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt

+    val remoteVariationId: Long,
+    val remoteProductId: Long,
+    val globalUniqueId: String,
+    val price: BigDecimal?,


💡 This is technically enough for now, but we'll likely need to have access to both sale and regular prices quite soon, so I'd consider including them similarly to how we do it in the product model. Having said that, feel free to leave it as is.

I'd leave these props for now and update the model when we need them.

malinajirka · 2025-09-05T08:49:37Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt

+        } else {
+            emptyList()
+        }
+    } catch (e: JsonSyntaxException) {


❓ I'd consider logging an error into the local log.

I realized this is related to the below comment - since we are doing the parsing within the model, not in a separate class, we don't have access to the logger. I'd personally vote for moving the parsing to a mapper so we can log issues and re-use a single instance of gson.

Yes, parsing logic is now in the WooPosVariationMapper.kt file.

malinajirka · 2025-09-05T08:51:19Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt

+    )
+}
+
+private val gson by lazy { Gson() }


⚠️ Having an instance of gson per instance feels a bit off. Have you considered creating a separate mapper class?

I like the idea, done: abba849

malinajirka · 2025-09-05T08:52:18Z

...e/android/ui/woopos/common/data/searchbyidentifier/WooPosSearchByIdentifierVariationFetch.kt

            parentId,
            variationId
-        )?.toAppModel()
+        )?.toAppModel()?.toWooPosVariation()


⚠️ Seems like we are mapping it to appModel and then to POS model - can we skip the first mapping?

Done in abba849

wpmobilebot · 2025-09-05T12:59:16Z

Version 23.2 has now entered code-freeze, so the milestone of this PR has been updated to 23.3.

…s-specific-variation-model-in

codecov-commenter · 2025-09-05T16:10:43Z

Codecov Report

❌ Patch coverage is 27.09677% with 113 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.29%. Comparing base (08931bc) to head (4ae74f9).
⚠️ Report is 9 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...oid/ui/woopos/common/data/WooPosVariationMapper.kt	3.06%	95 Missing ⚠️
...home/items/variations/WooPosVariationsViewModel.kt	57.14%	5 Missing and 1 partial ⚠️
...id/ui/woopos/common/data/WooPosGetVariationById.kt	0.00%	4 Missing ⚠️
...ome/items/variations/WooPosVariationsDataSource.kt	72.72%	1 Missing and 2 partials ⚠️
...e/android/ui/woopos/common/data/WooPosVariation.kt	86.66%	2 Missing ⚠️
...id/ui/woopos/home/totals/WooPosTotalsRepository.kt	33.33%	2 Missing ⚠️
.../home/items/variations/WooPosVariationsLRUCache.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #14564      +/-   ##
============================================
- Coverage     38.31%   38.29%   -0.03%     
- Complexity     9646     9651       +5     
============================================
  Files          2051     2053       +2     
  Lines        114860   114976     +116     
  Branches      15240    15247       +7     
============================================
+ Hits          44011    44029      +18     
- Misses        66817    66916      +99     
+ Partials       4032     4031       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

malinajirka

LGTM 🚢

samiuelson added 3 commits September 3, 2025 23:25

Create POS-specific variation model

5ac8671

Migrate POS to WooPosVariation

24f6350

Satisfy detekt's complaints

f10a079

samiuelson requested a review from Copilot September 3, 2025 21:35

This comment was marked as outdated.

Sign in to view

Remove comment

62474bf

Base automatically changed from woomob-1232-woo-poslocal-catalog-propagate-header-with-total-number-of to trunk September 4, 2025 10:33

samiuelson added 2 commits September 4, 2025 20:44

Handle null attributes

f5af9f9

Update tests

bf1f27a

samiuelson requested a review from Copilot September 4, 2025 19:04

Merge branch 'trunk' into woomob-1244-woo-poslocal-catalog-use-new-po…

4d83ab3

…s-specific-variation-model-in

This comment was marked as outdated.

Sign in to view

samiuelson added the feature: point of sale POS project label Sep 4, 2025

samiuelson modified the milestones: 23.3, 23.2 Sep 4, 2025

samiuelson requested a review from malinajirka September 4, 2025 19:12

samiuelson added 3 commits September 4, 2025 21:16

Clean up code

74ad241

Revert WooPosGetVariationById

5095fb6

Map WCProductVariationModel → WooPosVariation

abc8221

samiuelson marked this pull request as ready for review September 4, 2025 19:37

samiuelson requested a review from Copilot September 4, 2025 19:37

Copilot AI reviewed Sep 4, 2025

View reviewed changes

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt Outdated Show resolved Hide resolved

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosVariation.kt Outdated Show resolved Hide resolved

samiuelson marked this pull request as draft September 5, 2025 05:13

samiuelson added 3 commits September 5, 2025 07:16

Remove unnecessary try-catch

64a0510

Satisfy detekt's complaints

b4997b7

Make Gson instance a singleton

2c8b8fb

samiuelson marked this pull request as ready for review September 5, 2025 05:22

samiuelson added 5 commits September 5, 2025 07:27

Satisfy detekt's complaints

e2dc6d0

Merge branch 'trunk' into woomob-1244-woo-poslocal-catalog-use-new-po…

43257a1

…s-specific-variation-model-in

Fix tests

ed12b16

Fix tests

d12819b

Fix tests

a4c3903

malinajirka reviewed Sep 5, 2025

View reviewed changes

samiuelson added 2 commits September 5, 2025 11:39

Log JsonSyntaxException

cf5cecd

Extract JSON parsing and data mapping to the mapper class

abba849

wpmobilebot modified the milestones: 23.2, 23.3 Sep 5, 2025

samiuelson added 2 commits September 5, 2025 16:53

Update tests

2dc7ce9

Merge branch 'trunk' into woomob-1244-woo-poslocal-catalog-use-new-po…

4ae74f9

…s-specific-variation-model-in

samiuelson requested a review from malinajirka September 5, 2025 15:37

malinajirka approved these changes Sep 8, 2025

View reviewed changes

malinajirka merged commit f8e2fd7 into trunk Sep 8, 2025
18 checks passed

malinajirka deleted the woomob-1244-woo-poslocal-catalog-use-new-pos-specific-variation-model-in branch September 8, 2025 05:36

[POS][Local Catalog] WooPosVariation #14564

[POS][Local Catalog] WooPosVariation #14564

Uh oh!

Conversation

samiuelson commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Steps to reproduce

Testing information

Ensure that POS works without any visible change

The tests that have been performed

Images/gif

Uh oh!

dangermattic commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

wpmobilebot commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

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!

samiuelson Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

wpmobilebot commented Sep 5, 2025

Uh oh!

codecov-commenter commented Sep 5, 2025

Codecov Report

Uh oh!

malinajirka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

samiuelson commented Sep 3, 2025 •

edited

Loading

dangermattic commented Sep 3, 2025 •

edited

Loading

wpmobilebot commented Sep 3, 2025 •

edited

Loading

wpmobilebot commented Sep 3, 2025 •

edited

Loading

samiuelson Sep 5, 2025 •

edited

Loading