WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 7 weeks ago

#20299 new defect (bug)

Preview changes on a published post makes all post meta "live"

Reported by: jakemgold Owned by:
Milestone: Future Release Priority: normal
Severity: major Version: 3.3.1
Component: Revisions Keywords: editorial-flow has-patch needs-testing
Focuses: Cc:

Description

Here's the use case. Client wants to preview an update to a published post (as the Preview Changes button correctly implies they can). This post has some important post meta that impacts that preview.

Here's the problem - because post meta is not saved to a revision (it looks for the "real" post), when the preview button is pressed, save_post runs, and saves the meta data to the real, published post, even though the user only intends to preview the change.

Without realizing it, the user has updated the published version. That can be prevented by not saving post meta to revisions (when using custom save_post hooks), but then there's no non-hacky way to actually preview the full changes.

I believe this bug has been present for a while, we just rarely use the Preview function on published posts, and when we do, probably never tested it with critical post meta.

Attachments (3)

20299.patch (2.0 KB) - added by WraithKenny 13 months ago.
20299-2.patch (1.9 KB) - added by WraithKenny 13 months ago.
20299-test.diff (1.5 KB) - added by adamsilverstein 3 months ago.
test saving post meta to autosave wp_create_post_autosave

Download all attachments as: .zip

Change History (48)

comment:1 johnbillion2 years ago

Could you show the code you're using to save your post meta?

comment:2 alexkingorg2 years ago

  • Cc alexkingorg@… added

comment:3 jkudish2 years ago

  • Cc joachim.kudish@… added

comment:4 ryanduff2 years ago

  • Cc ryan@… added

comment:5 jaredatch2 years ago

  • Cc jared@… added

comment:6 bookwyrm2 years ago

  • Cc bookwyrm added

This is being done by the update_post_meta function:

    // make sure meta is added to the post, not a revision
    if ( $the_post = wp_is_post_revision($post_id) )
        $post_id = $the_post;

Why wouldn't you want the postmeta data saved to the revision?

comment:7 mintindeed2 years ago

  • Cc gabriel.koen@… added

comment:9 coocoomoo22 months ago

  • Cc coocoomoo added

comment:10 ryanduff20 months ago

So instead of updating the actual post_meta values with each revision, how about it saves an array of the post meta in one row of the post_meta table with the meta key being something like _revision and the id being that of the revision and not of the actual post?

The preview could work with that since you're essentially viewing that revision and we could grab the meta array for that and expand it out. The actual post would continue to use the unmodified entries in the post_meta table. On save the values would finally be pushed to the respective entries in post_meta.

The other option is a full blown post meta revisions system that Alex King proposed over in #20564

Either way, one route or the other needs to be picked before something can be implemented.

comment:11 DrewAPicture20 months ago

  • Cc xoodrew@… added

comment:12 jakemgold19 months ago

  • Version 3.3.1 deleted

I think some branches (pun intended) need to be shaken on this one. I think this is in many ways a serious bug, at least if we want to have a serious Preview feature.

Solution proposed by @ryanduff seems a bit janky to me, and risky (size wise) if a significant amount of post meta is stored.

I think @bookwyrm solution is where my mind is going - it's not immediately clear to me at all why that block of code that prevents post meta from saving to revisions is in there (preventing database clutter... ?).

Like the deeper solution @alexkingorg is proposing in his related ticket, but not sure it even has to be that involved.

comment:13 jakemgold19 months ago

  • Version set to trunk

comment:14 SergeyBiryukov19 months ago

  • Version changed from trunk to 3.3.1

Version number indicates when the bug was initially introduced/reported.

comment:15 mintindeed19 months ago

You'd probably want to exclude some post meta that are specific to the main post, like edit locks. I imagine the patch would look something like this:

$exclude_meta_keys = apply_filters( 'revision_meta_do_not_copy', array(
	'_encloseme',
	'_pingme',
	'_edit_last',
	'_edit_lock'
) );
$current_meta = get_post_custom( $GLOBALS['post'] );
if ( is_array( $current_meta ) ) {
	foreach ( $current_meta as $meta_key => $meta_value ) {
		if ( in_array( $meta_key, $exclude_meta_keys ) ) {
			continue;
		}
		
		update_post_meta( $revision_id, $meta_key, $meta_value );
	}
}

comment:16 follow-up: wonderboymusic19 months ago

You want to ignore everything starting with _
Those are supposed to be "private"

comment:17 in reply to: ↑ 16 jakemgold19 months ago

Replying to wonderboymusic:

You want to ignore everything starting with _
Those are supposed to be "private"

This isn't quite right.

First, it seems to me that some keys (like thumbnail id) should be revision specific (although this might open up another can of worms - I find it disturbing that selecting a new thumbnail immediately updates a published post without clicking "update").

Second, many developers use the prefix to prevent meta data from showing up int he free form "custom fields" meta box.

I think @mintindeed is more on the right track, explicitly ignoring certain keys, in a filterable way.

Are there any other keys we're missing that need to be attached to the "real" post?

comment:18 jkrill17 months ago

  • Cc jkrill added

comment:19 aaroncampbell16 months ago

  • Cc aaroncampbell added

comment:20 sennza16 months ago

  • Cc bronson@… added

comment:21 jb51015 months ago

  • Cc jbrown510@… added

