-
Couldn't load subscription status.
- Fork 16
Rewrite WordPressOrgRestApi
#724
Changes from all commits
f545297
b01de44
6f10cf4
535f3d4
26e7117
913a32e
52fb701
660f8b2
a7867c5
5d1f6ba
4777462
614daf8
5e77bcf
4f4b6b9
49f177f
a65b40b
c44bb13
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,8 @@ | ||
| import Foundation | ||
|
|
||
| public class BlockEditorSettingsServiceRemote { | ||
| let remoteAPI: WordPressRestApi | ||
| public init(remoteAPI: WordPressRestApi) { | ||
| let remoteAPI: WordPressOrgRestApi | ||
| public init(remoteAPI: WordPressOrgRestApi) { | ||
| self.remoteAPI = remoteAPI | ||
| } | ||
| } | ||
|
|
@@ -11,30 +11,15 @@ public class BlockEditorSettingsServiceRemote { | |
| public extension BlockEditorSettingsServiceRemote { | ||
| typealias EditorThemeCompletionHandler = (Swift.Result<RemoteEditorTheme?, Error>) -> Void | ||
|
|
||
| func fetchTheme(forSiteID siteID: Int?, _ completion: @escaping EditorThemeCompletionHandler) { | ||
| func fetchTheme(completion: @escaping EditorThemeCompletionHandler) { | ||
| let requestPath = "/wp/v2/themes" | ||
| let parameters: [String: AnyObject] = ["status": "active" as AnyObject] | ||
| let modifiedPath = remoteAPI.requestPath(fromOrgPath: requestPath, with: siteID) | ||
| remoteAPI.GET(modifiedPath, parameters: parameters) { [weak self] (result, _) in | ||
| guard let `self` = self else { return } | ||
| switch result { | ||
| case .success(let response): | ||
| self.processEditorThemeResponse(response) { editorTheme in | ||
| completion(.success(editorTheme)) | ||
| } | ||
| case .failure(let error): | ||
| completion(.failure(error)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private func processEditorThemeResponse(_ response: Any, completion: (_ editorTheme: RemoteEditorTheme?) -> Void) { | ||
| guard let responseData = try? JSONSerialization.data(withJSONObject: response, options: []), | ||
| let editorThemes = try? JSONDecoder().decode([RemoteEditorTheme].self, from: responseData) else { | ||
| completion(nil) | ||
| return | ||
| let parameters = ["status": "active"] | ||
| Task { @MainActor in | ||
| let result = await self.remoteAPI.get(path: requestPath, parameters: parameters, type: [RemoteEditorTheme].self) | ||
| .map { $0.first } | ||
| .mapError { error -> Error in error } | ||
| completion(result) | ||
| } | ||
| completion(editorThemes.first) | ||
| } | ||
|
|
||
| } | ||
|
|
@@ -43,30 +28,18 @@ public extension BlockEditorSettingsServiceRemote { | |
| public extension BlockEditorSettingsServiceRemote { | ||
| typealias BlockEditorSettingsCompletionHandler = (Swift.Result<RemoteBlockEditorSettings?, Error>) -> Void | ||
|
|
||
| func fetchBlockEditorSettings(forSiteID siteID: Int?, _ completion: @escaping BlockEditorSettingsCompletionHandler) { | ||
| let requestPath = "/wp-block-editor/v1/settings" | ||
| let parameters: [String: AnyObject] = ["context": "mobile" as AnyObject] | ||
| let modifiedPath = remoteAPI.requestPath(fromOrgPath: requestPath, with: siteID) | ||
|
|
||
| remoteAPI.GET(modifiedPath, parameters: parameters) { [weak self] (result, _) in | ||
| guard let `self` = self else { return } | ||
| switch result { | ||
| case .success(let response): | ||
| self.processBlockEditorSettingsResponse(response) { blockEditorSettings in | ||
| completion(.success(blockEditorSettings)) | ||
| func fetchBlockEditorSettings(completion: @escaping BlockEditorSettingsCompletionHandler) { | ||
| Task { @MainActor in | ||
| let result = await self.remoteAPI.get(path: "/wp-block-editor/v1/settings", parameters: ["context": "mobile"], type: RemoteBlockEditorSettings.self) | ||
| .map { settings -> RemoteBlockEditorSettings? in settings } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be a change for later on, but it seems like the only reason the If so, and since we're making breaking changes, I wonder whether we could make the type in the completion typealias non-optional. It would remove this step, and I suspect also simplify caller code by removing the need to deal with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not just for
If the JSON response contains all the properties declared in the Swift type, this function of course should return a model instance. However, if this API returns 200 with valid "block editor settings" JSON response, which does not contains all the non-optional proprieties declared in the That's the behaviour of the existing function. And the refactored implementation (mainly the line here and the code below that maps decoding error to a nil successful result) maintains the same runtime behaviour for backward compatibility. |
||
| .flatMapError { original in | ||
| if case let .unparsableResponse(response, _, underlyingError) = original, response?.statusCode == 200, underlyingError is DecodingError { | ||
| return .success(nil) | ||
| } | ||
| return .failure(original) | ||
| } | ||
| case .failure(let error): | ||
| completion(.failure(error)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private func processBlockEditorSettingsResponse(_ response: Any, completion: (_ editorTheme: RemoteBlockEditorSettings?) -> Void) { | ||
| guard let responseData = try? JSONSerialization.data(withJSONObject: response, options: []), | ||
| let blockEditorSettings = try? JSONDecoder().decode(RemoteBlockEditorSettings.self, from: responseData) else { | ||
| completion(nil) | ||
| return | ||
| .mapError { error -> Error in error } | ||
| completion(result) | ||
| } | ||
| completion(blockEditorSettings) | ||
| } | ||
| } | ||
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.
Here's a minor nitpick / curiosity. I have a thing for type names using acronyms, like
URLinstead ofUrlorJSONDecoderinstead ofJsonDecoder.How would this look like if it were
WordPressORGRESTAPI? Too many uppercase letters? 😳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'd probably use
Orginstead ofORG. But this PR simply reuses the existing name 😄 .