[DC-419] Sluggable and inheritance Created: 11/Jan/10  Updated: 06/Sep/11  Resolved: 15/Mar/10

Status: Closed
Project: Doctrine 1
Component/s: Behaviors
Affects Version/s: 1.2.0
Fix Version/s: 1.2.2

Type: Bug Priority: Major
Reporter: Pierre B Assignee: Jonathan H. Wage
Resolution: Fixed Votes: 2
Labels: None
Environment:

Symfony 1.4.1 LAMP


Attachments: Text File sluggable.patch    

 Description   

I've the same problem than this post : http://groups.google.com/group/doctrine-user/browse_thread/thread/3737fd293fef5fda/d86a8bc2578e4bac

Then, I've set "uniqueBy: [name, type] ", and my data goes in database BUT they can have the same slug.

So I can't retrieve an objects wich has equal slug with another.

The column aggregation inheritance does'nt take care of others slugs.



 Comments   
Comment by Jan Míšek [ 01/Mar/10 ]

First, inheritance is great feature, thanks doctrine team.

But I have this problem too.

I have identified, the problem is related to class: "Doctrine_Template_Listener_Sluggable", to method: getUniqueSlug($record, $slugFromFields). In mentioned method, table is retrieved from $record ($record->getTable()), but record could be inherited class, so query result on the table is limited only to records of the inherited class. But there could be already rows of other inherited classes with same slug in the database.

Fast hack to solve it, works for me:

  • Added option variable to behaviour "table"
  • Replaced all calls $record->getTable() as Doctrine::getTable($this->_options['table'])
Comment by Ivar Nesje [ 02/Mar/10 ]

Thanks for the solution. I do not like the idea of patching the core framework, but it's just how we will have to deal with it. I hope someone will comment on this bugreport so that I'll be informed when the issue is fixed.

The point of sluggable with column aggregation inheritance is to have a unique identifier to make a common interface for all entitis. Like /events/:slug where the presentation is dependent on the modell.
Ofcourse I could have set up one route for every child class, but that will make more code for configuration, whitch is a pain to maintain.

Comment by Jonathan H. Wage [ 02/Mar/10 ]

Can anyone provide a patch for the change so I can see everything clearly? Thanks, Jon

Comment by Ivar Nesje [ 03/Mar/10 ]

Here is a simple patch implementing the changes Jan Míšek proposed. (as far as I understod them)

To make the solution better, it would be nice if the Behaviour somhow found out that it was a child class (without requirering me to specify the parent ) and made sure that it made a slug unique across all subclases, unless the unique by setting spesified the type column. I'm realy glad I had a lot of data to import from a previous site, so that I discovered this bug. I use column aggregation inheritance to make the code for presenting different events in a different way simple. But every event sholud be accessable trough the same route mysite.com/events/:slug.

Comment by Ivar Nesje [ 03/Mar/10 ]

I hope this patch and test cases solves more than my problems.

Comment by Ivar Nesje [ 04/Mar/10 ]

Updated the patch a little, so that it does not try to instanciate an abstract class as doctrine generates them.

I was walking up the inheritance tree and tried to instanciate the class right under DoctrineRecord unfortunatly in symfony there is many layers of abstract classes before you find the base class. Now the plugin walks back to the highes not abstract class

Comment by Jonathan H. Wage [ 15/Mar/10 ]

Thanks for the issue and patch.

Comment by Klemens Ullmann-Marx [ 09/Apr/10 ]

This fix breaks my system.

@see: http://groups.google.com/group/doctrine-dev/browse_thread/thread/8028e51d5bde27eb

Comment by Ivar Nesje [ 09/Apr/10 ]

Hmm.. I'm sory that my ugly fix to remove the 'where type = $type' part of the query to find existing slugs that might cause a colission with the proposed slug.

Does anyone have a better idea on how to ask for all slugs in the same model? I had a pretty hard time traversing the inheritance tree to find the right parent class that were not abstract. I see that something similar has been done about soft delete, so that a new record would not get the same slug as a record marked as deleted, but not removed from the databse.

Comment by Klemens Ullmann-Marx [ 06/Sep/11 ]

Here's an improved patch with a better algorithm to find the column aggregation inhertiance base class:

http://trac.ullright.org/browser/trunk/plugins/ullCorePlugin/patch/Sluggable.patch?rev=3067

This fixes my problem (see above)

Generated at Mon Apr 21 00:31:11 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.