Fix #1 issue: Setting null for a named relation is correctly handled now

src/SaveRelationsBehavior.php

@@ -9,6 +9,7 @@ use yii\base\Exception;
 use yii\base\ModelEvent;
 use yii\db\ActiveQuery;
 use yii\db\ActiveRecord;
+use yii\db\Transaction;
 use yii\helpers\ArrayHelper;
 use yii\helpers\Inflector;
 
@@ -29,8 +30,8 @@ class SaveRelationsBehavior extends Behavior
     {
         return [
             ActiveRecord::EVENT_BEFORE_VALIDATE => 'beforeValidate',
-            ActiveRecord::EVENT_AFTER_INSERT => 'afterSave',
+            ActiveRecord::EVENT_AFTER_INSERT    => 'afterSave',
-            ActiveRecord::EVENT_AFTER_UPDATE => 'afterSave',
+            ActiveRecord::EVENT_AFTER_UPDATE    => 'afterSave',
         ];
     }
 
@@ -74,7 +75,6 @@ class SaveRelationsBehavior extends Behavior
     public function __set($name, $value)
     {
         if (in_array($name, $this->relations)) {
-            //echo "Setting " . $name . "\n";
             /** @var ActiveRecord $model */
             $model = $this->owner;
             Yii::trace("Setting {$name} relation value", __METHOD__);
@@ -126,16 +126,16 @@ class SaveRelationsBehavior extends Behavior
         } else {
             $fks = $data;
         }
-        // Load existing model or create one if no key was provided
+        // Load existing model or create one if no key was provided and data is not empty
         /** @var ActiveRecord $relationModel */
         $relationModel = null;
         if (!empty($fks)) {
             $relationModel = $modelClass::findOne($fks);
         }
-        if (!$relationModel) {
+        if (!($relationModel instanceof ActiveRecord) && !empty($data)) {
             $relationModel = new $modelClass;
         }
-        if (is_array($data)) {
+        if (($relationModel instanceof ActiveRecord) && is_array($data)) {
             $relationModel->setAttributes($data);
         }
         return $relationModel;
@@ -248,8 +248,7 @@ class SaveRelationsBehavior extends Behavior
         $relationName,
         ActiveRecord $relationModel,
         ModelEvent $event
-    )
+    ) {
-    {
         /** @var ActiveRecord $model */
         $model = $this->owner;
         if (!is_null($relationModel) && ($relationModel->isNewRecord || count($relationModel->getDirtyAttributes()))) {
@@ -301,7 +300,13 @@ class SaveRelationsBehavior extends Behavior
                         }
                     } else { // Has one relation
                         if ($this->_oldRelationValue[$relationName] != $model->{$relationName}) {
-                            $model->link($relationName, $model->{$relationName});
+                            if ($model->{$relationName} instanceof ActiveRecord) {
+                                $model->link($relationName, $model->{$relationName});
+                            } else {
+                                if ($this->_oldRelationValue[$relationName] instanceof ActiveRecord) {
+                                    $model->unlink($relationName, $this->_oldRelationValue[$relationName]);
+                                }
+                            }
                         }
                     }
                     unset($this->_oldRelationValue[$relationName]);
@@ -309,7 +314,7 @@ class SaveRelationsBehavior extends Behavior
             }
             $model->refresh();
             $this->_relationsSaveStarted = false;
-            if ($this->_transaction->isActive) {
+            if (($this->_transaction instanceof Transaction) && $this->_transaction->isActive) {
                 $this->_transaction->commit();
             }
         }

tests/SaveRelationsBehaviorTest.php

@@ -29,6 +29,7 @@ class SaveRelationsBehaviorTest extends \PHPUnit_Framework_TestCase
         $db->createCommand()->dropTable('project')->execute();
         $db->createCommand()->dropTable('user')->execute();
         $db->createCommand()->dropTable('company')->execute();
+        $db->createCommand()->dropTable('link_type')->execute();
         $db->createCommand()->dropTable('link')->execute();
         $db->createCommand()->dropTable('project_link')->execute();
         parent::tearDown();
@@ -66,12 +67,18 @@ class SaveRelationsBehaviorTest extends \PHPUnit_Framework_TestCase
         $db->createCommand()->createIndex('company_id-name', 'project', 'company_id,name', true)->execute();
 
         $db->createCommand()->createTable('link', [
-            'language' => $migration->string(5)->notNull(),
+            'language'     => $migration->string(5)->notNull(),
-            'name'     => $migration->string()->notNull(),
+            'name'         => $migration->string()->notNull(),
-            'link'     => $migration->string()->notNull(),
+            'link'         => $migration->string()->notNull(),
+            'link_type_id' => $migration->integer(),
             'PRIMARY KEY(language, name)'
         ])->execute();
 
+        $db->createCommand()->createTable('link_type', [
+            'id'   => $migration->primaryKey(),
+            'name' => $migration->string()->notNull()->unique()
+        ])->execute();
+
         $db->createCommand()->createTable('project_link', [
             'language'   => $migration->string(5)->notNull(),
             'name'       => $migration->string()->notNull(),
@@ -108,9 +115,14 @@ class SaveRelationsBehaviorTest extends \PHPUnit_Framework_TestCase
             [2, 'Windows 10', 2]
         ])->execute();
 
-        $db->createCommand()->batchInsert('link', ['language', 'name', 'link'], [
+        $db->createCommand()->batchInsert('link_type', ['id', 'name'], [
-            ['fr', 'mac_os_x', 'http://www.apple.com/fr/osx/'],
+            [1, 'public'],
-            ['en', 'mac_os_x', 'http://www.apple.com/osx/']
+            [2, 'private']
+        ])->execute();
+
+        $db->createCommand()->batchInsert('link', ['language', 'name', 'link', 'link_type_id'], [
+            ['fr', 'mac_os_x', 'http://www.apple.com/fr/osx/', 1],
+            ['en', 'mac_os_x', 'http://www.apple.com/osx/', 1]
         ])->execute();
 
         $db->createCommand()->batchInsert('project_link', ['language', 'name', 'project_id'], [
@@ -278,7 +290,8 @@ class SaveRelationsBehaviorTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(3, count($project->links), 'Project should have 3 links after assignment');
         $this->assertTrue($project->save(), 'Project could not be saved');
         $this->assertEquals(3, count($project->links), 'Project should have 3 links after save');
-        $this->assertEquals("https://www.microsoft.com/fr-fr/windows/features", $project->links[2]->link, 'Second link should be https://www.microsoft.com/fr-fr/windows/features');
+        $this->assertEquals("https://www.microsoft.com/fr-fr/windows/features", $project->links[2]->link,
+            'Second link should be https://www.microsoft.com/fr-fr/windows/features');
     }
 
     public function testSaveNewHasManyRelationWithCompositeFksAsArrayShouldSucceed()
@@ -293,8 +306,10 @@ class SaveRelationsBehaviorTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(4, count($project->links), 'Project should have 4 links after assignment');
         $this->assertTrue($project->save(), 'Project could not be saved');
         $this->assertEquals(4, count($project->links), 'Project should have 4 links after save');
-        $this->assertEquals("https://www.microsoft.com/fr-fr/windows/features", $project->links[2]->link, 'Second link should be https://www.microsoft.com/fr-fr/windows/features');
+        $this->assertEquals("https://www.microsoft.com/fr-fr/windows/features", $project->links[2]->link,
-        $this->assertEquals("https://www.microsoft.com/en-us/windows/features", $project->links[3]->link, 'Third link should be https://www.microsoft.com/en-us/windows/features');
+            'Second link should be https://www.microsoft.com/fr-fr/windows/features');
+        $this->assertEquals("https://www.microsoft.com/en-us/windows/features", $project->links[3]->link,
+            'Third link should be https://www.microsoft.com/en-us/windows/features');
     }
 
     public function testSaveUpdatedHasManyRelationWithCompositeFksAsArrayShouldSucceed()
@@ -305,7 +320,8 @@ class SaveRelationsBehaviorTest extends \PHPUnit_Framework_TestCase
         $links[1]->link = "http://www.otherlink.com/";
         $project->links = $links;
         $this->assertTrue($project->save(), 'Project could not be saved');
-        $this->assertEquals("http://www.otherlink.com/", $project->links[1]->link, 'Second link "Link" attribute should be "http://www.otherlink.com/"');
+        $this->assertEquals("http://www.otherlink.com/", $project->links[1]->link,
+            'Second link "Link" attribute should be "http://www.otherlink.com/"');
     }
 
     public function testSaveMixedRelationsShouldSucceed()
@@ -313,7 +329,7 @@ class SaveRelationsBehaviorTest extends \PHPUnit_Framework_TestCase
         $project = new Project();
         $project->name = "New project";
         $project->company = Company::findOne(2);
-        $users = User::findAll([1,3]);
+        $users = User::findAll([1, 3]);
         $this->assertEquals(0, count($project->users), 'Project should have 0 users before save');
         $project->users = $users; // Add users
         $this->assertEquals(2, count($project->users), 'Project should have 2 users after assignment');
@@ -322,5 +338,27 @@ class SaveRelationsBehaviorTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(2, $project->company_id, 'Company ID is not the one expected');
     }
 
+    public function testSettingANullRelationShouldSucceed()
+    {
+        $link = new Link();
+        $link->language = 'en';
+        $link->name = 'yii';
+        $link->link = 'http://www.yiiframework.com';
+        $link->linkType = null;
+        $this->assertTrue($link->save(), 'Link could not be saved');
+        $this->assertNull($link->linkType, "Link type should be null");
+        $this->assertNull($link->link_type_id, "Link type id should be null");
+    }
+
+    public function testUnsettingARelationShouldSucceed()
+    {
+        $link = Link::findOne(['language' => 'fr', 'name' => 'mac_os_x']);
+        $this->assertEquals(1, $link->link_type_id, 'Link type id should be 1');
+        $link->linkType = null;
+        $this->assertTrue($link->save(), 'Link could not be saved');
+        $this->assertNull($link->linkType, "Link type should be null");
+        $this->assertNull($link->link_type_id, "Link type id should be null");
+    }
+
 
 }

