-
Notifications
You must be signed in to change notification settings - Fork 121
POSTabEligibilityCheckerI2: i2 logic #15865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
POSTabEligibilityCheckerI2 and POSTabEligibilityCheckerI2Tests for i2 implementation in a separate class
|
|
POSTabEligibilityCheckerI2 and POSTabEligibilityCheckerI2Tests for i2 implementation in a separate class
joshheald
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as expected.
I mentioned renaming a duplicate, in my last PR, but now that you've done it the other way around I don't think it's worth undoing and redoing for a small benefit in source control history.
Thanks for the change.
| @@ -0,0 +1,260 @@ | |||
| import Combine | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we're using Combine here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, removed in b6854d4.
…and-eligibility' into feat/WOOMOB-756-eligibility-checker-i2 # Conflicts: # WooCommerce/Classes/ViewRelated/MainTabBarController.swift # WooCommerce/WooCommerce.xcodeproj/project.pbxproj
… `POSTabEligibilityCheckerTests`.
|
Thanks for reviewing both PRs Josh, I've copied the i2 content to the main checker & tests files, with i2 files deleted now. Merging this to unblock future PRs. |

Part 2 of WOOMOB-756
Just one review is required.
Apology for the large diffs, the test file is ~670 diffs with some duplication of the test helpers. When
pointOfSaleAsATabi2feature flag is removed,POSTabEligibilityCheckerI2andPOSTabEligibilityCheckerI2Testswill replacePOSTabEligibilityCheckerandPOSTabEligibilityCheckerTests.Description
A recap of the differences between i1 and i2:
As the base PR #15864 updated
POSTabEligibilityCheckerandPOSTabEligibilityCheckerTeststo be i1 only, this PR addsPOSTabEligibilityCheckerI2andPOSTabEligibilityCheckerI2Testsfor the i2 implementation.POSTabEligibilityCheckerTests, but all test cases are now for i2.Steps to reproduce
i2 is back now, and the POS tab is now shown when: on a tablet, store country is eligible, remote feature flag is enabled
Store eligible for POS
Store in an eligible country but not eligible for POS
Store not in an eligible country
Testing information
Screenshots
Ineligible UI is back for a store in an eligible country but not eligible for POS:
RELEASE-NOTES.txtif necessary.