`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?

[Clorith]

I had a look at 30854.3.diff​, which appears to add a post revision when a post gets the status publish`.

Cross referencing a bbPress ticket here (​https://bbpress.trac.wordpress.org/ticket/3389), which is a big scenario which needs to account for existing content as well.

The current behavior is that the first edit of an entry in a custom post type overwrites the initial post content entirely in the revision history, and then creates an autosave, completely removing any knowledge of the very first post made.

By just adding a filter to when a post gets the publish status, would this not be limiting the fix to new posts, not existing published ones?

[adamsilverstein]

By just adding a filter to when a post gets the publish status, would this not be limiting the fix to new posts, not existing published ones?

Right, this would only solve the case for newly (programmatically) published posts.

Cross referencing a bbPress ticket here (​https://bbpress.trac.wordpress.org/ticket/3389), which is a big scenario which needs to account for existing content as well.

The current behavior is that the first edit of an entry in a custom post type overwrites the initial post content entirely in the revision history, and then creates an autosave, completely removing any knowledge of the very first post made.

Can you test this patch with bbPress to see if it fixes the issue you linked?

Trac ticket: https://core.trac.wordpress.org/ticket/30854

[adamsilverstein]

Hey @Clorith can you please review the latest patch to confirm it fixes the bbPress issue you linked?

Running tests on this PR: ​https://github.com/WordPress/wordpress-develop/pull/1336

[Clorith]

Applied patch from ​PR 1336 and it does create appropriate revisions for _new_ topics that area created, but as indicated in my initial reply, it does not account for _existing_ posts, which is part of the bigger issue here.

Since revisions are enabled per post type with the supports section of register_post_type, I would expect the fix here to work for existing and new content alike when revisions is added as a supported feature of the given post type.

--

For anyone else looking to work on, and test this, the steps to test it (may be of interest to @hellofromTonya and the testing efforts she's heading up):

To start off, you need a test site with the ​bbPress plugin installed, using it for testing, since it has an interface, and is part of the WordPress project.

Some additional information, this bug is only applicable when _not_ using the post editor in the wp-admin back-end, this is why using bbPress is convenient, since it calls the functions from the front-end and as such is able to replicate the bug reliably from a UI point of view as well.

Create a Forum, since we need a place to create the topics.

Throughout testing I will be using primarily two texts:

Original content:

• Title: Test topic #N (increasing the number with each test I make)
• Post content: Post with initial content

Edited content:

• Title: (no change)
• Post content: Post with modified content

Now visit the front-end of your forum, create a new topic with the Original content from above.

After editing, click the Edit link in the forum topic, and replace your content with the Edited content from above.

If you visit the back-end and look in the Topics post type, you should see your new topic, but if you open it for editing in the back-end, no revisions section will show up, as there are no revisions yet.

In the front-end, edit the post a third time, this time enter any text you like, at this point it does not matter what is used, as we have our two baseline texts above.

Visit the back-end again and look at your topic, this time you will see revisions show up, but if you click in to look at them, you will see that there is one revision less than the total amount of edits and the original creation combined, and if you browse the revisions, you will not find the Post with initial content string, as the entry with Post with modified content is considered the first post.

After the patch is applied, if you complete the same steps above, you should see the correct amount of revisions (the total amount of edits you made plus one for the initial topic), and you should also see that the very first revision is the Post with initial content text.

[Clorith]

I should mention that for testing it for existing content, I created a new topic in the front-end first, then I applied the patch and performed an edit.

The expected scenario is that my content from the Original content should then show up in the revisions after the patch is applied, even if my topic was created without the patch.

The actual result at this time is the same as if the patch had not been applied, and the Original content is lost upon editing.

[hellofromTonya]

Thanks for pinging me @Clorith. Adding this one to the testing queue. Marking it as needs-testing. Will also run through the unit tests.

Sorry this didn't get done before today as today. As today 5.8 Beta 1 day and there's testing to is 5.8 Beta 1. Ran out of time for it to land in 5.8. Punting to 5.9 (instead of Future Release) as it's very close to landing.