Permalink
Browse files

Fix `DetailableBehavior` bug on `Model::saveAll`. Closes #2.

  • Loading branch information...
1 parent 09bd89c commit 14f01d933cfa81f8e3cf7c780f979e31c891e03d @jadb jadb committed Feb 28, 2014
@@ -105,6 +105,11 @@ public function afterValidate(Model $Model) {
public function afterSave(Model $Model, $created = false) {
$DetailModel = ClassRegistry::init($this->settings[$Model->alias]['alias']);
+ // Reset DetailModel data that was unset before validation.
+ if (!empty($this->__dataToSave[$Model->alias])) {
+ $Model->data[$DetailModel->alias] = $this->__dataToSave[$Model->alias];
@jadb

jadb Feb 28, 2014

Owner

Shouldn't DetailableBehavior::$__dataToSave be unset at this point?

+ }
+
if ($created && empty($Model->data[$DetailModel->alias]) && false === $this->detailsCreateDefaults($Model)) {
return false;
}
@@ -115,6 +120,21 @@ public function afterSave(Model $Model, $created = false) {
}
/**
+ * {@inheritdoc}
+ */
+ public function beforeValidate(Model $Model) {
+ $DetailModel = ClassRegistry::init($this->settings[$Model->alias]['alias']);
+
+ // Unset DetailModel data so it is not included in `Model::validateMany()`.
+ if (!empty($Model->data[$DetailModel->alias])) {
+ $this->__dataToSave[$Model->alias] = $Model->data[$DetailModel->alias];
@jadb

jadb Feb 28, 2014

Owner

Undefined property DetailableBehavior::$__dataToSave.

+ unset($Model->data[$DetailModel->alias]);
+ }
+
+ return true;
+ }
+
+/**
* [detailsCreateDefaults description]
* @param Model $Model [description]
* @param [type] $id [description]
@@ -280,7 +300,7 @@ public function detailsSaveSections(Model $Model, $sections = array()) {
}
$data += array('value' => $val);
- if (!$DetailModel->save($data, false)) {
+ if (!$DetailModel->save($data, array('validate' => false, 'bypass' => false))) {
return false;
}
}
@@ -161,6 +161,38 @@ public function getSection($id, $section = null, $schema = false, $format = true
}
/**
+ * By-pass the save operation by default. This is a workaround so only the `DetailableBehavior`
+ * can save records after settings the required fields.
+ *
+ * @param array $data Data to save.
+ * @param boolean|array $validate Either a boolean, or an array.
+ * If a boolean, indicates whether or not to validate before saving.
+ * If an array, can have following keys:
+ *
+ * - 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.
+ * - bypass: Set to false to force the save operation.
+ *
+ * @param array $fieldList List of fields to allow to be saved
+ * @return mixed On success Model::$data if its not empty or true, false on failure
+ */
+ public function save($data = null, $validate = true, $fieldList = array()) {
+ if (!is_array($validate)) {
+ $validate = compact('validate', 'fieldList');
+ }
+
+ $defaults = array('validate' => true, 'fieldList' => $fieldList, 'callbacks' => true, 'bypass' => true);
+ $options = array_merge($defaults, $validate);
+ if ($options['bypass']) {
+ return true;
+ }
+
+ return parent::save($data, $options);
+ }
+
+/**
* Saves associated details. Validates data only if the data is saved by section (not all sections together).
*
* Usage:
@@ -7,6 +7,8 @@ class CommonTestDetailableUser extends CommonAppModel {
public $actsAs = array('Common.Detailable');
+ public $hasMany = array('CommonTestDetailableBook' => array('foreignKey' => 'user_id'));
+
public $detailSchema = array(
'user' => array(
'fname' => array('type' => 'string', 'length' => 16),
@@ -30,6 +32,7 @@ class DetailableBehaviorTest extends CommonTestCase {
*/
public $fixtures = array(
'plugin.common.common_detail_model',
+ 'plugin.common.common_test_detailable_book',
'plugin.common.common_test_detailable_user',
);
@@ -133,4 +136,27 @@ public function testAfterSave() {
$this->assertEqual($result, $expected);
}
+ public function testAfterSaveTriggeredBySaveAll() {
+ $detailModel = $this->User->alias . 'Detail';
+
+ $data = array(
+ $this->User->alias => array('email' => 'test@example.com'),
+ $detailModel => array('user' => array('fname' => 'test', 'lname' => 'example')),
+ 'CommonTestDetailableBook' => array(array('name' => 'Test Book')),
+ );
+
+ $this->User->saveAll($data);
+
+ $expected = 2;
+ $result = $this->User->{$detailModel}->find('count', array('conditions' => array('foreign_model' => $this->User->alias, 'foreign_key' => $this->User->id)));
+ $this->assertEqual($result, $expected);
+
+
+ $data[$this->User->alias]['email'] = 'anothertest@example.com';
+ $this->User->saveAll($data, array('validate' => false));
+
+ $expected = 2;
+ $result = $this->User->{$detailModel}->find('count', array('conditions' => array('foreign_model' => $this->User->alias, 'foreign_key' => $this->User->id)));
+ $this->assertEqual($result, $expected);
+ }
}
@@ -0,0 +1,22 @@
+<?php
+/**
+ * CommonTestDetailableBookFixture
+ *
+ */
+class CommonTestDetailableBookFixture extends CommonTestFixture {
+
+/**
+ * {@inheritdoc}
+ */
+ public $fields = array(
+ 'id' => array('type' => 'string', 'null' => false, 'default' => NULL, 'length' => 36, 'key' => 'primary'),
+ 'user_id' => array('type' => 'string', 'null' => false, 'default' => NULL, 'length' => 36),
+ 'name' => array('type' => 'string', 'null' => true, 'default' => NULL),
+ 'created' => array('type' => 'datetime', 'null' => false, 'default' => NULL),
+ 'modified' => array('type' => 'datetime', 'null' => false, 'default' => NULL),
+ 'indexes' => array(
+ 'PRIMARY' => array('column' => 'id', 'unique' => 1),
+ ),
+ );
+
+}

0 comments on commit 14f01d9

Please sign in to comment.