Enh #3: Added the abilty to specify the validation scenario for related records

Exception is now thrown (with a transaction rollback when relevant) in case a related record could not be saved during afterSave event

.gitignore

@@ -4,3 +4,4 @@
 /vendor/
 /tests/report/
 /phpunit.phar
+/tests/runtime/

CHANGELOG.md

@@ -1,8 +1,19 @@
 # Yii2 Active Record Save Relations Behavior Change Log
 
-## [1.2.1]
+## [1.3.0]
 - Unreleased version
 
+### Added
+- Ability to define validation scenario for related records
+- More test cases
+
+### Fixed
+- False positive testLoadRelationsShouldSucceed test case
+
+### Changed
+- afterSave throw exception if a related record fail to be saved
+- related record are now correctly updated based on there primary key (Thanks to @DD174)
+
 ## [1.2.0]
 ### Changed
 - Use of `ActiveQueryInterface` and `BaseActiveRecord` to ensure broader DB driver compatibility (Thx @bookin)

README.md

@@ -135,7 +135,21 @@ Every declared related models will be validated prior to be saved. If any valida
 
 For `hasMany()` relations, the index of the related model will be used to identify the associated error message.
 
-
+It is possible to specify the validation scenario for each relation by declaring an associative array in which the `scenario` key must contain the needed scenario value.
+For instance, in the following configuration, the `links ` related records will be validated using the `Link::SOME_SCENARIO` scenario:
+```php
+...
+    public function behaviors()
+    {
+        return [
+            'saveRelations' => [
+                'class'     => SaveRelationsBehavior::className(),
+                'relations' => ['company', 'users', 'links' => ['scenario' => Link::SOME_SCENARIO]]
+            ],
+        ];
+    }  
+...
+```
 > **Tips :**
 > For relations not involving a junction table by using the `via()` or `viaTable()` methods, you should remove the attributes pointing to the owner model from the 'required' validation rules to be able to pass the validations.
 

src/SaveRelationsBehavior.php

@@ -13,6 +13,7 @@ use yii\db\BaseActiveRecord;
 use yii\db\Transaction;
 use yii\helpers\ArrayHelper;
 use yii\helpers\Inflector;
