la-haute-societe/yii2-save-relations-behavior

Some issue with one-to-one relationship

Closed this issue · 12 comments

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 commented

Hi,

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

Thanks.

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 commented

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.

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 commented

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.

@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!

Hi, @sem-soft.

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

juban commented

Hi @sem-soft,

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

Thanks.

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 commented

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.

Hi @juban .

It works fine. Thank you.

juban commented

Thx.