easy-slug
easy-slug copied to clipboard
viraj-khatavkar
generateUniqueSlug not working for slugs without -% at the end
Clients began to complain about the wrong records being retrieved when they clicked on links and I assumed that I had forgotten to enable EasySlug when creating those entries. I checked my code and found that I had indeed used EasySlug::generateUniqueSlug().
If I have an entry with a slug of football-season on my seasons table, EasySlug will generate the next slug as football-season. However, if I change the slug on the table to football-season-1, EasySlug generates the next as football-season-2.
As a test I tried using the wrong table name to see the SQL query behind the scenes and noticed this:
$title = "Football Season";
$slug = EasySlug::generateUniqueSlug($title, "season", "slug");
select count(*) as aggregate from `season` where `slug` LIKE football-season-%
So it appears to be looking only for the number of items with -% in their slug, not simply a slug without the number of entries.
Am I missing something? This worked until recently, perhaps a breaking change somewhere?
@opheliadesign is it creating same slug if count of slug is not present at end ?
@viraj-khatavkar correct. So if football-season exists already, EasySlug still uses football-season for the next entry. It only increments if the first one is football-season-1
I am thinking that this is an issue in the queries within EasySlugRepository.
@opheliadesign Please do composer update once and check. I have resolved it. But, please check and let me know
@opheliadesign Please do composer update once again. There was one syntax issue that I resolved just now
Any progress on this? Here is the code that I suspect is causing this:
if ( $count_of_matching_slugs > 0 )
{
$temporary_slug = $this->generateSlug( $string . " " . ( $count_of_matching_slugs + 1 ) , $separator );
$flag = false;
$i = 2;
while($flag == false)
{
$exact_slugs = $this->_easy_slug_repo->getCountOfExactSlugs($table, $column, $temporary_slug);
if($exact_slugs > 0)
{
$temporary_slug = $this->generateSlug( $string . " " . ( $count_of_matching_slugs + $i ) , $separator );
$i++;
}
else
{
$flag = true;
}
}
}
else {
$temporary_slug = $temporary_slug . '-1'; // This will always start at -1 rather than just a plain slug
}
@opheliadesign can you please post an use case of the issue. I have been using this version in about 13 production applications and I haven't deduced this issue. It would be really helpful with the usecase
@viraj-khatavkar the use case is generating a unique slug, as outlined on the initial issue report above. The first unique slug, according to the documentation and my preference, should not have a -1 suffix. This is being appended by:
....
} else {
$temporary_slug = $temporary_slug . '-1'; // This will always start at -1 rather than just a plain slug
}
To be clear, the code mentioned in this post and my last is from your package.
@viraj-khatavkar starting a new project and still running into this, a post with the title "Test Post" and this code generates "test-post-1" instead of "test-post"
EasySlug::generateUniqueSlug($request->input('title'), 'posts');
@opheliadesign any problem in that, I did this to avoid another conflicting issue in case of a string like "abc 1" and 2 times "abc"
@viraj-khatavkar Most sites, including mine, seem to favor adding -x only if there are multiple posts with that title. Your documentation and previous implementation follow this -
"This function looks for similar slugs in the table/column name specified in parameters. If slugs with similar pattern are found it appends numeric digits at the end of slug as follows :"
your-string
your-string-2
your-string-3
your-string-4
If we could go back to this format, I'd really appreciate it. Thanks for replying so quickly :)
Okay, give me an hour, I will look into that if I can go back to that :)
Apology for not replying for the last time you raised this, was bit busy in some work and then forgot :( Will look to this ASAP
Sure, no problem! Really appreciate it. I've been busy since then, too ;)
Just a thought, maybe a little RegEx magic to see if the title ends in a digit? That way you could prevent the issue that you mentioned but otherwise return the typical slug without an appended number if it's unique.
Doing that is not an issue. If I check that last digit is a number or not I am stuck in following case :
- "abc" -> "abc"
- "abc" -> "abc-1"
- "abc-1" -> "abc-1" //Now, this is a conflicting case for me
Hmm yes I see what you're saying. If there isn't already abc-1 in the database, start with abc-1. If it's the second abc-1, go to abc-1-2 and so on. Just a thought, have not looked at your source in a bit.
What about an optional parameter in generateUnique that would allow us to pass in the name column. So it could look for existing entries like it does now, then compare them against the name. If the name has a digit at the end, you would know you still need to append a digit.
@viraj-khatavkar nag nag nag nag ... sorry, just had to do it ;)
@stygiansabyss great idea, I like it.
Ended up using https://github.com/cviebrock/eloquent-sluggable. For this package though, you could look at how he is doing it. He has you add a build_from and save_to on the model. Which was the idea I suggested. Knowing the name gives you more control over how to slug.
@opheliadesign I found a solution, will implement it in a day, if you are still holding on to this package
Honestly, I've started using the other package mentioned in this thread. However, I like the simplicity of yours. If you can implement a fix I will most likely use it again.
Thanks!
