Makin replace the default strategy for saving belongsToMany, this

revealed some inconsistencies between both strategies which are now solved
cakephp · Dec 18, 2013 · a574f70 · a574f70
1 parent 1a9f262
commit a574f70
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 10 deletions.
diff --git a/Cake/ORM/Association/BelongsToMany.php b/Cake/ORM/Association/BelongsToMany.php
@@ -105,7 +105,7 @@ class BelongsToMany extends Association {
  *
  * @var string
  */
-	protected $_saveStrategy = self::SAVE_APPEND;
+	protected $_saveStrategy = self::SAVE_REPLACE;
 
 /**
  * Sets the table instance for the junction relation. If no arguments
@@ -644,7 +644,13 @@ function() use ($sourceEntity, $targetEntities, $primaryValue, $options) {
 				}
 
 				$property = $this->property();
-				$sourceEntity->set($property, $targetEntities);
+				$inserted = array_combine(
+					array_keys($inserts),
+					(array)$sourceEntity->get($property)
+				);
+				$targetEntities = $inserted + $targetEntities;
+				ksort($targetEntities);
+				$sourceEntity->set($property, array_values($targetEntities));
 				$sourceEntity->dirty($property, false);
 				return true;
 			}
@@ -711,7 +717,7 @@ protected function _diffLinks($existing, $jointEntities, $targetEntities) {
 			}
 		}
 
-		return array_values($targetEntities);
+		return $targetEntities;
 	}
 
 /**

diff --git a/Cake/Test/TestCase/ORM/Association/BelongsToManyTest.php b/Cake/Test/TestCase/ORM/Association/BelongsToManyTest.php
@@ -167,11 +167,11 @@ public function testJunctionWithDefaultTableName() {
  */
 	public function testSaveStrategy() {
 		$assoc = new BelongsToMany('Test');
-		$this->assertEquals(BelongsToMany::SAVE_APPEND, $assoc->saveStrategy());
-		$assoc->saveStrategy(BelongsToMany::SAVE_REPLACE);
 		$this->assertEquals(BelongsToMany::SAVE_REPLACE, $assoc->saveStrategy());
 		$assoc->saveStrategy(BelongsToMany::SAVE_APPEND);
 		$this->assertEquals(BelongsToMany::SAVE_APPEND, $assoc->saveStrategy());
+		$assoc->saveStrategy(BelongsToMany::SAVE_REPLACE);
+		$this->assertEquals(BelongsToMany::SAVE_REPLACE, $assoc->saveStrategy());
 	}
 
 /**
@@ -180,8 +180,8 @@ public function testSaveStrategy() {
  * @return void
  */
 	public function testSaveStrategyInOptions() {
-		$assoc = new BelongsToMany('Test', ['saveStrategy' => BelongsToMany::SAVE_REPLACE]);
-		$this->assertEquals(BelongsToMany::SAVE_REPLACE, $assoc->saveStrategy());
+		$assoc = new BelongsToMany('Test', ['saveStrategy' => BelongsToMany::SAVE_APPEND]);
+		$this->assertEquals(BelongsToMany::SAVE_APPEND, $assoc->saveStrategy());
 	}
 
 /**
@@ -1046,9 +1046,10 @@ public function testReplaceLinkSuccess() {
 		$options = ['foo' => 'bar'];
 		$assoc->expects($this->once())
 			->method('_saveTarget')
-			->with($entity, [$tags[1], $tags[2]], $options + ['associated' => false])
+			->with($entity, [1 => $tags[1], 2 => $tags[2]], $options + ['associated' => false])
 			->will($this->returnCallback(function($entity, $inserts) use ($tags) {
-				$this->assertSame([$tags[1], $tags[2]], $inserts);
+				$this->assertSame([1 => $tags[1], 2 => $tags[2]], $inserts);
+				$entity->tags = $inserts;
 				return true;
 			}));
 

diff --git a/Cake/Test/TestCase/ORM/TableTest.php b/Cake/Test/TestCase/ORM/TableTest.php
@@ -2881,7 +2881,10 @@ public function testReplacelinksBelongsToMany() {
 		$tags[] = new \TestApp\Model\Entity\Tag(['name' => 'foo']);
 
 		$table->association('tags')->replaceLinks($article, $tags);
-		$this->assertSame($tags, $article->tags);
+		$this->assertEquals(2, $article->tags[0]->id);
+		$this->assertEquals(3, $article->tags[1]->id);
+		$this->assertEquals(4, $article->tags[2]->id);
+
 		$article = $table->find('all')->where(['id' => 1])->contain(['tags'])->first();
 		$this->assertCount(3, $article->tags);
 		$this->assertEquals(2, $article->tags[0]->id);