Unable to trash redirects to temporarily pause them

[kasparsd]

The plugin supports trashing the redirect posts, however the logic evaluating the redirect request doesn't account for the post status at all:

https://github.com/Automattic/WPCOM-Legacy-Redirector/blob/2499ef08c3719986bdc138607faabf2523c02ee7/includes/class-lookup.php#L42-L54

If we do want to support the trashing, the logic needs to be extended to check for the post_status of the resolved $redirect_post. Alternatively, we could disable the trash support and allow hard-deletes.

Please let us know if this needs to be addressed and we can submit a pull request.

[GaryJones]

Rather than trashing, I would love to see a use of draft vs published, as a way to differentiate between inactive and active; that seems to me to be slightly more aligned with the existing features than trashing.

In that case, only publish redirects would be the ones to act on, which is pretty much where you're thinking anyway.

There would be a consideration of back-compat to consider (major release needed?) - since some folks may have already had redirects with various statuses all happily working, then a change here would stop some of those. I'm good with the change, it just needs visibility.

[kasparsd]

That's a great suggestion! Supporting drafts would require additional UX changes as it currently only supports trashing items. There is also no "Edit" screen for individual redirects for updating or drafting items.

If I remember correctly, the UX overall is a recent addition to the plugin which explains the limited scope.

[GaryJones]

If I remember correctly, the UX overall is a recent addition to the plugin which explains the limited scope.

Correct - I'm not sure if the UX ever got released in a tagged release or whether it only lives on the development branch. I think https://github.com/Automattic/WPCOM-Legacy-Redirector/issues/66 was related to this. i.e. a UX had been added, but it wasn't the best approach for enabling things like toggling post status and editing redirects.

[kasparsd]

Rather than trashing, I would love to see a use of draft vs published, as a way to differentiate between inactive and active; that seems to me to be slightly more aligned with the existing features than trashing.

Turns out that all redirect posts are already added as drafts due to lack of specific post type during WPCOM_Legacy_Redirector::insert_legacy_redirect:

https://github.com/Automattic/WPCOM-Legacy-Redirector/blob/dd6affaba45a817230ca150a9ae1fe5df8bbb1f9/includes/class-wpcom-legacy-redirector.php#L146-L160

so a following patch is needed now:

diff --git a/includes/class-lookup.php b/includes/class-lookup.php
index f59f647..94410f5 100644
--- a/includes/class-lookup.php
+++ b/includes/class-lookup.php
@@ -40,7 +40,7 @@ final class Lookup {

        if ( $redirect_post_id ) {
            $redirect_post = get_post( $redirect_post_id );
-           if ( ! $redirect_post instanceof \WP_Post ) {
+           if ( ! $redirect_post instanceof \WP_Post || 'draft' !== get_post_status( $redirect_post ) ) {
                // If redirect post object doesn't exist, reset cache.
                wp_cache_set( $url_hash, 0, self::CACHE_GROUP );

[GaryJones]

I wonder if it's worth doing an upgrade mechanism in 1.4.0 that makes all redirect posts have a publish status, and then we can start allowing draft status to paused/indicate inactive redirects.