-
Notifications
You must be signed in to change notification settings - Fork 3
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
Sync post status #42
Sync post status #42
Conversation
@adekbadek The changes look good and work fine, but I want to ask you opinion on whether we should this syncing in such a broad way. The feature request mentioned specifically the use case of trashing a post. I'm afraid that if we do it in this way we might introduce some unintentional behaviors and inconsistencies. Looking at distributor repo, you can see that this is not a simple topic, and that's why they don't support it yet. Look the description in this issue: And then see that there's a PR that never got merged 10up/distributor#446 So it seems to me that there might be some use cases we are not accounting for and simply syncing the post status in this way might break some editorial flows. I can't really think of anything but I would feel more comfortable if we only covered what we know is an issue in the current workflow, which is trashing the post (and maybe restoring?). WDYT? |
ah, and I pushed one little fix needed after all the code was put inside a class |
@leogermani – I've added a check so that only |
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.
Thanks!
# [1.2.0-alpha.2](v1.2.0-alpha.1...v1.2.0-alpha.2) (2024-02-15) ### Bug Fixes * assorted error handling fixes ([#52](#52)) ([234f883](234f883)) * dynamic class property deprecated warnings ([#47](#47)) ([693865c](693865c)) * prevent coauthors capability check infinite loop ([#46](#46)) ([9565cef](9565cef)) * set Yoast primary category ([#41](#41)) ([3457d19](3457d19)) ### Features * sync billing and shipping addresses ([#50](#50)) ([6a05580](6a05580)) * sync publish, trash post statuses ([#42](#42)) ([fd5d8b9](fd5d8b9))
🎉 This PR is included in version 1.2.0-alpha.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [1.2.0](v1.1.0...v1.2.0) (2024-02-20) ### Bug Fixes * assorted error handling fixes ([#52](#52)) ([234f883](234f883)) * **data-listeners:** handle no user in wc data update ([4eeefc0](4eeefc0)) * dynamic class property deprecated warnings ([#47](#47)) ([693865c](693865c)) * prevent coauthors capability check infinite loop ([#46](#46)) ([9565cef](9565cef)) * race condition when creating a Node ([1946473](1946473)) * set Yoast primary category ([#41](#41)) ([3457d19](3457d19)) ### Features * **ci:** add epic/* release workflow and rename `master` to `trunk` ([#39](#39)) ([9cee51d](9cee51d)) * sync billing and shipping addresses ([#50](#50)) ([6a05580](6a05580)) * sync publish, trash post statuses ([#42](#42)) ([fd5d8b9](fd5d8b9))
🎉 This PR is included in version 1.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
All Submissions:
Changes proposed in this Pull Request:
Adds post status synchronization. The functional change itself is small (ffec109), most of the diff is a refactor converting the global/base Distributor hooks file to a class (af03b03).
See 1205909204837811-as-1206399433499327/f
How to test the changes in this Pull Request:
Other information: