Add apiURL parameter for changing WP.com API URL
#731
Conversation
| enum Site { | ||
| case dotCom(siteID: UInt64, bearerToken: String) | ||
| case dotCom(siteID: UInt64, bearerToken: String, apiURL: URL) | ||
| case selfHosted(apiURL: URL, credential: SelfHostedSiteCredential) |
There was a problem hiding this comment.
Nitpick: What do you think of making apiURL the first associated value, so that it's in line with the selfHosted case?
One argument against doing so would be that siteID is more important when it comes to identifying a .com site, so it belongs in first position. I'd be fine with that. This is just a style nitpick / question that I'm not sure on.
There was a problem hiding this comment.
I'd agree with that argument, considering 'apiURL' is an optional parameter in the initialiser. 😄
| additionalHeaders["User-Agent"] = userAgent | ||
| } | ||
| if case let Site.dotCom(siteID: _, bearerToken: token) = site { | ||
| if case let Site.dotCom(siteID: _, bearerToken: token, _) = site { |
There was a problem hiding this comment.
Nitpick: The new _ entry made me realize that the siteID: label is unnecessary
| if case let Site.dotCom(siteID: _, bearerToken: token, _) = site { | |
| if case let Site.dotCom(_, bearerToken: token, _) = site { |
Site note: I looked for a SwiftLint rule for this but did not find one that would have picked that up. At first, I browsed the rules docs. Then, I tried with
cat WordPressKit/WordPressOrgRestApi.swift \
| ./Pods/SwiftLint/swiftlint lint --quiet --enable-all-rules --use-stdin \
| sort -v
| case .dotCom: | ||
| return URL(string: "https://public-api.wordpress.com")! | ||
| case let .dotCom(_, _, apiURL): | ||
| return apiURL |
There was a problem hiding this comment.
A nice side effect is that now we are one step closer to having a single source of truth for what this URL should be.
| static func dotCom(siteID: UInt64, bearerToken: String) -> Self { | ||
| .dotCom(siteID: siteID, bearerToken: bearerToken, apiURL: WordPressComRestApi.apiBaseURL) | ||
| } | ||
| } |
There was a problem hiding this comment.
Note for future readers: This method looks unused in the diff, but it's actually a way to bridge the new implementation, which expects an apiURL parameter, with the existing code that did not have it.
See for example line 103 in this file.
2 participants
Description
This PR brings back support of mocking WP.com .org REST API. See wordpress-mobile/WordPress-iOS#22612 (review) for context in the WordPress app.
Testing Details
I have added one unit test. Let me know if you think there are more can be added.
CHANGELOG.mdif necessary.