[Woo POS] Modularization: Bring existing changes to trunk #16132

staskus · 2025-09-16T11:34:40Z

Description

The goal of this task is to prepare for modularization by making the POS code independent from other Woo target code so it can be moved into a separate module.

Internally, I already tried to create PointOfSale module, moved all the POS files, and kept moving away different dependencies until I determined a list of dependencies that need to be extracted/reused to make a compiling PointOfSale module. However, due to time constraints, it's difficult to make all the refactorings in one go and decision to keep changes only in a feature branch was unproductive.

Therefore, I'm bringing all the changes I can from the woomob-935-woo-pos-hack-week-pos-modularization branch made in these PRs to trunk:

The gist of these changes:

PointOfSale module is not included
Moved reusable utilities, extensions and views to WooFoundation
Moved reusable business logic to Yosemite
Removed ServicesLocator. dependency from POS by creating protocols (POSDependencyProviding) and injecting dependencies trough environment. E.g analytics is now reached via \.posAnalytics environment object rather than ServicesLocator.analytics
Removed more complex view with a lot of dependencies and Woo app dependencies to POSDependencyProviding as well. I needed to use type erasure (AnyView), for example, when abstracting payment onboarding views. That's an acceptable price to pay to avoid further refactorings.

The remaining dependencies are defined in WOOMOB-935.

There may be more dependencies since I last inspected 2 months ago but I think after addressing them we could create PointOfSale module, move all the files within POS folder except Adaptors, and then deal with remaining errors on the go.

Steps to reproduce

To test:

POS Event Tracking
Feature Flags
Payment Onboarding
CI succeeds
Support view

Testing information

iPad Air Simulator and Device

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

- Move VersionHelpers to WooFoundationCore for cross-module reuse - Move String+Helpers to WooFoundationCore for cross-module reuse - Move Array+Helpers to WooFoundationCore for cross-module reuse

- Move View+AutofocusTextModifier to WooFoundation for cross-module reuse - Move View+Conditionals to WooFoundation for cross-module reuse - Move View+Measurements to WooFoundation for cross-module reuse - Move View+RoundedBorder to WooFoundation for cross-module reuse - Move View+ScrollModifiers to WooFoundation for cross-module reuse - Move View+SizeTracker to WooFoundation for cross-module reuse

- Move ScrollableVStack to WooFoundation for cross-module reuse - Move TextFieldStyles to WooFoundation for cross-module reuse - Move IndefiniteCircularProgressViewStyle to WooFoundation for cross-module reuse - Move KeyboardObserver to WooFoundation for cross-module reuse

- Move WooAnalyticsEvent to WooFoundationCore for cross-module reuse - Move WooAnalyticsStat to WooFoundationCore for cross-module reuse - Create WooAnalyticsEvent+WooApp for app-specific extensions

- Move OrderTotalsCalculator to Yosemite for cross-module reuse

- Move ConnectivityObserver protocol to WooFoundation for cross-module reuse - Move DefaultConnectivityObserver to WooFoundation for cross-module reuse - Create POS-specific Error+Connectivity extensions

- Create POSDependencyProviding protocol for dependency injection - Create service adaptors for analytics, currency settings, feature flags, connectivity - Create adaptors for external navigation and view creation - Create main POSServiceLocatorAdaptor implementing all protocols

- Update POS controllers to use injected dependencies - Update POS SwiftUI views to use environment dependencies with ServiceLocator fallbacks - Add POSDependenciesKey environment key for dependency injection - Update POSTabCoordinator to inject POSServiceLocatorAdaptor

- Add UIImage+POS extensions for POS-specific images - Delete unused CardPresentPaymentError file - Update Card Present Payment event handling

- Update ViewRelated files to use WooFoundation and WooFoundationCore imports - Update ViewModels to use new module structure - Update system files with proper imports - Update tools and utilities with module imports

- Update MockPOSAnalytics to implement POSAnalyticsProviding protocol - Update POS controller tests to use proper mock dependencies - Update POS presentation tests with new module imports - Update test mocks with updated imports and protocols

- Update project file references for moved and deleted files - Add new module dependencies and file references - Remove obsolete file references from deleted files

- Move WooAnalyticsEventPropertyType to WooFoundationCore - Update DefaultConnectivityObserver and tests in WooFoundation - Update authentication and POS utility files with proper imports - Complete final import updates across remaining files

…tion-progress-so-far-trunk

dangermattic · 2025-09-16T11:35:17Z

	3 Warnings
