[Woo POS] Fix POS menu item visibility issues with unit tests

[jaclync]

Part of: #12779
Closes #12780

Why

As we're coming to a conclusion (at least for the first iteration) on the POS eligibility conditions in p91TBi-beb-p2#comment-12134, we're good with the checks that were implemented in #12783. Some basic test cases before closing this issue. In the meanwhile, there are two issues with the visibility of the POS menu item:

• Sometimes the onboarding state isn't up to date when landing on the Menu tab
• After toggling the POS feature switch in Settings > Experimental Features, the POS menu item's visibility isn't reflected right away and relies on switching to another tab and then back

Note that we haven't made a decision about whether to keep the feature switch in Settings > Experimental Features so we have to continue supporting it p91TBi-beb-p2#comment-12143.

🗒️ Apology on the 500 diffs, More than half of the diffs are on the unit tests, as setting up dependencies for the POS eligibility checker isn't super straightforward.

How

The main cause for the visibility issue is that the onboarding state isn't refreshed if no other parts of the app has, and POSEligibilityChecker was just using the latest onboarding state value instead of observing the state publisher. In this PR, the major changes to POSEligibilityChecker are:

• isEligible boolean is now a publisher to emit values on changes
• Update siteSettings (for the country check) from [SiteSetting] to SelectedSiteSettings to allow potential changes that could be checked on the next onboarding state change
• For unit testing:
  • POSEligibilityCheckerProtocol was created for easier unit testing in its consumer
  • UIUserInterfaceIdiom is now DI'ed

Now the checks are broken into 3 parts:

• Fixed checks: feature flag & device type
• Site-specific checks: country & currency
• Onboarding check: completed with WCPay only or preferred
  The checker first checks the fixed conditions, and then observes the onboarding state. On onboarding state changes, it first performs the site-specific checks and then the onboarding state check.

This observable isEligible boolean is tricky for the BetaFeaturesConfiguration screen. Before this PR, the availability of beta features is fixed. However, as we can't easily DI POSEligibilityChecker because the Settings (where BetaFeaturesConfiguration screen is in) is in UIKit with a SwiftUI wrapper, the BetaFeaturesConfiguration screen has to now observe the eligibility check as well. I thought about opening up a readonly isEligible boolean value type, but the onboarding state observation will result in the latest value async and thus missed in the BetaFeaturesConfiguration screen. In the end, I created a view model for BetaFeaturesConfiguration that observes the eligibility value to show the available features.

I originally wanted to share the onboarding use case with InPersonPaymentsMenuViewModel, but got some SwiftUI warning about Publishing changes from within view updates is not allowed, this will cause undefined behavior., so I decided to revert it.

Please feel free to share any suggestions on a different approach!

Testing instructions

There are a few scenarios that @jaclync will test again after approval:

• Tablet device
  • Go to the Menu tab on iPhone --> no POS row should be shown in the menu tab, and in Experimental Features the POS feature switch should not be shown
  • Go to the Menu tab on iPad --> in Experimental Features the POS feature switch should be shown, and when the switch is enabled, the POS row should be shown in the menu tab if the store is eligible for POS

Continue with an iPad with the POS feature switch enabled for the following cases:

• USD currency (WC Settings > General)
  • Switch to a store with non-USD currency --> after revisiting the Menu tab, the POS row should not be shown
• US store location (WC Settings > General)
  • Switch to a store not based in the US --> after revisiting the Menu tab, the POS row should not be shown
• Woo Payments plugin enabled and user setup complete
  • Switch to a store based in the US and using USD, but the WCPay isn't active or its onboarding isn't complete --> after revisiting the Menu tab, the POS row should not be shown

Screenshots

Simulator.Screen.Recording.-.iPad.10th.generation.-.2024-05-22.at.16.46.01.mp4

---

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

[dangermattic]

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

Generated by 🚫 Danger

[wpmobilebot]

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

	App Name	WooCommerce iOS
	Build Number	pr12808-5ffdc0a
	Version	18.7
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	5ffdc0a
	App Center Build	WooCommerce - Prototype Builds #9241

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