comment:22 nacin15 months ago

  • Component changed from General to Revisions
  • Milestone changed from Awaiting Review to 3.6

This and #20564 would block #23314.

comment:23 adamsilverstein15 months ago

  • Cc adamsilverstein@… added

comment:24 danielbachhuber15 months ago

  • Cc danielbachhuber added
  • Keywords editorial-flow added

comment:25 beaulebens15 months ago

  • Cc beau@… added

comment:26 goto1015 months ago

  • Cc dromsey@… added

comment:27 kwight14 months ago

  • Cc kwight@… added

comment:28 WraithKenny13 months ago

  • Cc Ken@… added

comment:29 WraithKenny13 months ago

#20564 blocks this ticket, but if/when that goes in, the show preview functionality would have to somehow pull the meta from the $preview instead of the $post :

In revision.php -> function _set_preview($post)... maybe set $post->preview_id = $preview->ID (or use a global) then set up a filter on 'get_post_metadata' to pull out the right data.

comment:30 WraithKenny13 months ago

  • Keywords has-patch added

rough pass, not tested. Will test when I get home

comment:31 WraithKenny13 months ago

  • Keywords needs-testing added

WraithKenny13 months ago

comment:32 WraithKenny13 months ago

worked with patch from #20564 ('versioned_meta') and the recent 23842.

Last edited 13 months ago by WraithKenny (previous) (diff)

WraithKenny13 months ago

comment:33 WraithKenny13 months ago

v2 uses get_versioned_meta_keys from the patch on #20564

comment:34 follow-up: WraithKenny13 months ago

23862 included the gist of my 20299-2.patch (with an improved 'get_post_metadata filter, nice work kovshenin).

I think making _wp_post_revision_meta_keys() filterable, as well as adding some other core meta to the list (like thumbnail maybe) would solve this finally :).

Last edited 13 months ago by WraithKenny (previous) (diff)

comment:35 in reply to: ↑ 34 SergeyBiryukov13 months ago

Replying to WraithKenny:

I think making _wp_post_revision_meta_keys() filterable, as well as adding some other core meta to the list (like thumbnail maybe) would solve this finally :).

Related: #23973

comment:36 azaozz13 months ago

I think making _wp_post_revision_meta_keys() filterable...

_wp_post_revision_meta_keys() is specific for post formats, won't work for arbitrary meta. Even thinking it should be renamed to _post_format_meta_keys().

comment:37 ocean9011 months ago

  • Milestone changed from 3.6 to Future Release

Let's revisit this in 3.7.

comment:38 husobj11 months ago

  • Cc ben@… added

comment:39 sanchothefat11 months ago

I have some code that gets around this problem for previews that doesn't feel too horrendous. It's a problem that's been really bothering some of our clients who want to preview changes to custom post types (eg. events) with lots of metadata.

Here's the gist: https://gist.github.com/sanchothefat/498ed66a48a4a56d349d

You don't get post meta for each revision rather the preview meta data is stored against the original post with a prefixed meta_key as others have suggested. It gets overridden on each update/preview and you won't end up with orphaned post meta. Works fairly well so far but those sound like famous last words...

comment:40 in reply to: ↑ description yrosen8 months ago

  • Cc yrosen added

comment:41 doughamlin8 months ago

  • Cc doughamlin@… added

comment:42 westonruter6 months ago

  • Cc weston@… added

adamsilverstein3 months ago

test saving post meta to autosave wp_create_post_autosave

comment:43 adamsilverstein3 months ago

As far as I can tell - in trunk - the post meta is not saved to published posts when clicking preview. Testing using the 'Custom Fields' section of the edit post page, the values are not saved until I click publish (when a post is in the draft state, the meta is saved when I click the preview button).

I believe the ticket description is (no longer) accurate when it states "when the preview button is pressed, save_post runs, and saves the meta data to the real, published post".

The bigger, remaining, issue is: "there's no non-hacky way to actually preview the full changes". This is because the wp_create_post_autosave function does not save the post meta - when the preview is loaded from the revision, post meta is pulled from the parent post.

(The function post_preview calls wp_create_post_autosave for published posts, creating an 'autosave' revision, and returns a url for previewing this autosave. )

The current patch for #20564 (Framework for storing revisions of Post Meta) includes _wp_preview_meta_filter to display revisioned post_meta fields with the preview. Landing #20564 will go a long way towards fixing this and #23314.

comment:44 azaozz3 months ago

The "gist" of the problem is that clicking "Preview" submits the whole form, i.e. quite similar to clicking either Save or Update or even Publish, without changing the post_status.

For drafts this works well. It is equal to clicking Save Draft + View Post in a new tab. All data from all form fields (including those added by plugins) is saved.

However for published posts this is not ideal. The filters that run on updating and inserting posts can trigger all sorts of related tasks and many plugins are not aware that the post is not being updated, only previewed.

This needs standardizing. Thinking that best would be to limit what is being submitted on clicking "Preview" and "Preview Changes" only to post_title, post_content and excerpt. Then add a filter for additional form fields that will be saved and included in the preview. This could work by filtering $_POST. Even maybe set DOING_AUTOSAVE or perhaps DOING_PREVIEW.

Last edited 3 months ago by azaozz (previous) (diff)

comment:45 ircbot7 weeks ago

This ticket was mentioned in IRC in #wordpress-dev by danielbachhuber. View the logs.

Note: See TracTickets for help on using tickets.