⚠️	View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️	This PR is assigned to the milestone 23.3. This milestone is due in less than 2 days. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

	1 Message
📖	This PR contains changes to Tracks-related logic. Please ensure (author and reviewer) the following are completed: The tracks events must be validated in the Tracks system. Verify the internal Tracks spreadsheet has also been updated. Please consider registering any new events. The PR must be assigned the category: tracks label.

Generated by 🚫 Danger

wpmobilebot · 2025-09-16T11:46:26Z

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

	App Name	WooCommerce iOS Prototype
	Build Number	`pr16132-9da0a71`
	Version	`23.2`
	Bundle ID	`com.automattic.alpha.woocommerce`
	Commit	`9da0a71`
	Installation URL	1m8gk107ekm48

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

… files

jaclync · 2025-09-17T01:32:57Z

Re Periphery reset in ac5bbc7:

Could you briefly share how the baseline is reset? It's a bit hard to tell from the diffs which took some time to load.

staskus · 2025-09-17T05:12:40Z

Could you briefly share how the baseline is reset? It's a bit hard to tell from the diffs which took some time to load.

@jaclync, I had to reset the baseline since a lot of warnings came for the old code when it was moved to WooFoundation or Yosemite. I solved what I could, but some warnings were misleading, for methods or variables that were in fact used but not detected by Periphery.

From .periphery.yml:

A baseline is used to manage the large number of warnings from a mature codebase.
It contains a snapshot of all "existing/acceptable" warnings. Periphery will ignore all
warnings in this file, allowing you to focus on new issues in pull requests.

After fixing existing warnings, regenerate this file with:
periphery scan --write-baseline Scripts/Periphery/periphery_baseline.json
baseline: Scripts/Periphery/periphery_baseline.json

jaclync

Thanks for merging these changes before the POS module! LGTM mostly two comments marked with❓ .

jaclync · 2025-09-17T01:57:36Z

WooCommerce/Classes/POS/Presentation/POSReceiptEligibilityBanner.swift

    var body: some View {
        HStack(spacing: Constants.elementSpacing) {
-            Image(uiImage: .appIconDefault)
+            Image(uiImage: .posAppIconDefault)


I was wondering where this app icon image is used, and it looks like POSReceiptEligibilityBanner isn't needed anymore. POS requires WC version 9.6.0-beta, and the receipt eligibility check with the shared ReceiptEligibilityUseCase is on 9.5.0. I believe we can remove the eligibility check in the POS PaymentsActionButtons.handleSendReceiptAction along with this view & image. We can do the removal separately as well.

Thanks. Yes, I haven't checked how these images are used beyond the fact that we're accessing a UIImage extension within POS code so the easy move back then felt to just have a POS-focused extension with the images that it uses. I'll double check if this image is not used and remove the dependency.

jaclync · 2025-09-17T02:09:53Z

WooCommerce/WooCommerceTests/POS/Controllers/PointOfSaleItemsControllerTests.swift

-                                                                                name: "Parent product",
-                                                                                productImageSource: nil,
-                                                                                productID: 125))
+                                                                              name: "Parent product",
+                                                                              productImageSource: nil,
+                                                                              productID: 125))


super nit: indentation seems correct before, maybe these diffs are unintentional? Same for L500-L502.

jaclync · 2025-09-17T03:17:28Z

Modules/Sources/WooFoundationCore/String+Helpers.swift

+    /// - Returns: a string with the newline character removed, if the
+    ///            newline character is the last character in the string.
+    ///
+    static func stripLastNewline(in string: String) -> String {


nit: this function is in both String+Helpers files, is it intentional? It actually doesn't seem to be used from a quick search, maybe we can remove this in both files.

jaclync · 2025-09-17T03:20:14Z

Modules/Sources/WooFoundationCore/String+Helpers.swift

+    /// Get quotation marks from Locale
+    static var quotes: (String, String) {
+        guard
+            let bQuote = Locale.current.quotationBeginDelimiter,
+            let eQuote = Locale.current.quotationEndDelimiter
+        else { return ("\"", "\"") }
+
+        return (bQuote, eQuote)
+    }
+
+    /// Puts quotation marks at the beginning and the end of the string
+    var quoted: String {
+        let (bQuote, eQuote) = String.quotes
+        return bQuote + self + eQuote
+    }
+
+    /// Given an string made of tags separated by commas, returns an array with these tags
+    ///
+    func setOfTags() -> Set<String>? {
+        guard !self.isEmpty else {
+            return [String()]
+        }
+
+        let arrayOfTags = self.components(separatedBy: ",").map({ $0.trimmingCharacters(in: .whitespacesAndNewlines) })
+
+        guard !arrayOfTags.isEmpty else {
+            return nil
+        }
+
+        return Set(arrayOfTags)
+    }


nit: these 3 functions are also in the other String+Helpers file. Could we just keep them in one place?

jaclync · 2025-09-17T03:23:57Z

WooCommerce/Classes/POS/Presentation/PointOfSaleCollectCashView.swift

+                                                                                                 storeCurrencySettings: currencySettings,
+                                                                                                 allowNegativeNumber: false))
+        self.viewHelper = CollectCashViewHelper(currencySettings: currencySettings)
+        self.orderTotal = orderTotal


