-
Notifications
You must be signed in to change notification settings - Fork 117
Fix/1222 log post author change #1549
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: develop
Are you sure you want to change the base?
Conversation
Trying to catch everything that generated a log before but teasing them apart to get more data.
…is not update and only log post status update when it is not update
connectors/class-connector-posts.php
Outdated
| public $actions = array( | ||
| 'transition_post_status', | ||
| 'deleted_post', | ||
| 'post_updated', |
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.
💡 thought: I believe this will not log new posts creation because post_updated is only called for existing posts.
It may be a good thing because I think we don't need to log e.g. a post_title change for a new post. However, without transition_post_status callback we no longer log new posts creation. I guess we could use e.g. save_post action or bring back transition_post_status but limit its scope to only log transitions to the publish status.
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.
@delawski $update is a check on whether or not the data has a post_id which gets set immediately in the editor but ... if someone is using wp_insert_post() directly to add a post then it won't so I need to add that in, thank you!
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.
@delawski I've changed this to use wp_after_insert_post -- it's noisier but because of the orders of the filters in the REST API, it's the one that works there as well as in the Quick Edit section.
I still need to test and add automated tests though!
Fixes #1222
This changes the hook used for watching for post updates to
post_updatedand usesset_object_termsto watch for term changes. The idea is that everything which triggered or should have triggered a log previously still triggers a log but with more information.The strings in the summary are built separately and then the data itself is stored in an associative array in the meta table. (nb: this should be a JSON object for cross language portability I think)
Currently tests are failing as I'm not convinced this is completely the right approach.
Checklist
contributing.md).Release Changelog