Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import protocol Experiments.FeatureFlagService
}

@MainActor
func refreshEligibility() async throws {
// TODO: WOOMOB-720 - refresh eligibility
func refreshEligibility(reason: POSIneligibleReason) async throws {
eligibilityState = try await posEligibilityChecker.refreshEligibility(ineligibleReason: reason)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ struct PointOfSaleEntryPointView: View {
}
case let .ineligible(reason):
POSIneligibleView(reason: reason, onRefresh: {
try await posEntryPointController.refreshEligibility()
try await posEntryPointController.refreshEligibility(reason: reason)
})
}
}
Expand Down
6 changes: 3 additions & 3 deletions WooCommerce/Classes/POS/TabBar/POSIneligibleView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ struct POSIneligibleView: View {
comment: "Suggestion for disabled feature switch: enable feature in WooCommerce settings")
case .featureSwitchSyncFailure:
return NSLocalizedString("pos.ineligible.suggestion.featureSwitchSyncFailure",
value: "Try relaunching the app or check your internet connection and try again.",
comment: "Suggestion for feature switch sync failure: relaunch or check connection")
value: "Please check your internet connection and try again.",
comment: "Suggestion for feature switch sync failure: check connection and retry")
case let .unsupportedCurrency(supportedCurrencies):
let currencyList = supportedCurrencies.map { $0.rawValue }
let formattedCurrencyList = ListFormatter.localizedString(byJoining: currencyList)
Expand All @@ -114,7 +114,7 @@ struct POSIneligibleView: View {
return String.localizedStringWithFormat(format, formattedCurrencyList)
case .siteSettingsNotAvailable:
return NSLocalizedString("pos.ineligible.suggestion.siteSettingsNotAvailable",
value: "Check your internet connection and try relaunching the app. If the issue persists, please contact support.",
value: "Check your internet connection and try again. If the issue persists, please contact support.",
comment: "Suggestion for site settings unavailable: check connection or contact support")
case .selfDeallocated:
return NSLocalizedString("pos.ineligible.suggestion.selfDeallocated",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ final class LegacyPOSTabEligibilityChecker: POSEntryPointEligibilityCheckerProto
let eligibility = await checkI1Eligibility()
return eligibility == .eligible
}

func refreshEligibility(ineligibleReason: POSIneligibleReason) async throws -> POSEligibilityState {
assertionFailure("POS as a tab i1 implementation should not refresh eligibility as the eligibility check is performed in the visibility check.")
return .eligible
}
}

private extension LegacyPOSTabEligibilityChecker {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ protocol POSEntryPointEligibilityCheckerProtocol {
func checkVisibility() async -> Bool
/// Determines whether the site is eligible for POS.
func checkEligibility() async -> POSEligibilityState
/// Refreshes the eligibility state based on the provided ineligible reason.
func refreshEligibility(ineligibleReason: POSIneligibleReason) async throws -> POSEligibilityState
}

final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol {
Expand Down Expand Up @@ -113,6 +115,33 @@ final class POSTabEligibilityChecker: POSEntryPointEligibilityCheckerProtocol {

return await featureFlagEligibility == .eligible
}

func refreshEligibility(ineligibleReason: POSIneligibleReason) async throws -> POSEligibilityState {
switch ineligibleReason {
case .unsupportedIOSVersion:
// TODO: WOOMOB-768 - hide refresh CTA in this case
return .ineligible(reason: .unsupportedIOSVersion)
case .siteSettingsNotAvailable, .unsupportedCurrency:
do {
try await syncSiteSettingsRemotely()
return await checkEligibility()
} catch POSTabEligibilityCheckerError.selfDeallocated {
return .ineligible(reason: .selfDeallocated)
} catch {
throw error
}
case .unsupportedWooCommerceVersion, .wooCommercePluginNotFound:
// TODO: WOOMOB-799 - sync the WooCommerce plugin then check eligibility again.
// For now, it requires relaunching the app or switching stores to refresh the plugin info.
return await checkEligibility()
case .featureSwitchDisabled:
// TODO: WOOMOB-759 - enable feature switch via API and check eligibility again
// For now, just checks eligibility again.
return await checkEligibility()
case .featureSwitchSyncFailure, .selfDeallocated:
return await checkEligibility()
}
}
}

// MARK: - WC Plugin Related Eligibility Check
Expand Down Expand Up @@ -235,6 +264,25 @@ private extension POSTabEligibilityChecker {
}
return .eligible
}

