Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0
    • Fix Version/s: 1.2.2
    • Component/s: Behaviors
    • Labels:
      None
    • Environment:
      Symfony 1.4.1 LAMP

      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.

        Activity

        Pierre B created issue -
        Hide
        Jan Míšek added a comment -

        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'])
        Show
        Jan Míšek added a comment - 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'] )
        Hide
        Ivar Nesje added a comment -

        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.

        Show
        Ivar Nesje added a comment - 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.
        Hide
        Jonathan H. Wage added a comment -

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

        Show
        Jonathan H. Wage added a comment - Can anyone provide a patch for the change so I can see everything clearly? Thanks, Jon
        Hide
        Ivar Nesje added a comment -

        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.

        Show
        Ivar Nesje added a comment - 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.
        Ivar Nesje made changes -
        Field Original Value New Value
        Attachment sluggable.patch [ 10416 ]
        Ivar Nesje made changes -
        Attachment sluggable.patch [ 10416 ]
        Hide
        Ivar Nesje added a comment -

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

        Show
        Ivar Nesje added a comment - I hope this patch and test cases solves more than my problems.
        Ivar Nesje made changes -
        Attachment sluggableInheritance.patch [ 10420 ]
        Ivar Nesje made changes -
        Attachment sluggableInheritance.patch [ 10420 ]
        Hide
        Ivar Nesje added a comment -

        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

        Show
        Ivar Nesje added a comment - 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
        Ivar Nesje made changes -
        Attachment sluggable.patch [ 10426 ]
        Hide
        Jonathan H. Wage added a comment -

        Thanks for the issue and patch.

        Show
        Jonathan H. Wage added a comment - Thanks for the issue and patch.
        Jonathan H. Wage made changes -
        Status Open [ 1 ] Closed [ 6 ]
        Fix Version/s 1.2.2 [ 10047 ]
        Resolution Fixed [ 1 ]
        Show
        Klemens Ullmann-Marx added a comment - This fix breaks my system. @see: http://groups.google.com/group/doctrine-dev/browse_thread/thread/8028e51d5bde27eb
        Hide
        Ivar Nesje added a comment -

        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.

        Show
        Ivar Nesje added a comment - 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.
        Hide
        Klemens Ullmann-Marx added a comment -

        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)

        Show
        Klemens Ullmann-Marx added a comment - 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)

        This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

        • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={query=DC-419, expand=changesets[0:20].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            Pierre B
          • Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: