-
Notifications
You must be signed in to change notification settings - Fork 121
[POS as a tab i2] Refresh POS eligibility in ineligible UI: site settings and feature switch disabled cases #15895
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
2adc901
Add `refreshEligibility(ineligibleReason:)` to `POSEntryPointEligibil…
jaclync 5ce0a9f
Move site settings sync function to the appropriate section.
jaclync f3fa930
Update TODO comment for refreshing WC plugin.
jaclync b730d95
Add test cases for `refreshEligibility`.
jaclync 793a492
Update ineligible UI copy to not mention relaunching the app now that…
jaclync File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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 😊
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 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}Servicefor{Entity}Storein Yosemite? Currently, we havePOSOrderService,POSReceiptService,POSEligibilityServicebehind their own protocol in Yosemite. I can make this change separately.