[Local catalog] Feature eligibility checker #16276

joshheald · 2025-10-23T11:51:14Z

Merge after: #16277

Description

This PR adds a POSLocalCatalogEligibilityService to check whether the local catalog is available, rather than relying on the presence of GRDBManager/Sync Coordinator. I also took

We check the following conditions at present:

Catalog size
Local feature flag
POS tab eligible

In future, we can add additional conditions:

Remote feature flag
A new state for "eligibleForSync", which includes last POS use time, etc. This new state may be false even if the tab would still use local catalog when it's tapped, to prevent background syncs for users who aren't really using POS.

We create it as part of the POSTabEligibilityChecker, which means it's updated when we switch store and ready by the time the tab is tapped.

Apologies it's so long. Getting it to work properly had more edge cases than I expected.

Testing information

You can test whether the local catalog is used by opening POS and scrolling – it's clear from the loading behaviour which is in use. You can also check the logs for messages including POSLocalCatalogEligibilityService

I've tested the following scenarios

Feature flag off
Feature flag on
- Catalog size over 1000
- Catalog size under 1000
- POS Ineligible store
- Catalog size check failed – includes recovery on subsequent attempts to open POS.

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

This allows us to directly check the feature flag

dangermattic · 2025-10-23T11:51:54Z

	1 Warning
⚠️	View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.

Generated by 🚫 Danger

wpmobilebot · 2025-10-23T12:17:36Z

📲 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	`pr16276-1ba7a70`
	Version	`23.5`
	Bundle ID	`com.automattic.alpha.woocommerce`
	Commit	`1ba7a70`
	Installation URL	0rc5cdgu0msj8

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

staskus · 2025-10-23T16:26:18Z

Apologies it's so long. Getting it to work properly had more edge cases than I expected.

My experience so far with the local catalog tasks 😀

staskus

Thanks for the work!

It will help a lot with the future tasks.

In practice, it worked well and I couldn't break it.

I made a few comments for now and for the future. I think for tidiness we should move the creation to POSTabCoordinator to have everything in a single place.

Also, consider refreshing strategy, possibly utilizing the foreground dispatcher. If we don't want eligibility to be able to change in the middle of using POS, we could consider just injecting the eligibilityState into any POS controllers instead of the service itself, to guard against the cases of inconsistency.

WooCommerce/Classes/POS/Eligibility/POSLocalCatalogEligibilityService.swift

staskus · 2025-10-24T05:15:04Z

WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift

+            // Check local catalog eligibility before initializing infrastructure
+            // Try to use pre-created service from eligibility checker, otherwise create it now
+            let eligibilityService: POSLocalCatalogEligibilityServiceProtocol
+            if let preCreatedService = (eligibilityChecker as? POSTabEligibilityChecker)?.localCatalogEligibilityService {


We could include it in POSEntryPointEligibilityCheckerProtocol, or even use POSTabEligibilityChecker directly in POSTabCoordinator. This coordinator doesn't rely on protocols. It's like a factory that builds and uses concrete types.

Yep, good point. I'll neaten this up, the optionality here was annoying me anyway.

staskus · 2025-10-24T05:22:49Z

WooCommerce/Classes/ViewRelated/Dashboard/Settings/POS/POSTabEligibilityChecker.swift

    private let siteSettingService: POSSiteSettingServiceProtocol
    private let appPasswordSupportState: ApplicationPasswordsExperimentState

+    /// Local catalog eligibility service - created asynchronously on init


I looked at how dependencies are created, and I think this could be moved from POSTabEligibilityChecker to POSTabCoordinator.

POSTabCoordinator is a good choice since:

It's created at the time of login/site changes

Responsible for creating/providing POS dependencies

A single place where we create a local catalog eligibility checker

We pass tabEligibilityChecker into init which is convenient

We need the dependency to be within POSTabCoordinator to check the eligibility

Therefore, we could move the Task { } into the init of POSTabCoordinator and the tab eligibility checker wouldn't be affected. We could still use stores to get credentials and selectedSite.

It was there originally. The reason I moved it to the tab eligibility checker was to cleanly check that the store was actually POS eligible in the flow of doing the local catalog eligibility check. I'll look again though, and see if I can do both.

staskus · 2025-10-24T05:30:48Z

WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift

+            switch eligibilityService.eligibilityState {
+            case .ineligible(reason: .catalogSizeCheckFailed):
+                // If we cached a failed check, we can recover by refreshing the value before we next open POS
+                await eligibilityService.refreshEligibilityState()


As I understand, we will get a refreshed state in 3 other cases:

After the login

After changing the site

Fresh launch of the app

If POS and the app are continued to be used, we're not checking eligibility. If we introduce a remote feature flag, for example, we may want to know earlier if the site is still eligible.

One of the potential places is ForegroundPOSCatalogSyncDispatcher. We not only need to introduce eligibilityState checks into the dispatcher, but it could also refresh the state before checking. We could make sure a task is created to do that. WDYT?

One thing I want to avoid is the eligibility changing from while POS is in use. If it did, we'd need to restart POS, or we'd have had to use your other suggestion of passing only the initial state in so it stayed that way throughout.

If POS and the app are continued to be used, we're not checking eligibility. If we introduce a remote feature flag, for example, we may want to know earlier if the site is still eligible.

That's true, and really it's deliberate. I don't know when we actually fetch remote feature flags, but perhaps we should also refresh eligibility when they leave POS – I thought maybe on foreground, but I don't really want to do it if they foreground the app in POS mode.

One of the potential places is ForegroundPOSCatalogSyncDispatcher. We not only need to introduce eligibilityState checks into the dispatcher, but it could also refresh the state before checking

AIUI, the foreground sync lives outside POS's context, and runs while the app is in the foreground, regardless of whether POS is in use. So it is appropriate to respect eligibility changes here, even though that would lead to a degraded experience if it went from eligible to ineligible while the POS was in use. Trouble is, refreshing the state here would result in it being ineligible everywhere.

It's kind of tricky. On one hand, we have a good single source of truth right now for "does the app think that this site is local catalog eligible". It might be outdated... but it can only change at times when we're actually in a position to do something about it – when POS is not in use.

On the other hand, there are arguments for losing that single source of truth in favour of passing a snapshot of the eligibility at a particular point in time to the different parts of the POS. I'd really rather avoid the potential inconsistencies with doing that, though.

I do worry a little about this – we still have POS tab as a discrete mode in the app, but that feels like something which can't last forever, and I'd rather not rely on it too much. While we do have it, it still feels best to only check the eligibility when we're not in POS mode. I recognise that someone using as a kiosk may never get an updated value... but given this is a temporary check (we'll eventually use local catalog for everyone) perhaps that's not too big a risk?

staskus · 2025-10-24T05:35:54Z

Modules/Sources/PointOfSale/Models/PointOfSaleSettingsController.swift

    }
