`wp_old_slug_redirect` ignores rewrite endpoints

[swissspidy]

For the ​oEmbed feature plugin we're adding an embed rewrite endpoint to posts, to have URLs like http://example.com/my-post/embed/

However, when the slug changes from my-post to new-post, wp_old_slug_redirect redirects to http://example.com/new-post/, ignoring the endpoint.

The function should check for such query vars / endpoints in the process.

Additionally, a filter for the resulting URL in this function would be helpful for developers.

[swissspidy]

I just added a patch that adds missing endpoints to the URL.

[pento]

I like it.

While it's kind of hacky, we could add unit tests using the new old_slug_redirect_url filter - hook into the filter, verify that the URL is correct, then return false to short circuit before the wp_redirect().

[swissspidy]

The new patch adds unit tests using the method you suggested.

While testing I noticed that WordPress doesn't properly redirect feed URLs, even with the patch applied.

• /old-slug/feed/ doesn't redirect to /new-slug/feed/ but instead shows a feed for a non-existent post.
• /old-slug/trackback/ redirects to /new-slug/

This is because feeds and trackbacks are pretty much hardcoded and not added via add_rewrite_endpoint.

[swissspidy]

New patch also fixes feeds.

[pento]

Unit tests are passing when they're run by themselves, but not when they're run as part of the full suite.

3) Tests_Rewrite_OldSlugRedirect::test_old_slug_redirect
is_404 should be true. is_single, is_singular should be false.
Failed asserting that false is true.

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:492
/srv/www/wordpress-develop/tests/phpunit/tests/rewrite/oldSlugRedirect.php:62

4) Tests_Rewrite_OldSlugRedirect::test_old_slug_redirect_endpoint
is_404 should be true. is_single, is_singular should be false.
Failed asserting that false is true.

/srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:492
/srv/www/wordpress-develop/tests/phpunit/tests/rewrite/oldSlugRedirect.php:80

5) Tests_Rewrite_OldSlugRedirect::test_old_slug_redirect_feed
Failed asserting that null matches expected 'http://example.org/?p=1087/feed/'.

/srv/www/wordpress-develop/tests/phpunit/tests/rewrite/oldSlugRedirect.php:99

[swissspidy]

Thanks.

Fixed in 33920.4.diff​ by setting up everything in setUp / tearDown.

[swissspidy]

33920.5.diff​ now properly respects custom endpoints with a different query var.

[pento]

I haven't tested it, what happens with attachment URLs?

Either way, it'd be nice to have a unit test for it.

[swissspidy]

This doesn't change behaviour for attachments, as they already seem to work properly. Perhaps because of redirect_canonical.

In WP_Query name is set to the attachment slug, so wp_old_slug_redirect has no effect.

I added a test for it in 33920.6.diff​ anyway, and also tested this with paginated posts.

[swissspidy]

See also #34043 for implementing old slug redirects for attachments.

[pento]

In 34659:

Rewrite: When redirecting old slugs, include URL endpoints.

Historically, wp_old_slug_redirect() has only ever redirected the old slug of posts, it hasn't included URL endpoints, or worked with comment feed URLs. By adding support for these, we ensure a greater range of URLs aren't killed when the slug changes.

Props swissspdy.

Fixes #33920.

[ocean90]

@pento: What about the unit test from 33920.6.diff​?

[pento]

They all look like they're there to me? (Changeset view doesn't show new files in the diff, only in the file list.)

https://core.trac.wordpress.org/browser/trunk/tests/phpunit/tests/rewrite/oldSlugRedirect.php?rev=34659