WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#34043 closed enhancement (fixed)

wp_check_for_changed_slugs() ignores attachments

Reported by: swissspidy Owned by: pento
Milestone: 4.4 Priority: normal
Severity: normal Version: 2.1
Component: Posts, Post Types Keywords: has-patch
Focuses: Cc:
PR Number:

Description

While working on #33920 I noticed that wp_check_for_changed_slugs() only works for published, non-hierarchical posts.

However, I think it should also store the changed slug for attachments (post status 'inherit'). Currently, when you change the slug of an attachment and visit its old URL, you get a 404 error.

When implementing this we could enhance the unit tests #33920 introduces and also add tests for the wp_check_for_changed_slugs function itself beforehand.

Attachments (2)

34043.diff (4.4 KB) - added by swissspidy 4 years ago.
34043.2.diff (5.3 KB) - added by swissspidy 4 years ago.

Download all attachments as: .zip

Change History (7)

#1 @swissspidy
4 years ago

  • Keywords has-patch added; needs-patch needs-unit-tests removed
  • Milestone changed from Awaiting Review to 4.4

@swissspidy
4 years ago

#2 @pento
4 years ago

  • Keywords needs-refresh added
  • Given that there are other post types that can use the post_status inherit, I think the in_array($post->post_status, array( 'publish', 'inherit' ) ) check needs to be stricter, and only allow attachments with that post_status.
  • While you're editing the next line, s/hasnt/hasn't/ in the comment. :-)
  • A few spots missing a space inside the brackets.

I'm also pondering if, when changing the slug on a post, the slug on attachments should change, too. I'm open to arguments either way.

@swissspidy
4 years ago

#3 follow-up: @swissspidy
4 years ago

  • Keywords needs-refresh removed

Thanks! I just updated the patch now. You're right, the check should be more strict.

I'm also pondering if, when changing the slug on a post, the slug on attachments should change, too. I'm open to arguments either way.

Can you elaborate on this? Accessing an attachment URL with a wrong post slug in it was handled by [34272].

#4 in reply to: ↑ 3 @pento
4 years ago

Replying to swissspidy:

Can you elaborate on this? Accessing an attachment URL with a wrong post slug in it was handled by [34272].

Ooops, my mistake. I forgot how it worked. :-)

#5 @pento
4 years ago

  • Owner set to pento
  • Resolution set to fixed
  • Status changed from new to closed

In 34685:

Rewrite: Redirect attachment URLs when their slug changes.

Using the same logic that we use to redirect posts when their slug changes, we can provide the same functionality for attachments. Attachment pages are posts, too.

Props swissspdy.

Fixes #34043.

Note: See TracTickets for help on using tickets.