Forum post_status switch from wpdb->update to wp_update_post

[nofearinc]

Forum post_status update has been applied with $wpdb->user which is not the best practice as some conditionals are being ignored that normally happen in wp_update_post. Additionally, wp_update_post is used in several other areas of the website.

The transient switch is also turned off now as it is called by wp_insert_post which is triggered at the end of wp_update_post.

[nofearinc]

Migrate from wpdb->update to wp_update_post

[nofearinc]

Patch applied for includes/forums/functions.php

[johnjamesjacoby]

Can you explain what conditionals are being skipped, and what the bug is?

IIRC, the $wpdb->update() approach is used by design to avoid the additional overhead and potential for filter recursion caused by running through wp_update_post(), wp_insert_post(), et all.

[nofearinc]

Two things:

First off, I know update() is faster - it's called from wp_update_post() as well. However, there are lots of filters and actions on the way of wp_update_post, such as:

• filter wp_insert_post_empty_content
• function sanitize_title has filters
• filter wp_insert_post_parent
• function wp_unique_post_slug has various filters
• filter wp_insert_post_data
• action pre_post_update
• clean_post_cache - function that takes care of caching + various actions

I'm also not sure that save_post and wp_insert_post are fired in wpdb->update (I believe they're skipped).

I'm personally using some of these in WP plugins to take care of additional meta data, dynamic template attachment 'on the fly', tracking (for stats) in other tables and translating Cyrillic symbols to Latin.

Second, there are other wp_update_post() calls in the plugin and if you disagree with the first point, then probably they should also be translated to update() calls for consistency.

[johnjamesjacoby]

Relpaces all $wpdb->update() usages, for testing actions/filters, and transition_post_status() changes

[johnjamesjacoby]

In the patch attached, I'm worried that wp_transition_post_status() is going to get called too late. It uses wp_update_post(), but I'm still not convinced it's the correct approach without doing extensive testing.

[nofearinc]

I'm worried that wp_transition_post_status() is going to get called too late

Do you think that this would be a problem? In the original code it's called after the update, I'm not sure how late would it be when migrating to wp_update_post - can you think of a problematic use case as I've been using this for a few days now with no issues so far?

[johnjamesjacoby]

Replying to nofearinc:

Do you think that this would be a problem? In the original code it's called after the update, I'm not sure how late would it be when migrating to wp_update_post - can you think of a problematic use case as I've been using this for a few days now with no issues so far?

It might be, yes. We specifically pass the old visibility value into those functions, and use it when we manually call wp_transition_post_status(). I think this is because we're already likely calling wp_update_post() farther up the stack, and are relying on the original post_status being passed in.

[johnjamesjacoby]

In 4992:

Use wp_update_post() in bbp_update_reply_position(), instead of $wpdb->update(). Ensures that post caches are cleared when manually updating a reply position via its menu_order property. See #2351.

[johnjamesjacoby]

Bumping this to 2.5, so we can audit these calls as part of 2.5's emphasis on performance.

[johnjamesjacoby]

In 5060:

Normalize forum, topic, and reply dropdown form fields and associated functions. Includes several new functions with _get_ equivalents to replace incorrectly named _select() functions.

First pass at adding custom topic status handling. Props jkudish. See #2187.

Also replaces $wpdb->update() calls with wp_update_post() in associated functions. Fixes #2351.