Details
-
Type:
New Feature
-
Status:
Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: None
-
Fix Version/s: 1.2.0-ALPHA3
-
Component/s: Sluggable
-
Labels:None
Description
Recently I had a model where I needed a custom unique slug instead of a slug which is built from fields.
At first I thought that maybe the "builder"-option was there for that purpose but soon I realized that this is obviously not the right one.
The only left option was that I would need to create the method "getUniqueSlug" in the record class. This is of course an option and has its uses but with it you would have to reimplement the collision detection for uniqueness again which are already implemented in the getUniqueSlug-method of the sluggable class.
After reading the code several hours I came to the conclusion that this functionality is simply missing so I created a new option called "provider" which is a callback. This callback provides the equivalent of the concatenation of the table fields. The great thing about this is that you don't have to care for uniqueness or the proper urlized format because this is still taken care of by the sluggable behaviour. Thus this patch is very unobtrusive and completely backwards-compatible of course.
Usage is pretty straight-forward:
model:
actAs:
Sluggable:
provider: [modelTable, provideSlug]
Or whatever you want to name the method which is called.
jwage responded in trac:
This is already possible if you just implement a toString() method and don't specify any fields.
But I have to disagree. If I use toString() I still have to care about the uniqueness of the slug by myself because only when fields are specified this gets called inside the code:
$this->getUniqueSlug($record, $value);
Otherwise only the builder-function is called.
So atm this is NOT possible.
Activity
If I might say this I think that the behaviour of the sluggable template is highly incomprehensible if you don't use the standard function with simple fields. It looks like a bit of patchwork to me.
The first thing I don't quite understand is that why is there an option to disable uniqueness? Isn't a slug by itself supposed to be always unique? What other uses could a slug have if it isn't?
Second, I don't think that "$value = (string) $record;" should be anywhere used for real logic. The toString() method is actually there for debugging purposes but not for providing slugs or anything else.
If you want to change it then really change it but not by doing a "$this->getUniqueSlug(..)" around the "(string) $record". I know that backwards compatibility is important. That is why I thought about a very unobtrusive way. Maybe if you look at the patch a second time you're starting to like it more 
Thanks for implementing it. Although I don't use this possibility I think you broke backwards-compatibility which I wanted to avoid, notably for the getUniqueSlug-way because in the past this was the way for caring about the uniqueness on ones own, and now you make it unique again by doing the "if ($this->_options['unique'] === true)
{" check for every way. This probably won't make a difference because it's unique in the first place but the point of having this "} else if (method_exists($record, 'getUniqueSlug')) {" was that one could save the effort that doctrine itself would have made which are also db queries. So effectively you lowered the performance of Sluggable in some cases
Ah Ok. I think maybe we could make some change to always run the slug through the function to ensure uniqueness and proper format. We could try that.