yii2 icon indicating copy to clipboard operation
yii2 copied to clipboard

Version 2.0.46 brakes saving in DB

Open rsemenyshyn opened this issue 3 years ago • 6 comments

I have updated Yii2 up to 2.0.46

My code was working well, but I started to get the following:

PHP Notice 'yii\base\ErrorException' with message 'Undefined index: Id' 

in .../vendor/yiisoft/yii2/db/BaseActiveRecord.php:1765

Stack trace:
#0 ...vendor/yiisoft/yii2/db/BaseActiveRecord.php(1765): yii\base\ErrorHandler->handleError(8, 'Undefined index...', '...', 1765, Array)
#1 .../vendor/yiisoft/yii2/db/BaseActiveRecord.php(642): yii\db\BaseActiveRecord->isAttributeDirty('Id', 22813)

...

I have analyzed changes done between 2.0.45 and 2.0.46 and found the following: https://github.com/yiisoft/yii2/commit/0f004db99b93e731b0e4cdbd35dcf78d86701f87 Here,

 - if (isset($names[$name]) && (!array_key_exists($name, $this->_oldAttributes) || $value !== $this->_oldAttributes[$name])) {
 + if (isset($names[$name]) && (!array_key_exists($name, $this->_oldAttributes) || $this->isAttributeDirty($name, $value))) {

Where isAttributeDirty is a new function that looks like:

private function isAttributeDirty($attribute, $value)
    {
        $old_attribute = $this->oldAttributes[$attribute];
        if (is_array($value) && is_array($this->oldAttributes[$attribute])) {
            $value = ArrayHelper::recursiveSort($value);
            $old_attribute = ArrayHelper::recursiveSort($old_attribute);
        }

        return $value !== $old_attribute;
    }

For me, interesting is, why we used $this->_oldAttributes in the older versions, but $this->oldAttributes in the newest?

So I changed the code locally to:

private function isAttributeDirty($attribute, $value)
    {
        $old_attribute = $this->_oldAttributes[$attribute];
        if (is_array($value) && is_array($this->_oldAttributes[$attribute])) {
            $value = ArrayHelper::recursiveSort($value);
            $old_attribute = ArrayHelper::recursiveSort($old_attribute);
        }

        return $value !== $old_attribute;
    }

and that works back as it used to be before.

Is this a bug, or feature?

rsemenyshyn avatar Aug 31 '22 20:08 rsemenyshyn

looks like typo for me

KirDE avatar Aug 31 '22 20:08 KirDE

Damn, indeed. I wonder why the tests didn't show anything. Could you prepare PR fixing this? With proper tests if possible?

bizley avatar Sep 01 '22 06:09 bizley

@bizley, This isn`t a typo, see this

lesha724 avatar Sep 01 '22 11:09 lesha724

Hmm, right... Missed that getter. So what is going on here? This should not affect anything. @rsemenyshyn could you provide us with a minimal data to reproduce the problem?

bizley avatar Sep 01 '22 12:09 bizley

For me, interesting is, why we used $this->_oldAttributes in the older versions, but $this->oldAttributes in the newest?

because $this->oldAttributes calling $this->getOldAttributes(), by default, $this->_oldAttributes is null i.e. not array https://github.com/yiisoft/yii2/blob/27344557ea415011afda1d26be0df1496a1cc51a/framework/db/BaseActiveRecord.php#L93

WinterSilence avatar Sep 10 '22 02:09 WinterSilence

private function isAttributeDirty($attribute, $value)
{
    $oldAttributes = $this->getOldAttributes();
    if (!array_key_exists($attribute, $oldAttributes)) {
        return false;
    }
    if ($value === $oldAttributes[$attribute]) {
        return true;
    }
    return \yii\helpers\Comparator::compare($value, $oldAttributes[$attribute]);
}

Comparator

WinterSilence avatar Sep 10 '22 02:09 WinterSilence

Since this change broke app and we have many reports about it I would say that we need to rollback it. @samdark ?

bizley avatar Oct 27 '22 06:10 bizley

Hi @samdark, was there some discussion/progress on @bizley request to revert?

markch123 avatar Nov 02 '22 06:11 markch123

Hi @samdark, was there some discussion/progress on @bizley request to revert?

@markch123 see https://github.com/yiisoft/yii2/issues/19546

schmunk42 avatar Nov 02 '22 09:11 schmunk42

Closing since it was reverted in master.

samdark avatar Nov 03 '22 11:11 samdark