Make WordPress Core

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#51147 closed defect (bug) (fixed)

avoid_blog_page_permalink_collision shouldn't change the post_name when the page has a parent.

Reported by: stormrockwell's profile stormrockwell Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.9 Priority: normal
Severity: normal Version: 5.5
Component: Posts, Post Types Keywords: has-patch has-unit-tests
Focuses: administration, multisite Cc:

Description

How to reproduce:

Create multisite install (subdirectory)
Add subsite "bar"
On the main site, create a page for "foo" and a page for "bar" with the parent "foo"

When trying to publish that page you will get /foo/bar{rand1-10}.

Attachments (4)

51147.diff (455 bytes) - added by stormrockwell 4 years ago.
51147.2.diff (2.8 KB) - added by stormrockwell 4 years ago.
Add unit test
51147.3.diff (2.8 KB) - added by stormrockwell 4 years ago.
Unit tests & fix typo in comment.
51147.4.diff (2.8 KB) - added by stormrockwell 4 years ago.
Change the class name

Download all attachments as: .zip

Change History (20)

@stormrockwell
4 years ago

#1 @SergeyBiryukov
4 years ago

  • Component changed from Administration to Posts, Post Types
  • Focuses administration multisite added
  • Keywords has-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 5.6
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

Hi there, welcome back to WordPress Trac!

Thanks for the ticket and the patch, this makes sense at a glance.

A unit test to confirm the fix would also be great.

@stormrockwell
4 years ago

Add unit test

#2 @stormrockwell
4 years ago

This was my first go at unit tests. Any feedback is much appreciated.

@stormrockwell
4 years ago

Unit tests & fix typo in comment.

@stormrockwell
4 years ago

Change the class name

#3 @helen
4 years ago

  • Milestone changed from 5.6 to Future Release
  • Version changed from trunk to 5.5

Don't want to go changing these internals in late beta, moving out of 5.6 and assigning the reported version to 5.5 although I imagine this has existed since long before that. If this really is new to trunk please feel free to move back and kindly add more context around what change caused it.

#4 @stormrockwell
4 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

#5 @stormrockwell
3 years ago

Is there anything else you need from me on this?

#6 @terriann
3 years ago

This defect still exists in 5.8.1, echoing the last question from @stormrockwell - is there anything else that needs to be done to get this into a future release plan?

#7 @whyisjake
3 years ago

#44112 was marked as a duplicate.

#8 @whyisjake
3 years ago

  • Keywords commit added

Tests look great, marking for commit.

#9 @whyisjake
3 years ago

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

In 51855:

Posts, Post Types: Don't add a trailing number when there is a unique post parent.

WordPress tries to avoid an issue where slugs might match an existing slug of a page/post.
If we are in a hierarchical post type, there will be a level, and we can leave it the same.

Props stormrockwell, SergeyBiryukov, terriann, tubys, jeremyfelt, Daschmi, MaximeCulea, knutsp, whyisjake.
Fixes #51147.
See also #44112 and #45260.

#10 @hellofromTonya
3 years ago

  • Keywords needs-patch added; commit has-patch has-unit-tests removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening as [51855] needs a follow-up to upgrade the test case fixture methods from camelCase to snake_case, i.e. setUp() to set_up() etc. I'll fix shortly.

#11 @SergeyBiryukov
3 years ago

  • Milestone changed from Future Release to 5.9

This ticket was mentioned in PR #1699 on WordPress/wordpress-develop by hellofromtonya.


3 years ago
#12

  • Keywords has-patch has-unit-tests added; needs-patch removed

#13 @hellofromTonya
3 years ago

In 51857:

Build/Test Tools: Upgrades Tests_Multisite_MS_Permalink_Collision fixture methods and strict assertion.

Improvements include:

  • Upgrades the test fixture methods to the new snake_case methods.
  • Reorders the fixture methods for consistency.
  • Uses strict assertions of assertSame and assertNotSame.

Follow-up to [51855-51856].

Props hellofromTonya.
See #51147.

#14 @hellofromTonya
3 years ago

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

Reclosing with [51857].

#16 @SergeyBiryukov
3 years ago

In 51859:

Tests: Further improve the tests for avoid_blog_page_permalink_collision():

  • Rename the test filename and class to match the name of the function being tested.
  • Remove unnecessary setUp() and tearDown() methods.
  • Replace the only test group with post.

Follow-up to [51855-51857].

See #51147.

Note: See TracTickets for help on using tickets.