-
Notifications
You must be signed in to change notification settings - Fork 118
Require selected site for AlamofireNetwork
#16124
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
base: trunk
Are you sure you want to change the base?
Require selected site for AlamofireNetwork
#16124
Conversation
@@ -18,9 +20,10 @@ public final class PointOfSaleItemFetchStrategyFactory: PointOfSaleItemFetchStra | |||
private let variationsRemote: ProductVariationsRemote | |||
|
|||
public init(siteID: Int64, | |||
credentials: Credentials?) { | |||
credentials: Credentials?, | |||
selectedSite: AnyPublisher<JetpackSite?, Never>? = nil) { |
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.
This has nil as the default value to avoid updating all related unit tests.
|
Generated by 🚫 Danger |
@iamgabrielma I see that you're already reviewing this PR. I just added some changes above to inject app password support availability (from remote feature flag and experimental feature flag) to POS's networks. This PR is ready for a new testing round 🙇 |
Closes WOOMOB-1332
Description
This PR removes the default nil value for
selectedSite
inAlamofireNetwork
. It is now required to explicitly decide if network switching should be supported when the network is created.Network switching is disabled for the following flows:
For POS flows, since
AlamofireNetwork
is created separately and there were no clear disagreement on the network switching in p91TBi-dve-p2, I updated all the services to inject selected site and app password support availability to their networks.Testing steps
Testing information
Screenshots
N/A
RELEASE-NOTES.txt
if necessary.