tests/models/Link.php

@@ -2,6 +2,8 @@
 
 namespace tests\models;
 
+use lhs\Yii2SaveRelationsBehavior\SaveRelationsBehavior;
+
 class Link extends \yii\db\ActiveRecord
 {
     /**
@@ -15,12 +17,33 @@ class Link extends \yii\db\ActiveRecord
     /**
      * @inheritdoc
      */
+    public function behaviors()
+    {
+        return [
+            'saveRelations' => [
+                'class'     => SaveRelationsBehavior::className(),
+                'relations' => ['linkType']
+            ],
+        ];
+    }
+
+    /**
+     * @inheritdoc
+     */
     public function rules()
     {
         return [
             [['language', 'name', 'link'], 'required'],
             [['name'], 'unique', 'targetAttribute' => ['language', 'name']],
+            [['link_type_id'], 'safe']
         ];
     }
 
+    /**
+     * @return \yii\db\ActiveQuery
+     */
+    public function getLinkType()
+    {
+        return $this->hasOne(LinkType::className(), ['id' => 'link_type_id']);
+    }
 }

tests/models/LinkType.php

@@ -0,0 +1,25 @@
+<?php
+
+namespace tests\models;
+
+class LinkType extends \yii\db\ActiveRecord
+{
+    /**
+     * @inheritdoc
+     */
+    public static function tableName()
+    {
+        return 'link_type';
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function rules()
+    {
+        return [
+            [['name'], 'required'],
+            [['name'], 'unique']
+        ];
+    }
+}

tests/models/Project.php

@@ -2,7 +2,6 @@
 
 namespace tests\models;
 
-
 use lhs\Yii2SaveRelationsBehavior\SaveRelationsBehavior;
 
 class Project extends \yii\db\ActiveRecord
@@ -21,7 +20,7 @@ class Project extends \yii\db\ActiveRecord
     public function behaviors()
     {
         return [
-            'translateable' => [
+            'saveRelations' => [
                 'class'     => SaveRelationsBehavior::className(),
                 'relations' => ['company', 'users', 'links']
             ],
@@ -50,7 +49,7 @@ class Project extends \yii\db\ActiveRecord
     }
 
     /**
-     * @return ActiveQuery
+     * @return \yii\db\ActiveQuery
      */
     public function getCompany()
     {
@@ -58,7 +57,7 @@ class Project extends \yii\db\ActiveRecord
     }
 
     /**
-     * @return ActiveQuery
+     * @return \yii\db\ActiveQuery
      */
     public function getProjectUsers()
     {
@@ -66,7 +65,7 @@ class Project extends \yii\db\ActiveRecord
     }
 
     /**
-     * @return ActiveQuery
+     * @return \yii\db\ActiveQuery
      */
     public function getUsers()
     {
@@ -74,7 +73,7 @@ class Project extends \yii\db\ActiveRecord
     }
 
     /**
-     * @return ActiveQuery
+     * @return \yii\db\ActiveQuery
      */
     public function getProjectLinks()
     {
@@ -82,7 +81,7 @@ class Project extends \yii\db\ActiveRecord
     }
 
     /**
-     * @return ActiveQuery
+     * @return \yii\db\ActiveQuery
      */
     public function getLinks()
     {