@MainActor
func syncSiteSettingsRemotely() async throws {
try await withCheckedThrowingContinuation { [weak self] (continuation: CheckedContinuation<Void, Error>) in
guard let self else {
return continuation.resume(throwing: POSTabEligibilityCheckerError.selfDeallocated)
}
stores.dispatch(SettingAction.synchronizeGeneralSiteSettings(siteID: siteID) { [weak self] error in
guard let self else {
return continuation.resume(throwing: POSTabEligibilityCheckerError.selfDeallocated)
}
if let error {
return continuation.resume(throwing: error)
}
siteSettings.refresh()
continuation.resume(returning: ())
})
}
}
Comment on lines +269 to +285
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we could/should start defining these sorts of things in Yosemite? It's not as good as full async, but better than potentially writing this much nested code more than once. The errors would have to be generic, and we might need to pass in stores, but that seems potentially OK?

Not thought this through particularly thoroughly though. There's a good chance we'd be better off investing time in making Yosemite more async friendly. Nothing to do in this PR 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. Definitely would be nice to create better interface in Yosemite for the POS use case, though sometimes there's already quite some logic in the Yosemite Store implementation. In general, I've been creating new APIs in Yosemite if it's something that doesn't exactly exist in a Yosemite Store. For something that's already available in a Yosemite Store, I still reuse it.

Would like to get your thought on the naming of the new APIs in Yosemite. WDYT about creating POS{Entity}Service for {Entity}Store in Yosemite? Currently, we have POSOrderService, POSReceiptService, POSEligibilityService behind their own protocol in Yosemite. I can make this change separately.

}

// MARK: - Remote Feature Flag Eligibility Check
Expand Down Expand Up @@ -277,6 +325,10 @@ private extension POSTabEligibilityChecker {
}
}

private enum POSTabEligibilityCheckerError: Error {
case selfDeallocated
}

private extension POSTabEligibilityChecker {
enum Constants {
static let wcPluginName = "WooCommerce"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,8 @@ final class MockPOSEligibilityChecker: POSEntryPointEligibilityCheckerProtocol {
func checkEligibility() async -> POSEligibilityState {
eligibility
}

func refreshEligibility(ineligibleReason: POSIneligibleReason) async throws -> POSEligibilityState {
.ineligible(reason: ineligibleReason)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -692,4 +692,8 @@ private final class MockAsyncPOSEligibilityChecker: POSEntryPointEligibilityChec
}
}
}

func refreshEligibility(ineligibleReason: POSIneligibleReason) async throws -> POSEligibilityState {
.ineligible(reason: ineligibleReason)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,180 @@ struct POSTabEligibilityCheckerTests {
// Then
#expect(result == .eligible)
}

// MARK: - `refreshEligibility` Tests

@Test func refreshEligibility_returns_ineligible_for_unsupportedIOSVersion() async throws {
// Given
let checker = POSTabEligibilityChecker(siteID: siteID,
siteSettings: siteSettings,
pluginsService: pluginsService,
stores: stores)

// When
let result = try await checker.refreshEligibility(ineligibleReason: .unsupportedIOSVersion)

// Then
#expect(result == .ineligible(reason: .unsupportedIOSVersion))
}

@Test(arguments: [
POSIneligibleReason.siteSettingsNotAvailable,
POSIneligibleReason.unsupportedCurrency(supportedCurrencies: [.USD])
])
fileprivate func refreshEligibility_syncs_site_settings_and_checks_eligibility_for_site_settings_issues(ineligibleReason: POSIneligibleReason) async throws {
// Given
setupCountry(country: .us, currency: .USD)
setupWooCommerceVersion("9.6.0")
setupPOSFeatureEnabled(.success(true))

var syncCalled = false
stores.whenReceivingAction(ofType: SettingAction.self) { action in
switch action {
case .synchronizeGeneralSiteSettings(_, let completion):
syncCalled = true
completion(nil) // Success
case .isFeatureEnabled(_, _, let completion):
completion(.success(true))
default:
break
}
}

let checker = POSTabEligibilityChecker(siteID: siteID,
siteSettings: siteSettings,
pluginsService: pluginsService,
stores: stores)

// When
let result = try await checker.refreshEligibility(ineligibleReason: ineligibleReason)

// Then
#expect(syncCalled == true)
#expect(result == .eligible)
}

@Test(arguments: [
POSIneligibleReason.siteSettingsNotAvailable,
POSIneligibleReason.unsupportedCurrency(supportedCurrencies: [.USD])
])
fileprivate func refreshEligibility_returns_siteSettingsNotAvailable_when_site_settings_sync_fails(ineligibleReason: POSIneligibleReason) async throws {
// Given
setupCountry(country: .us, currency: .USD)
setupWooCommerceVersion("9.6.0")

var syncCalled = false
stores.whenReceivingAction(ofType: SettingAction.self) { action in
switch action {
case .synchronizeGeneralSiteSettings(_, let completion):
syncCalled = true
completion(NSError(domain: "test", code: 500)) // Network error
default:
break
}
}

let checker = POSTabEligibilityChecker(siteID: siteID,
siteSettings: siteSettings,
pluginsService: pluginsService,
stores: stores)

// When & Then - Should throw the network error
#expect(syncCalled == false) // Not called yet
await #expect(throws: NSError.self) {
try await checker.refreshEligibility(ineligibleReason: ineligibleReason)
}
#expect(syncCalled == true) // Called during the attempt
}

@Test func refreshEligibility_checks_eligibility_for_unsupportedWooCommerceVersion() async throws {
// Given
setupCountry(country: .us, currency: .USD)
setupWooCommerceVersion("9.5.0") // Still below minimum

let checker = POSTabEligibilityChecker(siteID: siteID,
siteSettings: siteSettings,
pluginsService: pluginsService,
stores: stores)

// When
let result = try await checker.refreshEligibility(ineligibleReason: .unsupportedWooCommerceVersion(minimumVersion: "9.6.0-beta"))

// Then - Should check eligibility again (still ineligible due to version)
#expect(result == .ineligible(reason: .unsupportedWooCommerceVersion(minimumVersion: "9.6.0-beta")))
}

@Test func refreshEligibility_checks_eligibility_for_wooCommercePluginNotFound() async throws {
// Given
setupCountry(country: .us, currency: .USD)
setupWooCommerceVersion("9.6.0") // Now eligible version
setupPOSFeatureEnabled(.success(true))

let checker = POSTabEligibilityChecker(siteID: siteID,
siteSettings: siteSettings,
pluginsService: pluginsService,
stores: stores)

// When
let result = try await checker.refreshEligibility(ineligibleReason: .wooCommercePluginNotFound)

// Then - Should check eligibility again (now eligible)
#expect(result == .eligible)
}

@Test func refreshEligibility_checks_eligibility_for_featureSwitchDisabled() async throws {
// Given
setupCountry(country: .us, currency: .USD)
setupWooCommerceVersion("10.0.0") // Version that supports feature switch
setupPOSFeatureEnabled(.success(true)) // Now enabled

let checker = POSTabEligibilityChecker(siteID: siteID,
siteSettings: siteSettings,
pluginsService: pluginsService,
stores: stores)

// When
let result = try await checker.refreshEligibility(ineligibleReason: .featureSwitchDisabled)

// Then - Should check eligibility again (now eligible)
#expect(result == .eligible)
}

@Test func refreshEligibility_checks_eligibility_for_featureSwitchSyncFailure() async throws {
// Given
setupCountry(country: .us, currency: .USD)
setupWooCommerceVersion("10.0.0")
setupPOSFeatureEnabled(.failure(NSError(domain: "test", code: 0))) // Still failing

let checker = POSTabEligibilityChecker(siteID: siteID,
siteSettings: siteSettings,
pluginsService: pluginsService,
stores: stores)

// When
let result = try await checker.refreshEligibility(ineligibleReason: .featureSwitchSyncFailure)

// Then - Should check eligibility again (still failing)
#expect(result == .ineligible(reason: .featureSwitchSyncFailure))
}

@Test func refreshEligibility_checks_eligibility_for_selfDeallocated() async throws {
// Given
setupCountry(country: .us, currency: .USD)
setupWooCommerceVersion("9.6.0")
setupPOSFeatureEnabled(.success(true))

let checker = POSTabEligibilityChecker(siteID: siteID,
siteSettings: siteSettings,
pluginsService: pluginsService,
stores: stores)

// When
let result = try await checker.refreshEligibility(ineligibleReason: .selfDeallocated)

// Then - Should check eligibility again (now eligible)
#expect(result == .eligible)
}
}

private extension POSTabEligibilityCheckerTests {
Expand Down