nit: now that orderTotal is DI'ed, the property could be private.

jaclync · 2025-09-17T04:00:15Z

...asses/POS/Presentation/Payments Onboarding/PointOfSaleCardPresentPaymentOnboardingView.swift

+
+import enum Yosemite.CardPresentPaymentOnboardingState
+
+class PreviewOnboardingViewFactoryConfiguration: CardPresentPaymentsOnboardingViewConfiguration {


nit: could be final.

jaclync · 2025-09-17T04:04:33Z

WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift

-        analytics.track(.pointOfSaleCheckoutCashPaymentTapped)
+        analytics.track(.pointOfSaleCashPaymentTapped)


❓ I'm guessing this event change is from merging from trunk and not intentional? It was updated in 363e0b0#diff-b9d72a7102bb1a5778e338bc3b2046cd3490aa8dc893fc3a05f18cb655ce698cR371.

Good catch. Yes, not intentional.

jaclync · 2025-09-17T05:02:27Z

WooCommerce/Classes/POS/Protocols/POSDependencyProviding.swift

+/// Protocol that provides analytics tracking capabilities for POS
+protocol POSAnalyticsProviding {
+    func track(event: WooAnalyticsEvent)
+    func track(_ stat: WooAnalyticsStat)


nit: this could be an extension of POSAnalyticsProviding, calling track(_ stat: WooAnalyticsStat, parameters: [String: WooAnalyticsEventPropertyType]) with [:] for the parameters

jaclync · 2025-09-17T05:03:58Z

WooCommerce/Classes/POS/Adaptors/POSServiceLocatorAdaptor.swift

+
+private struct POSFeatureFlagAdaptor: POSFeatureFlagProviding {
+    func isFeatureFlagEnabled(_ flag: FeatureFlag) -> Bool {
+        return ServiceLocator.featureFlagService.isFeatureFlagEnabled(flag)


super nit:

Suggested change

return ServiceLocator.featureFlagService.isFeatureFlagEnabled(flag)

ServiceLocator.featureFlagService.isFeatureFlagEnabled(flag)

jaclync · 2025-09-17T05:08:24Z

WooCommerce/WooCommerceTests/POS/Models/PointOfSaleAggregateModelTests.swift

-            #expect(mockAnalyticsProvider.receivedEvents.first(where: { $0 == "checkout_cash_payment_tapped" }) != nil)
-        }
-
-        @Test func collectCashPayment_when_invoked_tracks_expected_event() async throws {


❓ is this removal intentional?

No, it was unintentional.

staskus · 2025-09-17T06:09:31Z

@jaclync thank you very much for the review. There are so many changes, I tried to go through each of them carefully, but some details still slipped away. I'll address the comments.

…hat supports receipts

…roviding

staskus added 14 commits September 16, 2025 13:35

POS Modularization: Move core utilities to WooFoundationCore

2e09e22

- Move VersionHelpers to WooFoundationCore for cross-module reuse - Move String+Helpers to WooFoundationCore for cross-module reuse - Move Array+Helpers to WooFoundationCore for cross-module reuse

POS Modularization: Move analytics infrastructure to WooFoundationCore

a2afa8f

- Move WooAnalyticsEvent to WooFoundationCore for cross-module reuse - Move WooAnalyticsStat to WooFoundationCore for cross-module reuse - Create WooAnalyticsEvent+WooApp for app-specific extensions

POS Modularization: Move business logic to Yosemite

bb2fce8

- Move OrderTotalsCalculator to Yosemite for cross-module reuse

POS Modularization: Move connectivity utilities to WooFoundation

45d8b16

- Move ConnectivityObserver protocol to WooFoundation for cross-module reuse - Move DefaultConnectivityObserver to WooFoundation for cross-module reuse - Create POS-specific Error+Connectivity extensions

POS Modularization: Add POS-specific extensions and cleanup

541baa3

- Add UIImage+POS extensions for POS-specific images - Delete unused CardPresentPaymentError file - Update Card Present Payment event handling

POS Modularization: Update main app files with new module imports

becc6a7

- Update ViewRelated files to use WooFoundation and WooFoundationCore imports - Update ViewModels to use new module structure - Update system files with proper imports - Update tools and utilities with module imports

POS Modularization: Update Xcode project file

db81fe3

- Update project file references for moved and deleted files - Add new module dependencies and file references - Remove obsolete file references from deleted files

Merge branch 'trunk' into woomob-935-woo-pos-hack-week-pos-modulariza…

19a31f6

…tion-progress-so-far-trunk

staskus added this to the 23.3 milestone Sep 16, 2025

staskus added type: task An internally driven task. feature: POS labels Sep 16, 2025

staskus added 2 commits September 16, 2025 14:56

Reset periphery baseline due to a lot of false-positives after moving…

ac5bbc7

… files

Remove unused code

7de0c31

staskus added the category: tracks Related to analytics, including Tracks Events. label Sep 16, 2025

Update PointOfSaleOrderControllerTests.swift

cbce231

staskus force-pushed the woomob-935-woo-pos-hack-week-pos-modularization-progress-so-far-trunk branch from f386672 to cbce231 Compare September 16, 2025 13:23

staskus added 4 commits September 16, 2025 16:49

Remove ServiceLocator dependency in new features

bd8b367

Update RELEASE-NOTES.txt

3a0301d

Revert Package.resolved

57d0c68

Remove @available(iOS 17.0, *)

bf7fd6f

staskus requested a review from jaclync September 16, 2025 14:31

jaclync approved these changes Sep 17, 2025

View reviewed changes

staskus mentioned this pull request Sep 17, 2025

[Woo POS] Modularization: Extract FormattableAmountTextField, Coupon Creation, ProductImageThumbnail #16134

Merged

1 task

staskus added 14 commits September 17, 2025 09:22

Fix PointOfSaleItemsControllerTests identation

1cd63ca

Remove unused or duplicate methods from String+Helpers

925a365

Make orderTotal private in PointOfSaleCollectCashView

6e23e39

Remove receipt non-eligible banner since POS only supports versions t…

9e603b5

…hat supports receipts

Remove unused SwiftUI import

187d0c6

Remove unused CurrencySettings import

3260430

Remove optional return

23e59ee

Revert to track pointOfSaleCheckoutCashPaymentTapped

6e6afbe

Make PreviewOnboardingViewFactoryConfiguration final

7efe91e

Create an extension for optional parameters tracking in POSAnalyticsP…

6bfc4eb

…roviding

Revert PointOfSaleAggregateModelTests

c1fdc2b

Remove isEligibleForPointOfSaleReceipts

7037fc3

Rename OnboardingViewFactory to OnboardingViewContainer

e4f2117

Address periphery warning

9da0a71

staskus merged commit e46322c into trunk Sep 17, 2025
14 checks passed

staskus deleted the woomob-935-woo-pos-hack-week-pos-modularization-progress-so-far-trunk branch September 17, 2025 10:14


		import enum Yosemite.CardPresentPaymentOnboardingState

		class PreviewOnboardingViewFactoryConfiguration: CardPresentPaymentsOnboardingViewConfiguration {

		analytics.track(.pointOfSaleCheckoutCashPaymentTapped)
		analytics.track(.pointOfSaleCashPaymentTapped)

	return ServiceLocator.featureFlagService.isFeatureFlagEnabled(flag)
	ServiceLocator.featureFlagService.isFeatureFlagEnabled(flag)

[Woo POS] Modularization: Bring existing changes to trunk #16132

[Woo POS] Modularization: Bring existing changes to trunk #16132

Uh oh!

Conversation

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

Uh oh!

Description

Steps to reproduce

Testing information

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

jaclync commented Sep 17, 2025

Uh oh!

staskus commented Sep 17, 2025

Uh oh!

jaclync left a comment

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!

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!

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!

staskus commented Sep 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

staskus commented Sep 16, 2025 •

edited

Loading

dangermattic commented Sep 16, 2025 •

edited

Loading

wpmobilebot commented Sep 16, 2025 •

edited

Loading