+
+    private func checkLocalCatalogEligibility() {
+        isLocalCatalogEligible = localCatalogEligibilityService?.eligibilityState == .eligible


This kind of implementation (deliberately?) assumes that eligibilityState cannot change while the POS is in use. Which may be what we want, so I just want to make sure it's intended?

Yeah, it's intended... but we build that in to the service, so we could just use the service directly and still get the benefit.

staskus · 2025-10-24T05:37:02Z

Modules/Sources/PointOfSale/Protocols/POSLocalCatalogEligibilityServiceProtocol.swift

+    case posTabNotEligible
+    case featureFlagDisabled
+    case catalogSizeTooLarge(totalCount: Int, limit: Int)
+    case catalogSizeCheckFailed(underlyingError: String)


👍 This is good and tidy, we will be able to include all the ineligibility cases here.

staskus · 2025-10-24T05:51:59Z

WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift

+                eligibilityService = preCreatedService
+            } else {
+                // Fallback: assume we're POS eligible and create service
+                eligibilityService = await POSLocalCatalogEligibilityService(


I recognize this is not ideal since it would block the UI with no loading indication after pressing the tab. I tested with the network link conditioner, and the delay is fairly manageable, even with Edge.

Yeah, it's annoying but I felt like it was an acceptable trade off too. Thanks for checking it. We could make yet another loading screen, but I really want to avoid that, and we can't safely use the existing ones when we don't know the catalog type.

staskus · 2025-10-24T11:56:06Z

WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift

+        Task { @MainActor [weak self] in
+            guard let self else { return }
+
+            let service = await POSLocalCatalogEligibilityService(


Is there a reason we cannot await checkEligibility() here to set isPOSTabVisible?

Visibility and eligibility are different... Ineligible sites can still have the tab visible, where we use it as an upsell.

The ticket (it turns out) was worded as visibility, and we're discussing which it should be here: p1761304079623909/1757587788.433699-slack-C070SJRA8DP

I'm going to keep it as visibility for now, and we can revert the last commit to go back to eligibility if we change our minds.

joshheald added 10 commits October 22, 2025 14:52

Add POSLocalCatalogEligibilityService

cf1753c

Tests for POSLocalCatalogEligibilityService

e25a69e

Integrate the new catalog eligibility checker

38a326c

Check eligibility when choosing item controller

6046dc5

Use eligibility service without passing result

d598372

Remove delay for catalog eligibility on open POS

93c5cdc

Ensure we recover from cached ineligible states

8571ccd

Move catalog eligibility service to App target

081b6f0

This allows us to directly check the feature flag

Include POS Tab in local catalog eligibility

071e1a4

Avoid rechecking results which are unlikely to change

beba51b

joshheald added this to the 23.6 milestone Oct 23, 2025

joshheald added type: task An internally driven task. feature: POS labels Oct 23, 2025

Remove unsupported syntax for Xcode 16

ae8b3e0

joshheald added 2 commits October 23, 2025 13:19

Fix test

08e30f5

Periphery fixes

8fbe34b

joshheald requested a review from staskus October 23, 2025 12:48

joshheald marked this pull request as ready for review October 23, 2025 12:49

joshheald mentioned this pull request Oct 23, 2025

[Woo POS][Local Catalog] Loading screen for missing catalog when launching POS #16272

Open

1 task

staskus self-assigned this Oct 23, 2025

Base automatically changed from woomob-1287-local-catalog-currency-country-checker-refactor to trunk October 23, 2025 16:57

staskus approved these changes Oct 24, 2025

View reviewed changes

joshheald added 5 commits October 24, 2025 09:36

Include catalog eligibilty checker in protocol

1a3ca53

Use service for catalog eligibility in settings

15b717f

Rename catalog service variable for clarity

8d9a123

Pass catalog eligibility value when presenting POS

d402747

Use POS visibility for catalog checks

1ba7a70

staskus reviewed Oct 24, 2025

View reviewed changes

joshheald merged commit 79ac3b9 into trunk Oct 24, 2025
13 checks passed

joshheald deleted the woomob-1287-woo-poslocal-catalog-high-level-feature-eligibility-checker branch October 24, 2025 13:21

[Local catalog] Feature eligibility checker #16276

[Local catalog] Feature eligibility checker #16276

Uh oh!

Conversation

joshheald commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing information

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

staskus commented Oct 23, 2025

Uh oh!

staskus left a comment

Choose a reason for hiding this comment

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!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

joshheald commented Oct 23, 2025 •

edited

Loading

dangermattic commented Oct 23, 2025 •

edited

Loading

wpmobilebot commented Oct 23, 2025 •

edited

Loading