diff --git a/src/SaveRelationsBehavior.php b/src/SaveRelationsBehavior.php index d0c12d7..af5f854 100644 --- a/src/SaveRelationsBehavior.php +++ b/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_UPDATE => 'afterSave', + ActiveRecord::EVENT_AFTER_INSERT => '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(); } } diff --git a/tests/SaveRelationsBehaviorTest.php b/tests/SaveRelationsBehaviorTest.php index f39fa6b..76e949c 100644 --- a/tests/SaveRelationsBehaviorTest.php +++ b/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(), - 'name' => $migration->string()->notNull(), - 'link' => $migration->string()->notNull(), + 'language' => $migration->string(5)->notNull(), + 'name' => $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'], [ - ['fr', 'mac_os_x', 'http://www.apple.com/fr/osx/'], - ['en', 'mac_os_x', 'http://www.apple.com/osx/'] + $db->createCommand()->batchInsert('link_type', ['id', 'name'], [ + [1, 'public'], + [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/en-us/windows/features", $project->links[3]->link, 'Third link should be https://www.microsoft.com/en-us/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'); + $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"); + } + } diff --git a/tests/models/Link.php b/tests/models/Link.php index a8bf7c8..3ed8eaa 100644 --- a/tests/models/Link.php +++ b/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']); + } } diff --git a/tests/models/LinkType.php b/tests/models/LinkType.php new file mode 100644 index 0000000..e3ebac0 --- /dev/null +++ b/tests/models/LinkType.php @@ -0,0 +1,25 @@ + [ + '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() {