WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#15140 closed enhancement (fixed)

When non-hierarchical CPT permalinks change, we should 301 redirect like with posts

Reported by: jorbin Owned by: markjaquith
Milestone: 3.1 Priority: normal
Severity: normal Version:
Component: Canonical Keywords:
Focuses: Cc:

Description


Change History (21)

comment:1 scribu4 years ago

I've been trying to find the code that does that, but I guess I haven't been trying hard enough.

+1 from me.

comment:2 scribu4 years ago

  • Component changed from General to Canonical

comment:3 nacin4 years ago

  • Milestone changed from Awaiting Review to 3.1
  • Owner set to nacin
  • Status changed from new to accepted

I've been meaning to create this ticket for about two weeks.

comment:4 Denis-de-Bernardy4 years ago

  • Milestone 3.1 deleted
  • Resolution set to duplicate
  • Status changed from accepted to closed

comment:5 follow-up: nacin4 years ago

  • Milestone set to 3.1
  • Resolution duplicate deleted
  • Status changed from closed to reopened

The scope for this ticket is much narrower.

I will post a patch.

comment:6 in reply to: ↑ 5 Denis-de-Bernardy4 years ago

Replying to nacin:

The scope for this ticket is much narrower.

I beg to differ. If you implement it for the previous URL without bugs (I.e. deleting conflicts as they appear), implementing it for all URLs is a matter of using multiple key/values instead of a single one.

comment:7 dd324 years ago

I've been trying to find the code that does that, but I guess I haven't been trying hard enough.

Probably be looking for wp_old_slug_redirect() ..That really should be merged with Canonical IMO..

comment:8 dd324 years ago

(In [15831]) Save _wp_old_slug for all published post_type's, brings old-slug redirection to Posts and CPT's. See #15140

comment:9 dd324 years ago

(In [15832]) Use $wpdb->prepare in wp_old_slug_redirect(). See #15140

comment:10 dd324 years ago

I realised this morning, That I didn't limit the redirection to posts of the same post type as being requested..

Potential thoughts of mine:

  • Request ALL matching records, foreach ($post ) { if ( compare post type ) { redirect;break; } }
  • JOIN in SQL, seems overkill.

comment:11 dd324 years ago

(In [15848]) Limit wp_old_slug_redirect() to redirecting to only posts of the same post_type kind. See #15140

comment:12 dd323 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Closing as fixed, Re-open if a bug related to this crops up in 3.1 testing, Else, open a new ticket.

comment:13 Denis-de-Bernardy3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Let's implement #9825 step/bug by step/bug...

If both the slug and the parent change, the redirect doesn't work.

comment:14 Denis-de-Bernardy3 years ago

Oh, forgot to mention. You then need to create a new page, different parent, same slug, to fully appreciate why we need to store/flush the actual permalink, rather than just the slug.

comment:15 nacin3 years ago

#9825 should stay in #9825. However, handling only the slug for hierarchical post types isn't a very good idea, I agree. Per discussion in IRC, this should be limited to non-hierarchical post types only for now.

comment:16 markjaquith3 years ago

  • Owner changed from nacin to markjaquith
  • Status changed from reopened to reviewing

Denis' objection is noted. I don't think this is ready for hierarchical post types. I'm working on a patch to restrict this to non-hierarchical post types for now, and we can look at a hierarchical fix for 3.2

comment:17 automattor3 years ago

(In [16818]) Do not do slug logging/redirects for hierarchical post types. see #15140

comment:18 markjaquith3 years ago

  • Milestone changed from 3.1 to Future Release

comment:19 markjaquith3 years ago

(In [16819]) Use is_page_type_hierarchical(). props nacin. see #15140

comment:20 dd323 years ago

  • Milestone changed from Future Release to 3.1
  • Resolution set to fixed
  • Status changed from reviewing to closed
  • Summary changed from When page permalinks change, we should 301 redirect like with posts to When non-hierarchical CPT permalinks change, we should 301 redirect like with posts

Closing as fixed in 3.1 for non-hierarchical post types, For Pages, We have #4328 still

comment:21 nacin3 years ago

http://core.trac.wordpress.org/changeset/16818/trunk/wp-includes/query.php broke a trick I've leveraged where I stuffed _wp_old_slug into the DB for a page, and then the redirect took place. Very few have likely done that, but it's technically a regression.

Note: See TracTickets for help on using tickets.