Browse Source

Fixed bug where deleteAll tried to delete same id multiple times.

Ensure find done in deleteAll only returns distinct ids. A wacky
combination of association and conditions can sometimes generate
multiple rows per id.
ADmad 12 years ago
parent
commit
f3900e89fd
2 changed files with 38 additions and 1 deletions
  1. 1 1
      lib/Cake/Model/Model.php
  2. 37 0
      lib/Cake/Test/Case/Model/ModelDeleteTest.php

+ 1 - 1
lib/Cake/Model/Model.php

@@ -2699,7 +2699,7 @@ class Model extends Object implements CakeEventListener {
 		}
 
 		$ids = $this->find('all', array_merge(array(
-			'fields' => "{$this->alias}.{$this->primaryKey}",
+			'fields' => "DISTINCT {$this->alias}.{$this->primaryKey}",
 			'recursive' => 0), compact('conditions'))
 		);
 

+ 37 - 0
lib/Cake/Test/Case/Model/ModelDeleteTest.php

@@ -455,6 +455,43 @@ class ModelDeleteTest extends BaseModelTest {
 	}
 
 /**
+ * testDeleteAllMultipleRowsPerId method
+ *
+ * Ensure find done in deleteAll only returns distinct ids. A wacky combination
+ * of association and conditions can sometimes generate multiple rows per id.
+ *
+ * @return void
+ */
+	public function testDeleteAllMultipleRowsPerId() {
+		$this->loadFixtures('Article', 'User');
+
+		$TestModel = new Article();
+		$TestModel->unbindModel(array(
+			'belongsTo' => array('User'),
+			'hasMany' => array('Comment'),
+			'hasAndBelongsToMany' => array('Tag')
+		), false);
+		$TestModel->bindModel(array(
+			'belongsTo' => array(
+				'User' => array(
+					'foreignKey' => false,
+					'conditions' => array(
+						'Article.user_id = 1'
+					)
+				)
+			)
+		), false);
+
+		$result = $TestModel->deleteAll(
+			array('Article.user_id' => array(1, 3)),
+			true,
+			true
+		);
+
+		$this->assertTrue($result);
+	}
+
+/**
  * testRecursiveDel method
  *
  * @return void