Model::deleteAll() now returns false if the 'find' to fetch records i…

…ds returns false (in case of sql error). Closes #272
cakephp · Apr 24, 2010 · 46df1be · 46df1be
1 parent 3e85577
commit 46df1be
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 6 deletions.
diff --git a/cake/libs/model/model.php b/cake/libs/model/model.php
@@ -1907,14 +1907,15 @@ function deleteAll($conditions, $cascade = true, $callbacks = false) {
 		if (!$cascade && !$callbacks) {
 			return $db->delete($this, $conditions);
 		} else {
-			$ids = Set::extract(
-				$this->find('all', array_merge(array(
-					'fields' => "{$this->alias}.{$this->primaryKey}",
-					'recursive' => 0), compact('conditions'))
-				),
-				"{n}.{$this->alias}.{$this->primaryKey}"
+			$ids = $this->find('all', array_merge(array(
+				'fields' => "{$this->alias}.{$this->primaryKey}",
+				'recursive' => 0), compact('conditions'))
 			);
+			if ($ids === false) {
+				return false;
+			}
 
+			$ids = Set::extract($ids, "{n}.{$this->alias}.{$this->primaryKey}");
 			if (empty($ids)) {
 				return true;
 			}

diff --git a/cake/tests/cases/libs/model/model_delete.test.php b/cake/tests/cases/libs/model/model_delete.test.php
@@ -423,6 +423,12 @@ function testDeleteAll() {
 
 		$result = $TestModel->deleteAll(array('Article.user_id' => 999));
 		$this->assertTrue($result, 'deleteAll returned false when all no records matched conditions. %s');
+
+		$this->expectError();
+		ob_start();
+		$result = $TestModel->deleteAll(array('Article.non_existent_field' => 999));
+		ob_get_clean();
+		$this->assertFalse($result, 'deleteAll returned true when find query generated sql error. %s');
 	}
 
 /**