-
Notifications
You must be signed in to change notification settings - Fork 121
[POS as a tab i2] Enable POS tab for stores in eligible countries behind i2 feature flag #15824
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
|
|
staskus
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.
Thanks! Works well 👍
| break | ||
| case let .ineligible(reason): | ||
| if reason == .unsupportedCurrency { | ||
| break |
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.
As I understand, it's intentional to show POS option with unsupported currency for i2?
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.
Yup, as in the delivery plan pdfdoF-7po-p2, we're enabling the POS tab for all stores in eligible countries (US and UK for now).
| (country: Country.gb, currency: CurrencyCode.USD, isPointOfSaleAsATabi2Enabled: true), | ||
| (country: Country.gb, currency: CurrencyCode.USD, isPointOfSaleAsATabi2Enabled: false) | ||
| ]) | ||
| fileprivate func is_ineligible_when_currency_is_not_supported(country: Country, currency: CurrencyCode, isPointOfSaleAsATabi2Enabled: Bool) async throws { |
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 see much difference between is_ineligible_when_currency_is_not_supported and is_ineligible_when_country_is_not_supported tests. They both end up verifying the same condition .ineligible(reason: .unsupportedCountry))
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.
For the eligibility check (not the visibility check), the ineligible reason is different in these two test cases. is_ineligible_when_currency_is_not_supported takes in eligible countries but mismatching currency code and results in .ineligible(reason: .unsupportedCurrency), while is_ineligible_when_country_is_not_supported takes in ineligible countries and results in .ineligible(reason: .unsupportedCountry). Please let me know if I misunderstood anything.
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.
Thanks for explaining, sorry, it's my mistake. For some reason, I read that both .ineligible results in unsupportedCountry, but that's not the case.
# Conflicts: # Modules/Sources/Experiments/DefaultFeatureFlagService.swift # Modules/Sources/Experiments/FeatureFlag.swift
|
Enabling auto-merge now, if anything actionable comes up I will address them separately. |

Closes WOOMOB-575
Just one reviewer is required.
Description
checkVisibility()method toPOSEntryPointEligibilityCheckerProtocolnow that the tab visibility is different from POS eligibility for i2pointOfSaleAsATabi2feature flag:checkEligibility()logicSteps to reproduce
Prerequisite: a WPCOM account with at least two stores where one store is eligible for POS, and the other in a country that is not eligible for POS (e.g. CA)
Testing information
Example screenshots
For a store in an eligible country but ineligible for other reason (e.g. currency):
RELEASE-NOTES.txtif necessary.