`wp_insert_post` doesn't save revision for new published post

[JamesDiGioia]

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.

[adamsilverstein]

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:

• use wp_insert_post to create an initial post and get the the post ID
• use wp_update_post to insert your post data

[JamesDiGioia]

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.

[georgestephanis]

#39011 was marked as a duplicate.

[georgestephanis]

Unit test attached, demonstrating the issue.

This affects Core as of 4.7-RC1 with Custom CSS losing its first revision.

[adamsilverstein]

Replying to georgestephanis:

Unit test attached, demonstrating the issue.

Thanks for the unit test @georgestephanis, thats a great start towards solving this issue.

[adamsilverstein]

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.

[adamsilverstein]

In 30854.diff​:

• Hook wp_save_post_revision on to new_to_publish in addition to update_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.

[westonruter]

In 39477:

Customize: Ensure a custom_css post insertion gets an initial post revision.

Props georgestephanis, westonruter.
See #30854, #38672, #35395.
Fixes #39032.

[hellofromTonya]

Replying to adamsilverstein:

In 30854.diff​:

• Hook wp_save_post_revision on to new_to_publish in addition to update_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?

[Mista-Flo]

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

[Mista-Flo]

Refresh

[Mista-Flo]

Clean

[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.

[Mista-Flo]

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?