Skip to content
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

Extend RemotePostService with new create and patch endpoints #743

Merged
merged 11 commits into from
Mar 14, 2024

Conversation

kean
Copy link
Contributor

@kean kean commented Mar 6, 2024

Description

This PR adds PostServiceRemoteExtended with new APIs for creating and patching posts:

public protocol PostServiceRemoteExtended: PostServiceRemote {
     func createPost(with parameters: RemotePostCreateParameters) async throws -> RemotePost
     func patchPost(withID postID: Int, parameters: RemotePostUpdateParameters) async throws -> RemotePost
 }

In the previous implementation, you had to use the response object (RemotePost) as an input parameter for updating the posts, which was less than ideal because of the mismatch between the set of available parameters and their format:

- (void)updatePost:(RemotePost *)post success:(void (^)(RemotePost *post))success failure:(void (^)(NSError *error))failure;

The new parameter formats also support creating diffs between two RemotePostCreateParameters and applying a diff to RemotePostCreateParameters. Both of these features are used by the updated PostRepository in WP-iOS.

Testing Details

See wordpress-mobile/WordPress-iOS#22807


  • Please check here if your pull request includes additional test coverage.
  • I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

@kean kean force-pushed the task/update-post-endpoints branch 2 times, most recently from af00e09 to cce07f0 Compare March 9, 2024 16:57
@kean kean force-pushed the task/update-post-endpoints branch from cce07f0 to 39e9dac Compare March 9, 2024 17:02
@kean kean changed the title Add RemotePost.ifNotModifiedSince Extend RemotePostService with new create and patch endpoints Mar 9, 2024
@kean kean force-pushed the task/update-post-endpoints branch from df4427f to 9033e9e Compare March 9, 2024 18:01
@kean kean force-pushed the task/update-post-endpoints branch from 9033e9e to 32840bd Compare March 9, 2024 18:02
@dangermattic
Copy link
Collaborator

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@kean kean force-pushed the task/update-post-endpoints branch from 63c8712 to f836925 Compare March 10, 2024 15:15
@kean kean force-pushed the task/update-post-endpoints branch 2 times, most recently from 4077149 to 9ab4b43 Compare March 10, 2024 20:28
@kean kean force-pushed the task/update-post-endpoints branch from e65b4c3 to a7fc344 Compare March 11, 2024 15:05
@kean kean marked this pull request as ready for review March 11, 2024 15:36
@kean kean force-pushed the task/update-post-endpoints branch from 9501e28 to 44309e7 Compare March 11, 2024 15:40
@kean kean requested a review from momo-ozawa March 11, 2024 15:41
Copy link
Contributor

@momo-ozawa momo-ozawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crazytonyli
Copy link
Contributor

👋 Hi @kean, would you mind adding some unit tests?

@kean
Copy link
Contributor Author

kean commented Mar 13, 2024

Hey, we have a suite of integration tests in the following PR. It has complete coverage for this change, including the underlyingError addition, diff, and both the new methods.

@crazytonyli
Copy link
Contributor

They are in a separate repository though. It'd be nice to have complete coverage on these changes in this repo here too.

@kean kean merged commit 32ab230 into trunk Mar 14, 2024
9 checks passed
@kean kean deleted the task/update-post-endpoints branch March 14, 2024 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants