-
Notifications
You must be signed in to change notification settings - Fork 11.6k
[12.x] Enhance the Str::slug method to support unique slugs #57355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 12.x
Are you sure you want to change the base?
Conversation
While this has its use cases, I'm not sure if the |
I'm not sure, but when this is being used by accessor and mutator wouldn't it cause unnecessary demand on the database? |
Adding to the above, This probably should rather be an after validation feature or something. I like the idea, though. |
If you need this feature, just make a service class for it. Keep slug generation more simple. Otherwise it becomes overcomplicated and introduces redundant dependency. |
@shaedrich @hctorres02 @NickSdot @antonkomarev @taylorotwell Appreciate the feedback, everyone, you’re right, the Str methods should stay simple. I’ve thought of two possible approaches. Let me know what you think.
use Illuminate\Database\Eloquent\Concerns\HasUniqueSlug;
class Post extends Model
{
use HasUniqueSlug;
protected $slugColumn = 'slug'; // optional, defaults to 'slug'
protected $slugSourceColumn = 'title'; // optional, defaults to 'title'
} Then, the trait automatically assigns a unique slug value during the model’s creating event. |
You're welcome 👍🏻 I'd say, both concepts have their own value. However, the latter is more likely to be rejected by Taylor in favor of a separate package if there isn't any already. In case of the former, I submitted my namespacing preference in my post above. I don't think that either |
I like the has uniqueslug trait. But that trait is so simple that it doesn't need to be in the framework or package. It's a simple Attribute with a setter or getter. |
You can achieve the same behavior using use Illuminate\Support\Str;
use Illuminate\Support\Facades\DB;
Str::macro('slugUnique', function ($value, $table, $column = 'slug') {
$slug = Str::slug($value);
if (DB::table($table)->whereExists($column, $slug)) {
throw new Exception("The '{$slug}' slug already used!");
}
return $slug;
}); What worries me most is the return type. Would it throw an exception (example) or |
This definitely should not be in a Str class and it has serious drawbacks:
Most of the time it's just better to have this logic on the model, service or application helper. |
This PR introduces support for generating unique slugs in the Str::slug method.
Example:
New Parameters:
How It Works:
When $unique is true, the method checks the database for existing slugs.
If a record with the same slug is found, it appends an incrementing number to the slug until a unique value is found.
If $ignoreId is set, the query will exclude that record from the uniqueness check.
The changes are fully backward compatible.
All newly introduced parameters are optional and have default values that preserve the original behavior of the method.