Fix issues saveMany & saveAssociated with boolean values.

For non-atomic, save operations that include models with boolean fields. The first false value would cause the save to abort. This regression was introduced in #6947. Instead of checking the data from save() we should be boolean casting save() to capture the success/failure. Refs #7069
cakephp · Jul 22, 2015 · c6e5026 · c6e5026
1 parent 27e40f0
commit c6e5026
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 4 deletions.
diff --git a/lib/Cake/Model/Model.php b/lib/Cake/Model/Model.php
@@ -1717,7 +1717,7 @@ public function saveField($name, $value, $validate = false) {
  *   - validate: Set to true/false to enable or disable validation.
  *   - fieldList: An array of fields you want to allow for saving.
  *   - callbacks: Set to false to disable callbacks. Using 'before' or 'after'
- *      will enable only those callbacks.
+ *     will enable only those callbacks.
  *   - `counterCache`: Boolean to control updating of counter caches (if any)
  *
  * @param array $fieldList List of fields to allow to be saved
@@ -2335,7 +2335,7 @@ public function saveMany($data = null, $options = array()) {
 					if ($options['deep']) {
 						$saved = $this->saveAssociated($record, array('atomic' => false) + $options);
 					} else {
-						$saved = $this->save($record, array('atomic' => false) + $options);
+						$saved = (bool)$this->save($record, array('atomic' => false) + $options);
 					}
 				}
 
@@ -2478,7 +2478,7 @@ public function saveAssociated($data = null, $options = array()) {
 					if ($options['deep']) {
 						$saved = $Model->saveAssociated($values, array('atomic' => false) + $options);
 					} else {
-						$saved = $Model->save($values, array('atomic' => false) + $options);
+						$saved = (bool)$Model->save($values, array('atomic' => false) + $options);
 					}
 					$validates = ($saved === true || (is_array($saved) && !in_array(false, Hash::flatten($saved), true)));
 				}
@@ -2534,7 +2534,7 @@ public function saveAssociated($data = null, $options = array()) {
 							if ($options['deep']) {
 								$saved = $Model->saveAssociated($values, array('atomic' => false) + $options);
 							} else {
-								$saved = $Model->save($values, $options);
+								$saved = (bool)$Model->save($values, $options);
 							}
 						}
 

diff --git a/lib/Cake/Test/Case/Model/ModelWriteTest.php b/lib/Cake/Test/Case/Model/ModelWriteTest.php
@@ -7684,6 +7684,37 @@ public function testSaveAllDeepEmptyHasManyHasMany() {
 		$this->assertEquals(2, count($result['Attachment']));
 	}
 
+/**
+ * Test that boolean fields don't cause saveMany to fail
+ *
+ * @return void
+ */
+	public function testSaveManyBooleanFields() {
+		$this->loadFixtures('Item', 'Syfile', 'Image');
+		$data = array(
+			array(
+				'Item' => array(
+					'name' => 'testing',
+					'syfile_id' => 1,
+					'published' => false
+				)
+			),
+			array(
+				'Item' => array(
+					'name' => 'testing 2',
+					'syfile_id' => 1,
+					'published' => true
+				)
+			),
+		);
+		$item = ClassRegistry::init('Item');
+		$result = $item->saveMany($data, array('atomic' => false));
+
+		$this->assertCount(2, $result, '2 records should have been saved.');
+		$this->assertTrue($result[0], 'Both should have succeded');
+		$this->assertTrue($result[1], 'Both should have succeded');
+	}
+
 /**
  * testSaveManyDeepHasManyValidationFailure method
  *
@@ -7806,6 +7837,65 @@ public function testSaveAssociatedDeepBelongsToHasManyValidateTrueValidationFail
 		), $TestModel->validationErrors);
 	}
 
+/**
+ * Test that boolean fields don't cause saveAssociated to fail
+ *
+ * @return void
+ */
+	public function testSaveAssociatedHasOneBooleanFields() {
+		$this->loadFixtures('Item', 'Syfile', 'Image');
+		$data = array(
+			'Syfile' => array(
+				'image_id' => 1,
+				'name' => 'Some file',
+			),
+			'Item' => array(
+				'name' => 'testing',
+				'published' => false
+			),
+		);
+		$syfile = ClassRegistry::init('Syfile');
+		$syfile->bindModel(array('hasOne' => array('Item')), false);
+		$result = $syfile->saveAssociated($data, array('atomic' => false));
+
+		$this->assertCount(2, $result, '2 records should have been saved.');
+		$this->assertTrue($result['Syfile'], 'Both should have succeded');
+		$this->assertTrue($result['Item'], 'Both should have succeded');
+	}
+
+/**
+ * Test that boolean fields don't cause saveAssociated to fail
+ *
+ * @return void
+ */
+	public function testSaveAssociatedBelongsToBooleanFields() {
+		$this->loadFixtures('Item', 'Syfile', 'Image');
+		$data = array(
+			'Syfile' => array(
+				'image_id' => 1,
+				'name' => 'Some file',
+			),
+			'Item' => array(
+				'name' => 'testing',
+				'syfile_id' => 2,
+				'published' => false
+			),
+		);
+		$item = ClassRegistry::init('Item');
+		$item->bindModel(array(
+			'belongsTo' => array(
+				'Item' => array(
+					'foreignKey' => 'image_id'
+				)
+			)
+		), false);
+		$result = $item->saveAssociated($data, array('atomic' => false));
+
+		$this->assertCount(2, $result, '2 records should have been saved.');
+		$this->assertTrue($result['Syfile'], 'Both should have succeded');
+		$this->assertTrue($result['Item'], 'Both should have succeded');
+	}
+
 /**
  * testUpdateAllBoolean
  *