Revisions: add filter before deleting revisions

[jhned]

I'm in a situation where I need to preserve the original post revision and prevent it from being deleted once a post is past the max number of revisions for the site. This is more difficult than it seemed, because WordPress doesn't have many filters around the revisions delete process.

If we add a new filter to the result of wp_get_post_revisions in ​revision.php, then I'd be able to slice out the revision that I don't want to get deleted and instead delete the next-oldest one.

[costdev]

Hi @jhned, thanks for creating this ticket!

Would hooking the ​the_posts filter suit your needs? The first parameter is the list of posts (or revisions in this case), and the second is the query object.

I'm thinking something along the lines of this:

<?php

add_filter( 'the_posts', 'myprefix_skip_original_revision', 10, 2 );

function myprefix_skip_original_revision( $revisions, $query ) {
    if ( 0 < $query->get( 'post_parent' ) && 'revision' === $query->get( 'post_type' ) ) {
        array_shift( $revisions );
    }

    return $revisions;
}

[jhned]

Hi @costdev, thanks for the thought. I tried both the_posts and pre_get_posts in coming up with this idea. In pre_get_posts I was able to modify the query and set the "exclude" or "post_not__in" args, but the wp_get_post_revisions call still returned all the revisions. With the_posts, for some reason it wasn't hitting my breakpoint for the wp_get_post_revisions call; I tried your example as well and it's not intercepting the revision deletion process.

When using either pre_get_posts or the_posts, however, we'd have to do extra work to make sure that the query in question is one that's getting ready to delete some revisions after it gets them. Using pre_get_posts or the_posts wouldn't give us that context. We could try debug_backtrace, but that seems like overkill compared to adding a new hook.

[costdev]

Thanks for the confirmation @jhned! I'll submit a patch for testing now.

This PR introduces a new filter for wp_save_post_revision().

'wp_save_post_revision_revisions_before_deletion' passes the revisions to be considered for deletion, and the new revision's post ID.

This allows extenders to exclude specific revisions from being considered for deletion.

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

[audrasjb]

I tested this filter using the following test snippet:

<?php
add_filter(
        'wp_save_post_revision_revisions_before_deletion',
        function( $revisions, $post_id ) {
                $original_revision = get_transient( 'original_revision_for_post_' . $post_id );
                if ( $original_revision ) {
                        // Always remove the oldest revision for the array of revisions to potentially delete.
                        unset( $revisions[ $original_revision ] );
                } else {
                        // Set the oldest revision in a transient, so we can verify that it is always ignored.
                        $original_revision = array_key_first( $revisions );
                        set_transient( 'original_revision_for_post_' . $post_id, $original_revision );
                }
                return $revisions;
        },
        10,
        2
);

I also set WP_POST_REVISIONS constant to 3.

Test report:
Works fine on my side: the oldest revision is never deleted ✅

[adamsilverstein]

Filter and tests look good to me!

[costdev]

Adding for commit consideration 🙂

[audrasjb]

In 55254:

Revisions: Add a way to filter the revisions considered for deletion.

This changeset introduces a new filter for wp_save_post_revision(). wp_save_post_revision_revisions_before_deletion passes the revisions to be considered for deletion, and the new revision's post ID.

This allows extenders to exclude specific revisions from being considered for deletion.

Props jhned, costdev, audrasjb, adamsilverstein, mukesh27.
Fixes #57320.

Committed in https://core.trac.wordpress.org/changeset/55254

[johnbillion]

I don't think there's a need to use a doing it wrong message here, especially as it introduces a new localised string that's practically never going to be used. There's an inherent expectation in WordPress that filters return the correct type, and casting to an array is probably sufficient here. Let's not add unnecessary burden to translators.

[johnbillion]

In fact if $filtered_revisions is not an array then a PHP warning will be triggered further down on lines 232, 238, etc when it's treated as an array.

[johnbillion]

Sorry for the comment spam. $revisions can also only be WP_Post[] so int[] needs to be removed from the doc for the filter.

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

Alright! Thanks John for putting together this PR! 👌

[johnbillion]

In 55406:

Revisions: Remove an unnecessary call to _doing_it_wrong() and corresponding new text string from the implementation of the new wp_save_post_revision_revisions_before_deletion filter.

While the guard condition was technically correct, it's not practical or necessary to provide this protection for every use of every filter, and it adds unnecessary burden to translators to provide translations for strings that will likely not be seen.

Follow up to [55254].

Fixes #57320

https://core.trac.wordpress.org/changeset/55406

[milana_cap]

Added to misc dev note. Draft: ​https://make.wordpress.org/core/?p=103089&preview=1&_ppp=36765ffd5f