+use yii\helpers\VarDumper;
 
 /**
  * This Active Record Behavior allows to validate and save the Model relations when the save() method is invoked.
@@ -29,12 +30,13 @@ class SaveRelationsBehavior extends Behavior
     private $_transaction;
 
     private $_relationsScenario = [];
-    private $_relationsCascadeDelete = [];
+
+    //private $_relationsCascadeDelete = []; //TODO
 
     public function init()
     {
         parent::init();
-        $allowedProperties = ['scenario', 'cascadeDelete'];
+        $allowedProperties = ['scenario'];
         foreach ($this->relations as $key => $value) {
             if (is_int($key)) {
                 $this->_relations[] = $value;
@@ -102,7 +104,15 @@ class SaveRelationsBehavior extends Behavior
         if (in_array($name, $this->_relations)) {
             Yii::trace("Setting {$name} relation value", __METHOD__);
             if (!isset($this->_oldRelationValue[$name])) {
-                $this->_oldRelationValue[$name] = $this->owner->{$name};
+                if ($this->owner->isNewRecord) {
+                    if ($this->owner->getRelation($name)->multiple === true) {
+                        $this->_oldRelationValue[$name] = [];
+                    } else {
+                        $this->_oldRelationValue[$name] = null;
+                    }
+                } else {
+                    $this->_oldRelationValue[$name] = $this->owner->{$name};
+                }
             }
             if ($this->owner->getRelation($name)->multiple === true) {
                 $this->setMultipleRelation($name, $value);
@@ -167,15 +177,14 @@ class SaveRelationsBehavior extends Behavior
         // Get the related model foreign keys
         if (is_array($data)) {
             $fks = [];
-            
+
             // search PK
             foreach($modelClass::primaryKey() as $modelAttribute) {
                 if (array_key_exists($modelAttribute, $data) && !empty($data[$modelAttribute])) {
                     $fks[$modelAttribute] = $data[$modelAttribute];
                 }
             }
-
+            if (empty($fks)) {
-            if (!$fks) {
                 // Get the right link definition
                 if ($relation->via instanceof BaseActiveRecord) {
                     $viaQuery = $relation->via;
@@ -186,7 +195,6 @@ class SaveRelationsBehavior extends Behavior
                 } else {
                     $link = $relation->link;
                 }
-
                 foreach ($link as $relatedAttribute => $modelAttribute) {
                     if (array_key_exists($modelAttribute, $data) && !empty($data[$modelAttribute])) {
                         $fks[$modelAttribute] = $data[$modelAttribute];
@@ -317,9 +325,9 @@ class SaveRelationsBehavior extends Behavior
         /** @var BaseActiveRecord $model */
         $model = $this->owner;
         if (!is_null($relationModel) && ($relationModel->isNewRecord || count($relationModel->getDirtyAttributes()))) {
-//            if (key_exists($relationModel, $this->_relationsScenario)) {
+            if (key_exists($relationName, $this->_relationsScenario)) {
-//                $relationModel->setScenario($this->_relationsScenario[$relationModel]);
+                $relationModel->setScenario($this->_relationsScenario[$relationName]);
-//            }
+            }
             Yii::trace("Validating {$pettyRelationName} relation model using " . $relationModel->scenario . " scenario", __METHOD__);
             if (!$relationModel->validate()) {
                 foreach ($relationModel->errors as $attributeErrors) {
@@ -343,56 +351,70 @@ class SaveRelationsBehavior extends Behavior
             /** @var BaseActiveRecord $model */
             $model = $this->owner;
             $this->_relationsSaveStarted = true;
-            foreach ($this->_relations as $relationName) {
+            try {
-                if (array_key_exists($relationName, $this->_oldRelationValue)) { // Relation was not set, do nothing...
+                foreach ($this->_relations as $relationName) {
-                    Yii::trace("Linking {$relationName} relation", __METHOD__);
+                    if (array_key_exists($relationName, $this->_oldRelationValue)) { // Relation was not set, do nothing...
-                    $relation = $model->getRelation($relationName);
+                        Yii::trace("Linking {$relationName} relation", __METHOD__);
-                    if ($relation->multiple === true) { // Has many relation
+                        $relation = $model->getRelation($relationName);
-                        // Process new relations
+                        if ($relation->multiple === true) { // Has many relation
-                        $existingRecords = [];
+                            // Process new relations
-                        /** @var BaseActiveRecord $relationModel */
+                            $existingRecords = [];
-                        foreach ($model->{$relationName} as $relationModel) {
+
-                            if ($relationModel->isNewRecord) {
+                            /** @var BaseActiveRecord $relationModel */
-                                if ($relation->via !== null) {
+                            foreach ($model->{$relationName} as $relationModel) {
-                                    $relationModel->save(false);
+                                if ($relationModel->isNewRecord) {
+                                    if ($relation->via !== null) {
+                                        if (!$relationModel->save()) {
+                                            throw new Exception('Related model ' . $relationName . ' could not be saved (' . VarDumper::dumpAsString($relationModel->getErrors()) . ')');
+                                        }
+                                    }
+                                    $model->link($relationName, $relationModel);
+                                } else {
+                                    $existingRecords[] = $relationModel;
+                                }
+                                if (count($relationModel->dirtyAttributes)) {
+                                    if (!$relationModel->save()) {
+                                        throw new Exception('Related model ' . $relationName . ' could not be saved (' . VarDumper::dumpAsString($relationModel->getErrors()) . ')');
+                                    }
                                 }
-                                $model->link($relationName, $relationModel);
-                            } else {
-                                $existingRecords[] = $relationModel;
                             }
-                            if (count($relationModel->dirtyAttributes)) {
+                            // Process existing added and deleted relations
-                                $relationModel->save(false);
+                            list($addedPks, $deletedPks) = $this->_computePkDiff($this->_oldRelationValue[$relationName], $existingRecords);
+                            // Deleted relations
+                            $initialModels = ArrayHelper::index($this->_oldRelationValue[$relationName], function (BaseActiveRecord $model) {
+                                return implode("-", $model->getPrimaryKey(true));
+                            });
+                            foreach ($deletedPks as $key) {
+                                $model->unlink($relationName, $initialModels[$key], true);
                             }
-                        }
+                            // Added relations
-                        // Process existing added and deleted relations
+                            $actualModels = ArrayHelper::index($model->{$relationName}, function (BaseActiveRecord $model) {
-                        list($addedPks, $deletedPks) = $this->_computePkDiff($this->_oldRelationValue[$relationName], $existingRecords);
+                                return implode("-", $model->getPrimaryKey(true));
-                        // Deleted relations
+                            });
-                        $initialModels = ArrayHelper::index($this->_oldRelationValue[$relationName], function (BaseActiveRecord $model) {
+                            foreach ($addedPks as $key) {
-                            return implode("-", $model->getPrimaryKey(true));
+                                $model->link($relationName, $actualModels[$key]);
-                        });
+                            }
-                        foreach ($deletedPks as $key) {
+                        } else { // Has one relation
-                            $model->unlink($relationName, $initialModels[$key], true);
+                            if ($this->_oldRelationValue[$relationName] !== $model->{$relationName}) {
-                        }
+                                if ($model->{$relationName} instanceof BaseActiveRecord) {
-                        // Added relations
+                                    $model->link($relationName, $model->{$relationName});
-                        $actualModels = ArrayHelper::index($model->{$relationName}, function (BaseActiveRecord $model) {
+                                } else {
-                            return implode("-", $model->getPrimaryKey(true));
+                                    if ($this->_oldRelationValue[$relationName] instanceof BaseActiveRecord) {
-                        });
+                                        $model->unlink($relationName, $this->_oldRelationValue[$relationName]);
-                        foreach ($addedPks as $key) {
+                                    }
-                            $model->link($relationName, $actualModels[$key]);
-                        }
-                    } else { // Has one relation
-                        if ($this->_oldRelationValue[$relationName] !== $model->{$relationName}) {
-                            if ($model->{$relationName} instanceof BaseActiveRecord) {
-                                $model->link($relationName, $model->{$relationName});
-                            } else {
-                                if ($this->_oldRelationValue[$relationName] instanceof BaseActiveRecord) {
-                                    $model->unlink($relationName, $this->_oldRelationValue[$relationName]);
                                 }
                             }
                         }
+                        unset($this->_oldRelationValue[$relationName]);
                     }
-                    unset($this->_oldRelationValue[$relationName]);
                 }
+            } catch (Exception $e) {
+                Yii::warning(get_class($e) . " was thrown during the saving of related records : " . $e->getMessage(), __METHOD__);
+                if (($this->_transaction instanceof Transaction) && $this->_transaction->isActive) {
+                    $this->_transaction->rollBack(); // If anything goes wrong, transaction will be rolled back
+                    Yii::info("Rolling back", __METHOD__);
+                }
+                throw $e;
             }
             $model->refresh();
             $this->_relationsSaveStarted = false;

tests/SaveRelationsBehaviorTest.php

@@ -10,7 +10,6 @@ use tests\models\DummyModel;
 use tests\models\DummyModelParent;
 use tests\models\Link;
 use tests\models\Project;
-use tests\models\ProjectLink;
 use tests\models\ProjectNoTransactions;
 use tests\models\User;
 use Yii;
@@ -258,7 +257,7 @@ class SaveRelationsBehaviorTest extends \PHPUnit_Framework_TestCase
         $this->assertCount(2, $project->users, 'Project should have 2 users before save');
         $project->users = array_merge($project->users, [$user]); // Add new user to the existing list
         $this->assertCount(3, $project->users, 'Project should have 3 users after assignment');
-        $this->assertTrue($project->save(), 'Project could not be saved'.VarDumper::dumpAsString($project->errors));
+        $this->assertTrue($project->save(), 'Project could not be saved' . VarDumper::dumpAsString($project->errors));
         $this->assertCount(3, $project->users, 'Project should have 3 users after save');
     }
 
@@ -407,10 +406,10 @@ class SaveRelationsBehaviorTest extends \PHPUnit_Framework_TestCase
     {
         $project = Project::findOne(1);
         $data = [
-            'Company'     => [
+            'Company' => [
                 'name' => 'YiiSoft'
             ],
-            'ProjectLink' => [
+            'Link'    => [
                 [
                     'language' => 'en',
                     'name'     => 'yii',
@@ -427,6 +426,8 @@ class SaveRelationsBehaviorTest extends \PHPUnit_Framework_TestCase
         $this->assertTrue($project->save(), 'Project could not be saved');
         $this->assertEquals('YiiSoft', $project->company->name, "Company name should be YiiSoft");
         $this->assertCount(2, $project->projectLinks, "Project should have 2 links");
+        $this->assertEquals($project->links[0]->link, 'http://www.yiiframework.com');
+        $this->assertEquals($project->links[1]->link, 'http://www.yiiframework.fr');
     }
 
     public function testAssignSingleObjectToHasManyRelationShouldSucceed()
@@ -460,4 +461,107 @@ class SaveRelationsBehaviorTest extends \PHPUnit_Framework_TestCase
         $this->assertTrue($dummy_a->save(), 'Dummy A could not be saved');
     }
 
+    /**
+     * @expectedException \Exception
+     */
+    public function testSavingRelationWithSameUniqueKeyShouldFail()
+    {
+        $project = new Project();
+        $project->name = "Yii Framework";
+        $data = [
+            'Company' => [
+                'name' => 'NewSoft'
+            ],
+            'Link'    => [
+                [
+                    'language' => 'en',
+                    'name'     => 'newsoft',
+                    'link'     => 'http://www.newsoft.com'
+                ],
+                [
+                    'language' => 'en',
+                    'name'     => 'newsoft',
+                    'link'     => 'http://www.newsoft.co.uk'
+                ]
+            ]
+        ];
+        $project->loadRelations($data);
+        $this->assertFalse($project->save(), 'Project should not be saved');
+    }
+
+    public function testUpdatingAnExistingRelationShouldSucceed()
+    {
+        $project = new Project();
+        $project->name = "Yii Framework";
+        $data = [
+            'Company' => [
+                'name' => 'YiiSoft'
+            ],
+            'Link'    => [
+                [
+                    'language' => 'en',
+                    'name'     => 'yii',
+                    'link'     => 'http://www.yiiframework.ru'
+                ]
+            ]
+        ];
+        $project->loadRelations($data);
+        $this->assertTrue($project->save(), 'Project could not be saved');
+        $this->assertEquals('YiiSoft', $project->company->name, "Company name should be YiiSoft");
+        $this->assertCount(1, $project->projectLinks, "Project should have 1 link");
+        $this->assertEquals($project->links[0]->link, 'http://www.yiiframework.ru');
+        $data = [
+            'Link' => [
+                [
+                    'language' => 'en',
+                    'name'     => 'yii',
+                    'link'     => 'http://www.yiiframework.com'
+                ],
+                [
+                    'language' => 'fr',
+                    'name'     => 'yii',
+                    'link'     => 'http://www.yiiframework.fr'
+                ]
+            ]
+        ];
+        $project->loadRelations($data);
+        $this->assertTrue($project->save(), 'Project could not be saved');
+        $this->assertEquals($project->links[0]->link, 'http://www.yiiframework.com');
+        $this->assertEquals($project->links[1]->link, 'http://www.yiiframework.fr');
+    }
+
+    public function testPerScenarioAttributeValidationShouldSucceed()
+    {
+        $project = new Project();
+        $project->name = "Yii Framework";
+        $data = [
+            'Company' => [
+                'name' => 'YiiSoft'
+            ],
+            'Link'    => [
+                [
+                    'language' => 'en',
+                    'name'     => 'yii',
+                    'link'     => 'Invalid value',
+                ]
+            ]
+        ];
+        $project->loadRelations($data);
+        $this->assertFalse($project->save(), 'Project could be saved');
+        $data = [
+            'Company' => [
+                'name' => 'YiiSoft'
+            ],
+            'Link'    => [
+                [
+                    'language' => 'en',
+                    'name'     => 'yii',
+                    'link'     => 'http://www.yiiframework.com',
+                ]
+            ]
+        ];
+        $project->loadRelations($data);
+        $this->assertTrue($project->save(), 'Project could not be saved');
+    }
+
 }

tests/bootstrap.php

@@ -15,10 +15,19 @@ new \yii\console\Application([
     'id'         => 'unit',
     'basePath'   => __DIR__,
     'vendorPath' => dirname(__DIR__) . '/vendor',
+//    'bootstrap'  => ['log'],
     'components' => [
         'db' => [
             'class' => 'yii\db\Connection',
             'dsn'   => 'sqlite::memory:',
         ],
-    ],
+//        'log' => [
+//            'targets' => [
+//                [
+//                    'class' => 'yii\log\FileTarget',
+//                    'categories' => ['yii\db\*', 'lhs\Yii2SaveRelationsBehavior\*']
+//                ],
+//            ]
+//        ],
+    ]
 ]);

tests/models/Link.php

@@ -6,9 +6,7 @@ use lhs\Yii2SaveRelationsBehavior\SaveRelationsBehavior;
 
 class Link extends \yii\db\ActiveRecord
 {
-
     const SCENARIO_FIRST = 'first';
-    const SCENARIO_SECOND = 'second';
 
     /**
      * @inheritdoc
@@ -39,6 +37,7 @@ class Link extends \yii\db\ActiveRecord
         return [
             [['language', 'name', 'link'], 'required'],
             [['name'], 'unique', 'targetAttribute' => ['language', 'name']],
+            [['link'], 'url', 'on' => [self::SCENARIO_FIRST]],
             [['link_type_id'], 'safe']
         ];
     }

tests/models/ProjectLink.php

@@ -4,6 +4,7 @@ namespace tests\models;
 
 class ProjectLink extends \yii\db\ActiveRecord
 {
+
     /**
      * @inheritdoc
      */
@@ -18,7 +19,8 @@ class ProjectLink extends \yii\db\ActiveRecord
     public function rules()
     {
         return [
-            [['language', 'name', 'project_id'], 'required']
+            [['language', 'name', 'project_id'], 'required'],
+            [['language', 'name'], 'unique']
         ];
     }