Consolidate post insertion APIs

[nacin]

In wp-includes, we have:

• wp_insert_post()
• wp_insert_attachment()
• wp_update_post()
• wp_publish_post()

For saving from the admin, we have:

• edit_post()
• wp_write_post()
• write_post()

wp_publish_post() is, as of [21942], now wraps wp_insert_post().

wp_update_post() is a fairly mundane wrapper of wp_insert_post(), but we really should eliminate the differences between the two functions and make it a straight-up wrapper.

wp_insert_attachment() was a fork of wp_insert_post(), and wp_insert_post() has gotten a lot of improvements that haven't reached wp_insert_attachment(). It doesn't take much to merge these two, though, and make wp_insert_attachment() a wrapper.

wp_write_post() calls edit_post() if it has a post ID. And since we have had a post ID since the days of auto-drafts, this function is dead code. It's wrapper, write_post(), can also be deprecated.

I'm attaching a patch that takes care of wp_insert_post(), wp_write_post(), and write_post(). wp_update_post() will require a bit more concentration.

Needs testing and unit tests.

[nacin]

See #11399. If [21942] needs to be reverted, we can do that here. See also [4077] #2737 #2715 #4620 for some history.

[dd32]

#22063 was marked as a duplicate.

[nacin]

#22047 was marked as a duplicate.

[nacin]

In [22189]:

Use wp_update_post() rather than wp_insert_post() in wp_publish_post() to avoid stomping of values like categories. props ericmann, fixes #22167. see #21963.

[ericlewis]

Freshness for 21963.diff, applies cleanly to trunk now.

[nacin]

​http://wordpress.org/support/topic/publish-scheduled-posts-in-35?replies=1#post-3558447

[nacin]

In 23206:

Revert [21942] and have wp_publish_post() deal with the database directly. clean_post_cache() is now also called directly due to [21943].

fixes #22944 for trunk.
Unit tests: [1174/tests].

see #11399. see #21963.

[nacin]

wp_insert_attachment() sometimes inserts a null GUID, see #18310.

[SergeyBiryukov]

Related: #23399

[helen]

#23399 was marked as a duplicate.

[wonderboymusic]

21963.3.diff​ makes wp_insert_attachment() a wrapper for wp_insert_post(). I am fairly confident all is good, plus I messed with them both a lot last week during #22400, but I will wait for someone else to chime in.

All unit tests pass.

[DrewAPicture]

O_o purdy.

21963.4.diff​ updates the inline docs for wp_insert_attachment() and changes the $object parameter to $args since we're basically overwriting the function anyway.

[wonderboymusic]

In 28579:

Combine wp_insert_attachment() and wp_insert_post(). wp_insert_attachment() becomes a wrapper. Update inline docs.

Props wonderboymusic, DrewAPicture.
See #21963.

[kpdesign]

Restore hook docs removed in r28579

[kpdesign]

21963.5.diff​ restores the hook docs for wp_insert_attachment_data removed in r28579, and moves the hook docs for wp_insert_post_data to the proper place.

[wonderboymusic]

In 28582:

Fix some inline docs churn in wp_insert_post() after [28579].

Props kpdesign.
See #21963.

[nacin]

In 28601:

Revert [28579] and [28582]. see #21963.

[nacin]

This broke stuff. Specifically, was causing major issues with attachment metadata. I went with a revert for now as I previously looked at merging these functions and I was having all sorts of problems, so I suspect it is not an easy/quick fix.

[wonderboymusic]

Killer - what specifically exploded? We should add unit tests for whatever it is. Every minor change I made to those functions caused like 500 tests to fail, but everything looked good in the end.

[kovshenin]

Replying to wonderboymusic: Patch + unit tests incoming.

[kovshenin]

It looks like update_attached_file() runs when updating the attachment, like adding a caption from the media modal, hence the _wp_attached_file meta value is gone. In wp_insert_attachment() we checked file with if ( $file ) before calling update_attached_file() which was false for updates, but in wp_insert_post() that became isset() which is always true because wp_insert_attachment() always sets the file argument.

I attached two patches. 21963.6.diff​ is against r28600 (before the revert) which shows what went wrong + a unit test. And 21963.7.diff​ which reverse-merges r28601 and applies .7.diff.

[wonderboymusic]

Please review at your leisure. Looks good to me, but I want it to be your fault next time.

[wonderboymusic]

kovshenin: what is your level of comfort with your latest patch?

[kovshenin]

Not sure what you mean, my patch is just empty vs. isset, which is pretty obvious and the first thing that broke. I haven't thoroughly tested the rest of it.

[wonderboymusic]

The original issue when we dropped this in last time was obvious once @kovshenin found it, he fixed it, and tests were added.

In .8.diff​:

• Use @kovshenin's patch ---> I tested it
• Moved the guid logic outside of the attachment if/else - it should never be empty as per #18310. This fixes a unit test that would have broken when this ticket was marked closed.
• Updated the unit test in Tests_Media::test_wp_prepare_attachment_for_js() to account for url no longer being empty - I can't imagine a scenario where this was ever desired

Let's try this again.

[wonderboymusic]

In 28788:

Reinstate the changes from [28579] with some adjustments:

• Check ! empty( $postarr['file'] ) before calling update_attached_file()
• Add a unit test: test_update_attachment_fields()
• Run the same logic for empty guid for attachments that always ran in wp_insert_post(), as per #18310. This fixes a unit test that would have broken when this ticket was marked closed.
• Updated the unit test in Tests_Media::test_wp_prepare_attachment_for_js() to account for url no longer being empty

Props kovshenin, wonderboymusic.
See #21963.

[helen]

Any issues now, a week in?

[helen]

Closing as fixed, then.