Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Some issue with one-to-one relationship #22

Closed
mythicallage opened this issue Oct 29, 2017 · 12 comments
Closed

Some issue with one-to-one relationship #22

mythicallage opened this issue Oct 29, 2017 · 12 comments
Assignees
Labels
Milestone

Comments

@mythicallage
Copy link

mythicallage commented Oct 29, 2017

Thank you for your useful extension. I would like to give you feedback to enhance your extension. Version is 1.3.1.

I have some issue with saving new models Dummy and DummyBrother if I try to save with validation. There will be an SQL error anyway. Saving Dummy and DummyMany models works fine.

2017-10-29 22:07:15 [127.0.0.1][-][-][warning][lhs\Yii2SaveRelationsBehavior\SaveRelationsBehavior::saveRelatedRecords] yii\db\Exception was thrown while saving related records during beforeValidate event: SQLSTATE[HY000]: General error: 1364 Field 'dummy_id' doesn't have a default value

I have to save Dummy model without validation. It works if I do only like that.

I would be grateful if this case would be considered in tests.

...
class Dummy extends \yii\db\ActiveRecord
{
    public function behaviors()
    {
        return [
            'saveRelations' => [
                'class'    => SaveRelationsBehavior::class,
                'relations' => ['dummyBrother', 'dummyManies']
            ],
        ];
    }

    public function transactions()
    {
        return [
            self::SCENARIO_DEFAULT => self::OP_ALL,
        ];
    }
...
CREATE TABLE dummy (
  id int(11) NOT NULL AUTO_INCREMENT,
  name varchar(50) NOT NULL,
  PRIMARY KEY (id)
)
ENGINE = INNODB
AUTO_INCREMENT = 1
CHARACTER SET utf8
COLLATE utf8_general_ci
ROW_FORMAT = DYNAMIC;

CREATE TABLE dummy_brother (
  dummy_id int(11) NOT NULL,
  name varchar(50) NOT NULL,
  PRIMARY KEY (dummy_id),
  UNIQUE INDEX UK_dummy_brother_dummy_id (dummy_id),
  CONSTRAINT FK_dummy_brother_dummy_id FOREIGN KEY (dummy_id)
  REFERENCES dummy (id) ON DELETE NO ACTION ON UPDATE RESTRICT
)
ENGINE = INNODB
CHARACTER SET utf8
COLLATE utf8_general_ci
ROW_FORMAT = DYNAMIC;

CREATE TABLE dummy_many (
  id int(11) NOT NULL AUTO_INCREMENT,
  dummy_id int(11) NOT NULL,
  name varchar(50) NOT NULL,
  PRIMARY KEY (id),
  CONSTRAINT FK_dummy_many_dummy_id FOREIGN KEY (dummy_id)
  REFERENCES dummy (id) ON DELETE NO ACTION ON UPDATE RESTRICT
)
ENGINE = INNODB
AUTO_INCREMENT = 1
CHARACTER SET utf8
COLLATE utf8_general_ci
ROW_FORMAT = DYNAMIC;
@juban juban self-assigned this Oct 30, 2017
@juban
Copy link
Contributor

juban commented Oct 30, 2017

Hi,

Could you provide your models definitions, specially the relations part please?

Thanks.

@mythicallage
Copy link
Author

Hi,
Sure, here it is.

/**
 * This is the model class for table "dummy".
 *
 * @property integer $id
 * @property string $name
 *
 * @property DummyBrother $dummyBrother
 * @property DummyMany[] $dummyManies
 */
class Dummy extends \yii\db\ActiveRecord
{
    public function behaviors()
    {
        return [
            'saveRelations' => [
                'class'    => SaveRelationsBehavior::class,
                'relations' => ['dummyBrother', 'dummyManies']
            ],
        ];
    }

    public function transactions()
    {
        return [
            self::SCENARIO_DEFAULT => self::OP_ALL,
        ];
    }

    public static function tableName()
    {
        return 'dummy';
    }

    /**
     * @inheritdoc
     */
    public function rules()
    {
        return [
            [['name'], 'required'],
            [['name'], 'string', 'max' => 50],
        ];
    }

    /**
     * @inheritdoc
     */
    public function attributeLabels()
    {
        return [
            'id' => 'ID',
            'name' => 'Name',
        ];
    }

    /**
     * @return \yii\db\ActiveQuery
     */
    public function getDummyBrother()
    {
        return $this->hasOne(DummyBrother::class, ['dummy_id' => 'id']);
    }

    /**
     * @return \yii\db\ActiveQuery
     */
    public function getDummyManies()
    {
        return $this->hasMany(DummyMany::class, ['dummy_id' => 'id']);
    }
}

/**
 * This is the model class for table "dummy_brother".
 *
 * @property integer $dummy_id
 * @property string $name
 *
 * @property Dummy $dummy
 */
class DummyBrother extends \yii\db\ActiveRecord
{
    public static function create(int $id, string $name): self
    {
        $item = new static();
//        $item->dummy_id = $id;
        $item->name = $name;
        return $item;
    }

    public static function tableName()
    {
        return 'dummy_brother';
    }

    /**
     * @inheritdoc
     */
    public function rules()
    {
        return [
            [[/*'dummy_id', */'name'], 'required'],
            [['dummy_id'], 'integer'],
            [['name'], 'string', 'max' => 50],
            [['dummy_id'], 'unique'],
//            [['dummy_id'], 'exist', 'skipOnError' => true, 'targetClass' => Dummy::class, 'targetAttribute' => ['dummy_id' => 'id']],
        ];
    }

