craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.28k stars 635 forks source link

Duplicate slug is not incrementing #5064

Closed rogertinch closed 5 years ago

rogertinch commented 5 years ago

Description

Saving a new entry with the same title as an old entry generates a duplicate slug rather than incrementing up to a new slug.

Steps to reproduce

  1. Create a new entry called "Testing"
  2. Verify slug is testing
  3. Create another entry called "Testing"
  4. Generated slug is testing

Additional info

rogertinch commented 5 years ago

Looks like this is related to turning on headlessMode.

billythekid commented 5 years ago

@tinchalamo slugs only increment if they're used in the URL I believe. So headless mode or not, if your section entries don't have URLs (or the slug isn't used as part of the url, I think) the slug won't be forced to be unique. The unique identifiers are UID and entry ID.

rogertinch commented 5 years ago

In talking to the Craft team this is expected behavior when using headless mode.

Craft has never cared about whether slugs themselves are unique; only that the resulting URI is unique. And when running in headless mode, or any time you have a section that doesn't have an entry URI, there's no URI to ensure uniqueness on.

It's expected to change in 3.4.

ryanpcmcquen commented 4 years ago

This is a serious, breaking change introduced in a minor update to Craft ...

Some of us use the slugs in other environments beyond our preview targets but still want headless mode. There should at least be a config option to ensure unique uris.

brandonkelly commented 4 years ago

@ryanpcmcquen Well, it wasn’t a breaking change; the entire concept of headless mode was introduced in 3.3, so you have to opt into the change in behavior. However it’s already been fixed in 3.4, per https://github.com/craftcms/cms/issues/4520#issuecomment-531040417.

ryanpcmcquen commented 4 years ago

@brandonkelly, I am running Craft 3.4.2 locally.

I can still duplicate an entry with a preview target of {uri} or {slug}, and the slug does not change (meaning we get dupes still). Is there something I am missing?

brandonkelly commented 4 years ago

Craft only cares about unique URIs, not slugs. And if the URI format contains a {slug} token, it will increment the slug as a way of finding a unique URI. So you will need to check your section’s settings and make sure you’ve given it an Entry URI Format – ideally one that contains {slug}.

ryanpcmcquen commented 4 years ago

@brandonkelly, I've given it a uri format containing both {uri} and {slug}, only {uri}, and only {slug}, the results are the same every time. It still isn't bothering to increment the slug, so I am getting duplicate URIs.

brandonkelly commented 4 years ago

Are you sure you’re looking at the Entry URI Format setting – not the preview target URL?

A section’s site settings, with an arrow pointed toward the Entry URI Format column

ryanpcmcquen commented 4 years ago

@brandonkelly, I did set the Entry URI (to {slug}) as well. It was still duplicating slugs but it isn't now after forcing a project config sync (it is weird because I had already saved the entry format, but forcing the sync seems to have actually applied the change). I did also update to 3.4.3, so maybe it was something there as well.

Is there actually a scenario where duplicate slugs are desired? Seems like the default behavior should be not to duplicate slugs/URIs, rather than the inverse.

brandonkelly commented 4 years ago

Yep, lots of cases. Craft 1.0 actually did enforce slug uniqueness, and a ton of people requested that they be allowed to create duplicates.

ryanpcmcquen commented 4 years ago

Nevermind, I can still reproduce the slug duplication even with the Entry URI format, here's a recording showing the behavior: duplicate_slugs_in_3 4 3_again

ryanpcmcquen commented 4 years ago

@brandonkelly, here's a screenshot of the section config as well in case I am missing something there: Screen Shot 2020-02-04 at 12 40 15 PM

brandonkelly commented 4 years ago

Do you have the maxSlugIncrement config setting set to 0 by chance? There is a special case where when duplicating an element, URI validation errors are ignored, so perhaps that’s happening here.

https://github.com/craftcms/cms/blob/8436bfe373e81a2cc9d551feb4f9bad8d0fbdefb/src/services/Elements.php#L761-L764

I’ve been thinking that maybe it makes sense to disable all elements by default when they are duplicated via that “Duplicate” element action. This is another good reason we should probably do that.

ryanpcmcquen commented 4 years ago

@brandonkelly, we don't have that setting in our codebase at all, which means we should have the default of 100.

brandonkelly commented 4 years ago

In that case I’m a little stumped. I’m not able to reproduce that on my end. Can you send your composer.json and composer.lock files, and a database backup (or at least your project.yaml file), over to support@craftcms.com, so I can try reproducing it with those?

ryanpcmcquen commented 4 years ago

@brandonkelly, thanks! I just sent it over.