Opened 6 years ago
Last modified 11 days ago
#30854 assigned defect (bug)
`wp_insert_post` doesn't save revision for new published post
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.8 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Revisions | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
Calling wp_insert_post
with post_status
of publish
or any other status on a new post doesn't save a new revision.
The reason for this is the $update
check at the beginning:
https://core.trac.wordpress.org/browser/tags/4.1/src/wp-includes/post.php#L3085
checks whether or not the args include an ID, indicating that we're updating a post. Further down, that update check is what calls post_updated
https://core.trac.wordpress.org/browser/tags/4.1/src/wp-includes/post.php#L3473
which wp_save_post_revision
is hooked to. Therefore, if we call wp_insert_post
in a plugin to insert a post with apost_status
of publish
, WordPress doesn't save a post revision.
Post revisions are supposed to be up-to-date with the latest post data, but we don't get a revision if we insert a post in this manner. I think the reason we ended up with this is to avoid saving a revision of the auto-draft that is created when opening up post-new.php, but that isn't necessary now, as wp_save_post_revision
checks whether the post_status
is an auto-draft.
I think the easiest solution is to move the wp_save_post_revision
to the save_post
hook, but I'm not sure if there are other places in the codebase this would cause issues.
Attachments (5)
Change History (24)
#2
@
6 years ago
- Keywords needs-patch added
- Owner set to adamsilverstein
- Status changed from new to assigned
#3
@
6 years ago
The only other thing I can think of is to change the $update
variable to be true
if it either has an ID or the post_status !== auto-draft
. Or if it only == 'publish'
. Does that make more sense?
I ended up working around it without an issue in my own plugin, but I think it's still a bug.
This ticket was mentioned in Slack in #core-customize by ocean90. View the logs.
4 years ago
#7
follow-up:
↓ 8
@
4 years ago
Unit test attached, demonstrating the issue.
This affects Core as of 4.7-RC1 with Custom CSS losing its first revision.
#8
in reply to:
↑ 7
@
4 years ago
Replying to georgestephanis:
Unit test attached, demonstrating the issue.
Thanks for the unit test @georgestephanis, thats a great start towards solving this issue.
#9
@
4 years ago
Still digging into this issue, however I posted test_wp_first_revision_is_not_lost.2.diff to demonstrate this workaround by calling wp_update_post
immediately after creating the post, passing the returned post object, the issue is resolved. I think this actually mirrors what happens when you create a post in the admin.
#10
follow-up:
↓ 14
@
4 years ago
In 30854.diff:
- Hook
wp_save_post_revision
on tonew_to_publish
in addition toupdate_post
- triggering a revision save when a new post is created - Adjust revisions unit test to accommodate change: bump expected revision counts by 1, add first revision to order by comparisons
This needs more testing, especially around creating posts in the admin interface and verifying that we aren't getting duplicate initial revisions.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
3 weeks ago
#14
in reply to:
↑ 10
@
3 weeks ago
Replying to adamsilverstein:
In 30854.diff:
- Hook
wp_save_post_revision
on tonew_to_publish
in addition toupdate_post
- triggering a revision save when a new post is created- Adjust revisions unit test to accommodate change: bump expected revision counts by 1, add first revision to order by comparisons
This needs more testing, especially around creating posts in the admin interface and verifying that we aren't getting duplicate initial revisions.
Hey @adamsilverstein, Can you provide a testing strategy please to give guidance for manual testers to do the testing and provide needed feedback?
#15
@
3 weeks ago
- Keywords has-patch has-unit-tests added; needs-patch removed
I have uploaded two patches.
The first one is just a refresh of the last patch.
The second one also remove the special workaround introduced in 39477
This ticket was mentioned in Slack in #core by paaljoachim. View the logs.
3 weeks ago
#17
follow-up:
↓ 18
@
3 weeks ago
Excellent, thank you @Mista-Flo!
@hellofromTonya testing by hand is tricky from the UI because this ticket deals with a programmatic insertion of a post rather than one created in the UI.
One possible test would to try importing some posts using the WordPress importer. After this patch, imported posts should get an initial revision, better matching posts created in the UI (and something we worked around recently in the revision screen). You will have to look in the DB to see the difference though because we "fake" the initial revision on the revisions screen when it is missing.
A second test would be to review the bug fixed in https://core.trac.wordpress.org/ticket/39032. The fix for that is now reverted in the latest patch, because the new fix here corrects the issue.
#18
in reply to:
↑ 17
@
3 weeks ago
Replying to adamsilverstein:
Excellent, thank you @Mista-Flo!
@hellofromTonya testing by hand is tricky from the UI because this ticket deals with a programmatic insertion of a post rather than one created in the UI.
One possible test would to try importing some posts using the WordPress importer. After this patch, imported posts should get an initial revision, better matching posts created in the UI (and something we worked around recently in the revision screen). You will have to look in the DB to see the difference though because we "fake" the initial revision on the revisions screen when it is missing.
A second test would be to review the bug fixed in https://core.trac.wordpress.org/ticket/39032. The fix for that is now reverted in the latest patch, because the new fix here corrects the issue.
btw, for the removal of #39032 I only removed it from the code, but I did not remove the unit test of that ticket, should we do it as well?
Hey JamesDiGioia!
Thanks for the bug report. Oh thats a problem, I can see why you aren't getting an initial revision for your posts! We didn't think about posts created this way when creating the original post revision.
I'm not sure moving save_post is good, I think that action fires twice on publish. We could try the change (want to try writing a patch for that?) and run our tests, I'd still worry about unintended consequences. Open to other ideas...
In the mean time, a workaround worth trying would be:
wp_insert_post
to create an initial post and get the the post IDwp_update_post
to insert your post data