Development

#3037 (Admin generator: admin_double_list cannot handle objects of the same class)

You must first sign up to be able to contribute.

Ticket #3037 (new defect)

Opened 9 months ago

Last modified 2 weeks ago

Admin generator: admin_double_list cannot handle objects of the same class

Reported by: dante Assigned to: fabien
Priority: minor Milestone:
Component: generator Version: 1.1.0 DEV
Keywords: Many relationship admin_double_list Cc:
Qualification: Ready for core team

Description

Admin generator generates faulty code for admin_double_list if the same class/table is referenced by both keys in the matching table.

As a result, you just cannot assign objects of the same class to each other. This is *not* a self-referential table issue. I managed to fix this by altering some generator files but I cannot assess if there are any side effects caused by that. I can post my changes if required.

Attachments

admin_generator-self_referential.patch (7.6 kB) - added by georgsorst on 04/30/08 17:11:14.
Fix for self-referential relations for the admin generator
admin_generator-self_referential-fixed.patch (7.6 kB) - added by georgsorst on 04/30/08 17:12:49.
Fix for self-referential relations for the admin generator (fixed since I cannot seem to replace the first version)

Change History

03/03/08 18:53:55 changed by dante

  • version changed from 1.0.3 to 1.0.11.

See this thread for a probable fix: http://www.symfony-project.org/forum/index.php/t/8493

03/03/08 18:58:46 changed by dante

Sorry, wrong URL (slash missing). Correct one: http://www.symfony-project.org/forum/index.php/t/8493/

04/16/08 20:31:41 changed by georgsorst

  • version changed from 1.0.11 to 1.0.12.

I couldn't get dante's fix to work so I've attached a more extensive one which works fine with Symfony 1.0.13. The only requirement is to specify which column points to the related object (which is of the same class / table as the local object), similar to the through class. It's done like this:

generator.yml:

...
        article_article_relation: 
          name: Related Articles
          type: admin_double_list
          params: through_class=ArticleArticle related_column=RELATED_ARTICLE_ID
...

As far as I can see it is necessary to explicitly specify the related column since there is no way to figure it out automatically as there are two column in the through table that point to the same table.

04/27/08 01:16:57 changed by georgsorst

  • version changed from 1.0.12 to 1.0.13.

I've updated my patch since at least with Symfony 1.0.13 there is a method to directly get the related object of the self-referential through-object. I might have missed just missed it before though.

04/30/08 17:11:14 changed by georgsorst

  • attachment admin_generator-self_referential.patch added.

Fix for self-referential relations for the admin generator

04/30/08 17:12:49 changed by georgsorst

  • attachment admin_generator-self_referential-fixed.patch added.

Fix for self-referential relations for the admin generator (fixed since I cannot seem to replace the first version)

04/30/08 20:42:24 changed by Carl.Vondrick

  • version changed from 1.0.13 to 1.1.0 DEV.
  • qualification changed from Unreviewed to Ready for core team.

This can't be included in 1.0 as it's a feature.

05/14/08 16:22:36 changed by georgsorst

I'm sorry since it's not my place to argue on that, but how is this a feature and not a bug? Self-referential relations are quite common, it is stated nowhere that they are not supported and there are no backwards-compatibility issues. This change will merely fix support for expected many-to-many functionality IMHO.

05/21/08 11:06:26 changed by cyruschoi

I found that the patch assume the related column's name is id, which unfortunately doesn't work for me. So, I do a little contribution below. I found that it work for me so far, if it is the permanent fix. Then, can georgsorst help to update accordingly ?

in file symfony/addon/propel/sfPropelManyToMany.class.php

$c->add(constant($middleClass.'Peer::'.$localColumn->getColumnName()), $object->getId());

change to

$c->add(constant($middleClass.'Peer::'.$localColumn->getColumnName()), $object->getPrimaryKey());

@@ -83,19 +116,32 @@
    * @param  $middleClass   The middle class used for the many-to-many relationship.
    * @param  $criteria      Criteria to apply to the selection.
    */
-  public static function getRelatedObjects($object, $middleClass, $criteria = null)
+  public static function getRelatedObjects($object, $middleClass, $relatedColumn = '', $criteria = null)
   {
     if (null === $criteria)
     {
       $criteria = new Criteria();
     }

-    $relatedClass = self::getRelatedClass(get_class($object), $middleClass);
+    $relatedClass = self::getRelatedClass(get_class($object), $middleClass, $relatedColumn);

     $relatedObjects = array();
-    $objectMethod = 'get'.$middleClass.'sJoin'.$relatedClass;
-    $relatedMethod = 'get'.$relatedClass;
-    $rels = $object->$objectMethod($criteria);
+    if (empty($relatedColumn))
+    {
+      $objectMethod = 'get'.$middleClass.'sJoin'.$relatedClass;
+      $relatedMethod = 'get'.$relatedClass;
+      $rels = $object->$objectMethod($criteria);
+    }
+    else
+    {
+      // as there is no way to join the related objects starting from this object we'll use the through class peer instead
+      $localColumn = self::getColumn(get_class($object), $middleClass, $relatedColumn);
+      $remoteColumn = self::getRelatedColumn(get_class($object), $middleClass, $relatedColumn);
+      $c = new Criteria();
+      $c->add(constant($middleClass.'Peer::'.$localColumn->getColumnName()), $object->getPrimaryKey());
+      $relatedMethod = 'get'.$relatedClass.'RelatedBy'.$remoteColumn->getPhpName();
+      $rels = call_user_func(array($middleClass.'Peer', 'doSelectJoin'.$relatedClass.'RelatedBy'.$remoteColumn->getPhpName()), $c);
+    }
     foreach ($rels as $rel)
     {
       $relatedObjects[] = $rel->$relatedMethod();

06/23/08 04:05:41 changed by dwhittle

  • milestone set to 1.1.0 FINAL.

06/23/08 04:12:16 changed by dwhittle

Fixed in sfPropelPlugin + dwhittle branch, in r9758, r9759.

06/25/08 13:59:03 changed by fabien

  • milestone changed from 1.1.0 FINAL to 1.2.0.

(follow-up: ↓ 12 ) 07/10/08 23:30:03 changed by jrupe

This works great for me. Make sure to add this code if you want to make the relations reflective, i.e. if item A is associated with item B, then item B is also associated with item B. This can be placed in the modules/mymodule/actions/actions.class.php file; you will have to modify based on your own module/db names.

  protected function savePortfolio($portfolio)
  {
    // Update many-to-many for "related_projects" -- other way around
    $c = new Criteria();
    $c->add(RelatedProjectsPeer::PORTFOLIO_ID, $portfolio->getPrimaryKey());
    RelatedProjectsPeer::doDelete($c);
    
    $ids = $this->getRequestParameter('associated_related_projects');
    if (is_array($ids))
    {
      foreach ($ids as $id)
      {
        $RelatedProjects = new RelatedProjects();
        /*
        $RelatedProjects->setPortfolioRelatedId($portfolio->getPrimaryKey());
        $RelatedProjects->setPortfolioId($id);
        */
        $RelatedProjects->setPortfolioRelatedId($id);
        $RelatedProjects->setPortfolioId($portfolio->getPrimaryKey());
        $RelatedProjects->save();
      }
    }
    parent::savePortfolio($portfolio);
  }

(in reply to: ↑ 11 ) 07/11/08 19:10:32 changed by jrupe

Replying to jrupe: Edit: make the call to parent::save<Object>($object); first, as otherwise $object->getPrimaryKey() will not work

11/18/08 01:39:09 changed by fabien

  • milestone deleted.