[joshheald]

A few things I noticed while testing:

Disabling the experimental feature toggle is not reflected in the menu until you switch store – whereas enabling it is reflected immediately.

disable-pos-broken.mp4

Onboarding completion isn't reflected until you switch tab. This is only possible if you've turned on the feature switch on another store previously, otherwise you don't even see the switch until you complete onboarding:

onboarding-refresh-tab-change.mp4

I'm not sure how much effort we should invest in making onboarding handling better now, as I think we may remove it as a condition and handle it within POS mode if needed. I just wanted to note it in case other issues are hiding in the woodwork like this. I suspect the tab switches are still required even for some other requirements changing, or perhaps when the onboarding cache is being refreshed.

[joshheald]

What benefit do we get from moving this to the viewModel?

[jaclync]

Good question - as I had to create a view model to observe POS eligibility check (from the onboarding state observation), I thought moving the appSettings dependency to the view model would enable unit testing in the future even though we don't have unit tests on it right now.

[joshheald]

nit: it might be clearer to use a filter then a map:

Suggested change

	.map { isBetaFeatureEnabled, isEligibleForPOS in
	if isBetaFeatureEnabled && isEligibleForPOS {
	return PointOfSaleEntryPoint()
	} else {
	return nil
	}
	}
	.filter { isBetaFeatureEnabled, isEligibleForPOS in
	isBetaFeatureEnabled && isEligibleForPOS
	}
	.map { _ in
	PointOfSaleEntryPoint()
	}

[jaclync]

If the downstream value is nil from isBetaFeatureEnabled && isEligibleForPOS = false => PointOfSaleEntryPoint from isBetaFeatureEnabled && isEligibleForPOS = true, then when isBetaFeatureEnabled && isEligibleForPOS = false, would filter + map set the value back to nil? I'll do a quick experiment and update.

[jaclync]

Tested in Playground today, if we apply the filter, the view model doesn't get reset back to nil when the entry point is not eligible anymore.

[jaclync]

Disabling the experimental feature toggle is not reflected in the menu until you switch store – whereas enabling it is reflected immediately.

Onboarding completion isn't reflected until you switch tab. This is only possible if you've turned on the feature switch on another store previously, otherwise you don't even see the switch until you complete onboarding:

I can't reproduce the first one 100%, but I think this is likely caused by the menu view's onAppear not being triggered when landing back on the menu screen within the menu tab (like navigating from menu > settings > back to menu). Visiting another tab then back triggers onAppear. After looking more into this unexpected behavior (onAppear is supposed to be triggered when the view becomes visible), it seems to be from onAppear being placed on the menu's NavigationStack instead of the menu list view itself:

woocommerce-ios/WooCommerce/Classes/ViewRelated/Hub Menu/HubMenu.swift

Lines 21 to 40 in 002c97b

	NavigationStack(path: $viewModel.navigationPath) {
	/// TODO: switch to `navigationDestination(item:destination)`
	/// when we drop support for iOS 16.
	menuList
	.navigationDestination(for: String.self) { id in
	detailView(menuID: id)
	}
	.navigationDestination(for: HubMenuNavigationDestination.self) { destination in
	detailView(destination: destination)
	}
	.navigationDestination(isPresented: $viewModel.showingReviewDetail) {
	reviewDetailView
	}
	.navigationDestination(isPresented: $viewModel.showingCoupons) {
	couponListView
	}
	}
	.onAppear {
	viewModel.setupMenuElements()
	}

After moving the onAppear to the menuList in 7221165, onAppear is triggered whenever the menu is visible where the menu content is updated.

I'm not sure how much effort we should invest in making onboarding handling better now, as I think we may remove it as a condition and handle it within POS mode if needed. I just wanted to note it in case other issues are hiding in the woodwork like this. I suspect the tab switches are still required even for some other requirements changing, or perhaps when the onboarding cache is being refreshed.

I agree, I'm hoping the onAppear fix would ensure the onboarding state is updated on every menu appearance, but I'll leave any other potential issues from the onboarding state for later.