Merge BelongsToMany associations more consistently.

The issue in #7808 was a marshaller issue not an association saving issue. By fixing the marshaller the association save does not have to change. Refs #7808
cakephp · Jan 7, 2016 · 419d0c4 · 419d0c4
1 parent b5a9068
commit 419d0c4
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 37 deletions.
diff --git a/src/ORM/Association/BelongsToMany.php b/src/ORM/Association/BelongsToMany.php
@@ -622,12 +622,9 @@ protected function _saveLinks(EntityInterface $sourceEntity, $targetEntities, $o
         $junctionAlias = $junction->alias();
 
         foreach ($targetEntities as $e) {
-            $joint = $jointVal = $e->get($jointProperty);
+            $joint = $e->get($jointProperty);
             if (!$joint || !($joint instanceof EntityInterface)) {
                 $joint = new $entityClass([], ['markNew' => true, 'source' => $junctionAlias]);
-                if (is_array($jointVal)) {
-                    $joint->set($jointVal);
-                }
             }
             $sourceKeys = array_combine($foreignKey, $sourceEntity->extract($bindingKey));
             $targetKeys = array_combine($assocForeignKey, $e->extract($targetPrimaryKey));

diff --git a/src/ORM/Marshaller.php b/src/ORM/Marshaller.php
@@ -657,7 +657,7 @@ protected function _mergeBelongsToMany($original, $assoc, $value, $options)
             return [];
         }
 
-        if (!in_array('_joinData', $associated) && !isset($associated['_joinData'])) {
+        if (!empty($associated) && !in_array('_joinData', $associated) && !isset($associated['_joinData'])) {
             return $this->mergeMany($original, $value, $options);
         }
 

diff --git a/tests/TestCase/ORM/Association/BelongsToManyTest.php b/tests/TestCase/ORM/Association/BelongsToManyTest.php
@@ -656,38 +656,6 @@ public function testReplaceLinkWithConditions()
         $this->assertSame(1, $jointCount, 'Non matching joint record should remain.');
     }
 
-    /**
-     * Tests that replaceLinks will apply _joinData when it has not been converted
-     * to an entity if that data is an array.
-     *
-     * @return void
-     */
-    public function testReplaceLinkJoinDataHandling()
-    {
-        $joint = TableRegistry::get('SpecialTags');
-        $articles = TableRegistry::get('Articles');
-
-        $assoc = $articles->belongsToMany('Tags', [
-            'through' => 'SpecialTags',
-            'conditions' => ['SpecialTags.highlighted' => true]
-        ]);
-        $id = 2;
-        $entity = $articles->get($id, ['contain' => 'Tags']);
-
-        // New tag
-        $tagData = [
-            new Entity(['id' => 2, '_joinData' => ['highlighted' => true]]),
-        ];
-
-        $assoc->replaceLinks($entity, $tagData);
-        $this->assertSame($tagData, $entity->tags, 'Tags should match replaced objects');
-        $this->assertFalse($entity->dirty('tags'), 'Should be clean');
-
-        $jointRecords = $joint->find()->where(['article_id' => $id])->toArray();
-        $this->assertCount(1, $jointRecords);
-        $this->assertTrue($jointRecords[0]->highlighted, 'joinData should be set.');
-    }
-
     /**
      * Provider for empty values
      *

diff --git a/tests/TestCase/ORM/MarshallerTest.php b/tests/TestCase/ORM/MarshallerTest.php
@@ -1730,6 +1730,46 @@ public function testMergeBelongsToManyJoinDataNotAccessible()
         $this->assertTrue($result->tags[0]->_joinData->highlighted);
     }
 
+    /**
+     * Test that _joinData is marshalled consistently with both
+     * new and existing records
+     *
+     * @return void
+     */
+    public function testMergeBelongsToManyHandleJoinDataConsistently()
+    {
+        TableRegistry::clear();
+        $articles = TableRegistry::get('Articles');
+        $articles->belongsToMany('Tags', [
+            'through' => 'SpecialTags'
+        ]);
+
+        $entity = $articles->get(1);
+        $data = [
+            'title' => 'Haz data',
+            'tags' => [
+                ['id' => 3, 'tag' => 'Cake', '_joinData' => ['highlighted' => true]],
+            ]
+        ];
+        $marshall = new Marshaller($articles);
+        $result = $marshall->merge($entity, $data, ['associated' => 'Tags']);
+        $this->assertInstanceOf('Cake\ORM\Entity', $result->tags[0]->_joinData);
+        $this->assertTrue($result->tags[0]->_joinData->highlighted);
+
+        // Also ensure merge() overwrites existing data.
+        $entity = $articles->get(1, ['contain' => 'Tags']);
+        $data = [
+            'title' => 'Haz data',
+            'tags' => [
+                ['id' => 3, 'tag' => 'Cake', '_joinData' => ['highlighted' => true]],
+            ]
+        ];
+        $marshall = new Marshaller($articles);
+        $result = $marshall->merge($entity, $data, ['associated' => 'Tags']);
+        $this->assertInstanceOf('Cake\ORM\Entity', $result->tags[0]->_joinData);
+        $this->assertTrue($result->tags[0]->_joinData->highlighted);
+    }
+
     /**
      * Test merging belongsToMany data doesn't create 'new' entities.
      *