    /**
     * @inheritdoc
     */
    public function attributeLabels()
    {
        return [
            'dummy_id' => 'Dummy ID',
            'name' => 'Name',
        ];
    }

    /**
     * @return \yii\db\ActiveQuery
     */
    public function getDummy()
    {
        return $this->hasOne(Dummy::class, ['id' => 'dummy_id']);
    }
}

juban pushed a commit that referenced this issue Nov 2, 2017
…d models are linked using there primary key, the owner attribute should not be updated in order to pass the validation

Added test cases for #22 issue
@juban
Copy link
Contributor

juban commented Nov 2, 2017

Hi @mythicallage

I have pushed fixes to the master branch that should fix the issue.
Could you tell me is that works for you?

Thanks.

@mythicallage
Copy link
Author

Hi @juban ,

I am sorry It does not work still, but probably I found a solution to fix the issue. I have added only one condition. It starts with the line "//It needs for excluding scenario like".
If it will not pass the tests, please let me know.

Thanks.

    /**
     * For each related model, try to save it first.
     * If set in the owner model, operation is done in a transactional way so if one of the models should not validate
     * or be saved, a rollback will occur.
     * This is done during the before validation process to be able to set the related foreign keys.
     * @param BaseActiveRecord $model
     * @param ModelEvent $event
     * @return bool
     */
    protected function saveRelatedRecords(BaseActiveRecord $model, ModelEvent $event)
    {
        if (($model->isNewRecord && $model->isTransactional($model::OP_INSERT)) || (!$model->isNewRecord && $model->isTransactional($model::OP_UPDATE)) || $model->isTransactional($model::OP_ALL)) {
            $this->_transaction = $model->getDb()->beginTransaction();
        }
        try {
            foreach ($this->_relations as $relationName) {
                if (array_key_exists($relationName, $this->_oldRelationValue)) { // Relation was not set, do nothing...
                    $relation = $model->getRelation($relationName);
                    if (!empty($model->{$relationName})) {
                        if ($relation->multiple === false) {
                            $relationModel = $model->{$relationName};
                            //It needs for excluding scenario like \yii\db\BaseActiveRecord::link (InvalidCallException('Unable to link models: at most one model can be newly created'))
                            if (!($model::isPrimaryKey(array_values($relation->link))
                                && $relationModel::isPrimaryKey(array_keys($relation->link))
                                && $model->getIsNewRecord()
                                && $relationModel->getIsNewRecord()
                            )) {
                                // Save Has one relation new record
                                $pettyRelationName = Inflector::camel2words($relationName, true);
                                $this->saveModelRecord($relationModel, $event, $pettyRelationName, $relationName);
                            }
                        } else {
                            // Save Has many relations new records
                            /** @var BaseActiveRecord $relationModel */
                            foreach ($model->{$relationName} as $i => $relationModel) {
                                $pettyRelationName = Inflector::camel2words($relationName, true) . " #{$i}";
                                $this->validateRelationModel($pettyRelationName, $relationName, $relationModel, $event);
                            }
                        }
                    }
                }
            }
            if (!$event->isValid) {
                throw new Exception("One of the related model could not be validated");
            }
        } catch (Exception $e) {
            Yii::warning(get_class($e) . " was thrown while saving related records during beforeValidate event: " . $e->getMessage(), __METHOD__);
            $this->_rollback();
            $event->isValid = false; // Stop saving, something went wrong
            return false;
        }
        return true;
    }

@juban
Copy link
Contributor

juban commented Nov 3, 2017

OK. Seems a pretty good fix (better than mine anyway ;) )
Tests are OK but I'll have to add a new one to cover your exact use case.
I'll release a 1.3.2 release soon.
Thank you for your contribution.

@sem-soft
Copy link

sem-soft commented Nov 6, 2017

@juban and what about save recursivly childs relations? For example, if main model have child relations and child relations have relations for save too? In my case, i have errors if i try to validate main model, beciuse in beforeValidation you do saving children models. Did you test with case? Thank U for answer!

@mythicallage
Copy link
Author

Hi, @sem-soft.

Could you please clarify the relationship between the main model and the children?

@juban
Copy link
Contributor

juban commented Nov 6, 2017

Hi @sem-soft,

Could you open a new issue and provide your use case scenario along with models definitions please?

Thanks.

@sem-soft
Copy link

sem-soft commented Nov 6, 2017

Hi, @mythicallage. Yes, shure! MainModel has many ChildModel, ChildModel has one SubChildModel. And when I tried to save MainModel AR-instance, i've got an error.

Hi, @juban,
l will do this.

Thx.

@juban
Copy link
Contributor

juban commented Nov 11, 2017

Hi @mythicallage

I've made some improvements to cover more hasOne scenarios.
Could you tell me, based on the latest unstable commit in the master branch that there are no regressions on your side?

Thx.

@juban juban reopened this Nov 11, 2017
@juban juban added this to the 1.4.0 milestone Nov 11, 2017
@juban juban added the bug label Nov 11, 2017
@mythicallage
Copy link
Author

Hi @juban .

It works fine. Thank you.

@juban
Copy link
Contributor

juban commented Nov 16, 2017

Thx.

@juban juban closed this as completed Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants