Skip to content

Commit 0492912

Browse files
joshhealdjoshheald
committed
Use POS tab eligibility, not visibility
1 parent 654f89f commit 0492912

File tree

7 files changed

+76
-54
lines changed

7 files changed

+76
-54
lines changed

‎Modules/Sources/Yosemite/Protocols/POSLocalCatalogEligibilityServiceProtocol.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ public enum POSLocalCatalogEligibilityState: Equatable {
1212

1313
/// Reasons why local catalog is ineligible
1414
public enum POSLocalCatalogIneligibleReason: Equatable {
15-
case posTabNotVisible
15+
case posTabNotEligible
1616
case featureFlagDisabled
1717
case catalogSizeTooLarge(totalCount: Int, limit: Int)
1818
case catalogSizeCheckFailed(underlyingError: String)
@@ -33,11 +33,11 @@ public protocol POSLocalCatalogEligibilityServiceProtocol {
3333
/// - Returns: Cached eligibility state, or eligible if not yet checked
3434
func catalogEligibility(for siteID: Int64) async -> POSLocalCatalogEligibilityState
3535

36-
/// Update the POS tab visibility state and refresh eligibility for the specified site
36+
/// Update POS eligibility and refresh catalog eligibility for the specified site
3737
/// - Parameters:
38-
/// - isPOSTabVisible: Whether the POS tab is visible
38+
/// - isEligible: Whether POS is eligible for the site
3939
/// - siteID: The site ID to refresh eligibility for
40-
func updateVisibility(isPOSTabVisible: Bool, for siteID: Int64) async
40+
func updatePOSEligibility(isEligible: Bool, for siteID: Int64) async
4141

4242
/// Refresh eligibility state for a specific site
4343
/// - Parameter siteID: The site ID to check eligibility for

‎Modules/Sources/Yosemite/Tools/POS/POSLocalCatalogEligibilityService.swift‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,25 @@ public actor POSLocalCatalogEligibilityService: POSLocalCatalogEligibilityServic
55
private let catalogSizeChecker: POSCatalogSizeCheckerProtocol
66
private let catalogSizeLimit: Int
77
private let isLocalCatalogFeatureFlagEnabled: Bool
8-
private var isPOSTabVisible: Bool
98

109
// Eligibility states cached per site
1110
private var eligibilityStates: [Int64: POSLocalCatalogEligibilityState] = [:]
1211

12+
// POS eligibility states cached per site
13+
private var posEligibilityStates: [Int64: Bool] = [:]
14+
1315
/// Initialize eligibility service
16+
/// - Parameters:
17+
/// - catalogSizeChecker: Service to check catalog size for sites
18+
/// - isLocalCatalogFeatureFlagEnabled: Whether the local catalog feature flag is enabled
19+
/// - catalogSizeLimit: Maximum allowed catalog size (products + variations)
1420
public init(
1521
catalogSizeChecker: POSCatalogSizeCheckerProtocol,
1622
isLocalCatalogFeatureFlagEnabled: Bool,
17-
isPOSTabVisible: Bool,
1823
catalogSizeLimit: Int? = nil
1924
) {
2025
self.catalogSizeChecker = catalogSizeChecker
2126
self.isLocalCatalogFeatureFlagEnabled = isLocalCatalogFeatureFlagEnabled
22-
self.isPOSTabVisible = isPOSTabVisible
2327
self.catalogSizeLimit = catalogSizeLimit ?? Constants.defaultCatalogSizeLimit
2428
}
2529

@@ -33,22 +37,28 @@ public actor POSLocalCatalogEligibilityService: POSLocalCatalogEligibilityServic
3337
return await refreshEligibilityState(for: siteID)
3438
}
3539

36-
/// Update the POS tab visibility state and refresh eligibility for the specified site
37-
public func updateVisibility(isPOSTabVisible: Bool, for siteID: Int64) async {
38-
self.isPOSTabVisible = isPOSTabVisible
39-
// Refresh eligibility for the current site now that visibility has changed
40+
/// Update POS eligibility and refresh catalog eligibility for the specified site
41+
/// - Parameters:
42+
/// - isEligible: Whether POS is eligible
43+
/// - siteID: The site ID to refresh eligibility for
44+
public func updatePOSEligibility(isEligible: Bool, for siteID: Int64) async {
45+
// Store the POS eligibility state for this site
46+
posEligibilityStates[siteID] = isEligible
47+
// Refresh eligibility for the current site now that POS eligibility has changed
4048
await refreshEligibilityState(for: siteID)
4149
}
4250

4351
/// Refresh eligibility state for a specific site
4452
@discardableResult
4553
public func refreshEligibilityState(for siteID: Int64) async -> POSLocalCatalogEligibilityState {
46-
// Check POS tab visibility FIRST - no point in checking catalog if POS tab isn't visible
47-
guard isPOSTabVisible else {
48-
let state = POSLocalCatalogEligibilityState.ineligible(reason: .posTabNotVisible)
49-
eligibilityStates[siteID] = state
50-
DDLogInfo("📋 POSLocalCatalogEligibilityService: POS tab not visible for site \(siteID)")
51-
return state
54+
// Check POS tab eligibility FIRST - no point in checking catalog if POS isn't eligible
55+
if let isPOSEligible = posEligibilityStates[siteID] {
56+
guard isPOSEligible else {
57+
let state = POSLocalCatalogEligibilityState.ineligible(reason: .posTabNotEligible)
58+
eligibilityStates[siteID] = state
59+
DDLogInfo("📋 POSLocalCatalogEligibilityService: POS not eligible for site \(siteID)")
60+
return state
61+
}
5262
}
5363

5464
// Check feature flag - if disabled, no need to check catalog size

‎Modules/Tests/YosemiteTests/Mocks/MockPOSLocalCatalogEligibilityService.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ actor MockPOSLocalCatalogEligibilityService: POSLocalCatalogEligibilityServicePr
1919
return eligibilityStates[siteID] ?? .eligible
2020
}
2121

22-
func updateVisibility(isPOSTabVisible: Bool, for siteID: Int64) async {
22+
func updatePOSEligibility(isEligible: Bool, for siteID: Int64) async {
2323
// No-op for tests
2424
}
2525

‎WooCommerce/Classes/POS/TabBar/POSTabCoordinator.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,11 @@ final class POSTabCoordinator {
130130
tabContainerController.wrappedController = POSTabViewController()
131131
}
132132

133-
/// Update local catalog eligibility when POS tab visibility changes
134-
func updatePOSTabVisibility(_ isPOSTabVisible: Bool) {
133+
/// Update local catalog eligibility when POS eligibility changes
134+
func updatePOSEligibility(_ isPOSEligible: Bool) {
135135
Task { @MainActor [weak self] in
136136
guard let self, let service = self.localCatalogEligibilityService else { return }
137-
await service.updateVisibility(isPOSTabVisible: isPOSTabVisible, for: siteID)
137+
await service.updatePOSEligibility(isEligible: isPOSEligible, for: siteID)
138138
}
139139
}
140140

‎WooCommerce/Classes/ViewRelated/MainTabBarController.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -721,8 +721,8 @@ private extension MainTabBarController {
721721
updateTabViewControllers(isPOSTabVisible: isPOSTabVisible, isBookingsTabVisible: isBookingsTabVisible)
722722
viewModel.loadHubMenuTabBadge()
723723

724-
// Update POS tab coordinator with new visibility state for local catalog eligibility
725-
posTabCoordinator?.updatePOSTabVisibility(isPOSTabVisible)
724+
// Update POS tab coordinator with new eligibility state for local catalog eligibility
725+
posTabCoordinator?.updatePOSEligibility(isPOSTabVisible)
726726

727727
// Begin foreground synchronization if POS tab becomes visible
728728
await isPOSTabVisible ? posSyncDispatcher.start() : posSyncDispatcher.stop()

‎WooCommerce/Classes/Yosemite/AuthenticatedState.swift‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -181,15 +181,14 @@ class AuthenticatedState: StoresManagerState {
181181
grdbManager: ServiceLocator.grdbManager
182182
) {
183183

184-
// Create eligibility service synchronously (actor allows this)
184+
// Create eligibility service
185185
let eligibilityService = POSLocalCatalogEligibilityService(
186186
catalogSizeChecker: POSCatalogSizeChecker(
187187
credentials: credentials,
188188
selectedSite: site,
189189
appPasswordSupportState: appPasswordSupportState.eraseToAnyPublisher()
190190
),
191-
isLocalCatalogFeatureFlagEnabled: isLocalCatalogFeatureFlagEnabled,
192-
isPOSTabVisible: false // Will be updated when POS tab is shown
191+
isLocalCatalogFeatureFlagEnabled: isLocalCatalogFeatureFlagEnabled
193192
)
194193
posCatalogEligibilityChecker = eligibilityService
195194

@@ -201,12 +200,8 @@ class AuthenticatedState: StoresManagerState {
201200
catalogEligibilityChecker: eligibilityService
202201
)
203202

204-
// Perform initial eligibility check asynchronously for current site
205-
Task {
206-
if let siteID = sessionManager.defaultStoreID {
207-
await eligibilityService.refreshEligibilityState(for: siteID)
208-
}
209-
}
203+
// Note: POS eligibility will be set later by POSTabCoordinator.updatePOSEligibility
204+
// when the POS tab visibility check completes in MainTabBarController
210205
} else {
211206
posCatalogSyncCoordinator = nil
212207
posCatalogEligibilityChecker = nil

‎WooCommerce/WooCommerceTests/POS/Eligibility/POSLocalCatalogEligibilityServiceTests.swift‎

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ struct POSLocalCatalogEligibilityServiceTests {
2222
let service = POSLocalCatalogEligibilityService(
2323
catalogSizeChecker: sizeChecker,
2424
isLocalCatalogFeatureFlagEnabled: featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1),
25-
isPOSTabVisible: true,
2625
catalogSizeLimit: 1000
2726
)
2827

@@ -38,7 +37,6 @@ struct POSLocalCatalogEligibilityServiceTests {
3837
let service = POSLocalCatalogEligibilityService(
3938
catalogSizeChecker: sizeChecker,
4039
isLocalCatalogFeatureFlagEnabled: featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1),
41-
isPOSTabVisible: true,
4240
catalogSizeLimit: 1000
4341
)
4442

@@ -56,7 +54,6 @@ struct POSLocalCatalogEligibilityServiceTests {
5654
let service = POSLocalCatalogEligibilityService(
5755
catalogSizeChecker: sizeChecker,
5856
isLocalCatalogFeatureFlagEnabled: featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1),
59-
isPOSTabVisible: true,
6057
catalogSizeLimit: 1000
6158
)
6259

@@ -88,7 +85,6 @@ struct POSLocalCatalogEligibilityServiceTests {
8885
let service = POSLocalCatalogEligibilityService(
8986
catalogSizeChecker: sizeChecker,
9087
isLocalCatalogFeatureFlagEnabled: featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1),
91-
isPOSTabVisible: true,
9288
catalogSizeLimit: 1000
9389
)
9490

@@ -118,7 +114,6 @@ struct POSLocalCatalogEligibilityServiceTests {
118114
let service = POSLocalCatalogEligibilityService(
119115
catalogSizeChecker: sizeChecker,
120116
isLocalCatalogFeatureFlagEnabled: featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1),
121-
isPOSTabVisible: true,
122117
catalogSizeLimit: 1000
123118
)
124119

@@ -142,7 +137,6 @@ struct POSLocalCatalogEligibilityServiceTests {
142137
let service = POSLocalCatalogEligibilityService(
143138
catalogSizeChecker: sizeChecker,
144139
isLocalCatalogFeatureFlagEnabled: featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1),
145-
isPOSTabVisible: true,
146140
catalogSizeLimit: 1000
147141
)
148142

@@ -166,7 +160,6 @@ struct POSLocalCatalogEligibilityServiceTests {
166160
let service = POSLocalCatalogEligibilityService(
167161
catalogSizeChecker: sizeChecker,
168162
isLocalCatalogFeatureFlagEnabled: featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1),
169-
isPOSTabVisible: true,
170163
catalogSizeLimit: 1000
171164
)
172165

@@ -205,7 +198,6 @@ struct POSLocalCatalogEligibilityServiceTests {
205198
let service = POSLocalCatalogEligibilityService(
206199
catalogSizeChecker: sizeChecker,
207200
isLocalCatalogFeatureFlagEnabled: featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1),
208-
isPOSTabVisible: true,
209201
catalogSizeLimit: 1000
210202
)
211203

@@ -236,7 +228,6 @@ struct POSLocalCatalogEligibilityServiceTests {
236228
let service = POSLocalCatalogEligibilityService(
237229
catalogSizeChecker: sizeChecker,
238230
isLocalCatalogFeatureFlagEnabled: featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1),
239-
isPOSTabVisible: true,
240231
catalogSizeLimit: 100 // Custom lower limit
241232
)
242233

@@ -256,62 +247,88 @@ struct POSLocalCatalogEligibilityServiceTests {
256247
#expect(limit == 100)
257248
}
258249

259-
// MARK: - POS Tab Visibility
250+
// MARK: - POS Eligibility
260251

261-
@Test("POS tab not visible returns ineligible")
262-
func testPOSTabNotVisibleReturnsIneligible() async {
252+
@Test("POS not eligible returns ineligible")
253+
func testPOSNotEligibleReturnsIneligible() async {
263254
let sizeChecker = MockPOSCatalogSizeChecker(
264255
sizeToReturn: .success(POSCatalogSize(productCount: 500, variationCount: 400))
265256
)
266257
let featureFlagService = MockFeatureFlagService(isLocalCatalogEnabled: true)
267258
let service = POSLocalCatalogEligibilityService(
268259
catalogSizeChecker: sizeChecker,
269260
isLocalCatalogFeatureFlagEnabled: featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1),
270-
isPOSTabVisible: false,
271261
catalogSizeLimit: 1000
272262
)
273263

264+
// Set POS as not eligible
265+
await service.updatePOSEligibility(isEligible: false, for: siteID)
266+
274267
let state = await service.catalogEligibility(for: siteID)
275268

276269
guard case .ineligible(let reason) = state else {
277270
Issue.record("Expected ineligible state")
278271
return
279272
}
280273

281-
guard case .posTabNotVisible = reason else {
282-
Issue.record("Expected posTabNotVisible reason")
274+
guard case .posTabNotEligible = reason else {
275+
Issue.record("Expected posTabNotEligible reason")
283276
return
284277
}
285278

286279
// Should not have checked catalog size
287280
#expect(sizeChecker.checkCatalogSizeCallCount == 0)
288281
}
289282

290-
@Test("POS tab visibility checked before catalog size")
291-
func testPOSTabVisibilityCheckedFirst() async {
283+
@Test("POS eligibility checked before catalog size")
284+
func testPOSEligibilityCheckedFirst() async {
292285
let sizeChecker = MockPOSCatalogSizeChecker(
293286
sizeToReturn: .success(POSCatalogSize(productCount: 2000, variationCount: 0))
294287
)
295288
let featureFlagService = MockFeatureFlagService(isLocalCatalogEnabled: true)
296289
let service = POSLocalCatalogEligibilityService(
297290
catalogSizeChecker: sizeChecker,
298291
isLocalCatalogFeatureFlagEnabled: featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1),
299-
isPOSTabVisible: false,
300292
catalogSizeLimit: 1000
301293
)
302294

303-
// Should be ineligible due to POS tab not visible, not catalog size
295+
// Set POS as not eligible
296+
await service.updatePOSEligibility(isEligible: false, for: siteID)
297+
298+
// Should be ineligible due to POS not eligible, not catalog size
304299
guard case .ineligible(let reason) = await service.catalogEligibility(for: siteID) else {
305300
Issue.record("Expected ineligible state")
306301
return
307302
}
308303

309-
guard case .posTabNotVisible = reason else {
310-
Issue.record("Expected posTabNotVisible reason, not catalogSizeTooLarge")
304+
guard case .posTabNotEligible = reason else {
305+
Issue.record("Expected posTabNotEligible reason, not catalogSizeTooLarge")
311306
return
312307
}
313308

314-
// Should not have checked catalog size since POS tab wasn't visible
309+
// Should not have checked catalog size since POS wasn't eligible
315310
#expect(sizeChecker.checkCatalogSizeCallCount == 0)
316311
}
312+
313+
@Test("POS eligible allows catalog size check")
314+
func testPOSEligibleAllowsCatalogSizeCheck() async {
315+
let sizeChecker = MockPOSCatalogSizeChecker(
316+
sizeToReturn: .success(POSCatalogSize(productCount: 500, variationCount: 400))
317+
)
318+
let featureFlagService = MockFeatureFlagService(isLocalCatalogEnabled: true)
319+
let service = POSLocalCatalogEligibilityService(
320+
catalogSizeChecker: sizeChecker,
321+
isLocalCatalogFeatureFlagEnabled: featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1),
322+
catalogSizeLimit: 1000
323+
)
324+
325+
// Set POS as eligible
326+
await service.updatePOSEligibility(isEligible: true, for: siteID)
327+
328+
let state = await service.catalogEligibility(for: siteID)
329+
#expect(state == .eligible)
330+
331+
// Should have checked catalog size since POS was eligible
332+
#expect(sizeChecker.checkCatalogSizeCallCount == 1)
333+
}
317334
}

0 commit comments

Comments
 (0)