Ensure consistent behavior in ActiveRecord::afterSave()

extensions/elasticsearch/ActiveRecord.php

@@ -395,7 +395,9 @@ class ActiveRecord extends BaseActiveRecord
         if ($runValidation && !$this->validate($attributes)) {
             return false;
         }
-        if ($this->beforeSave(true)) {
+        if (!$this->beforeSave(true)) {
+            return false;
+        }
         $values = $this->getDirtyAttributes($attributes);
 
         $response = static::getDb()->createCommand()->insert(
@@ -406,9 +408,6 @@ class ActiveRecord extends BaseActiveRecord
             $options
         );
 
-//            if (!isset($response['ok'])) {
-//                return false;
-//            }
         $pk = static::primaryKey()[0];
         $this->$pk = $response['_id'];
         if ($pk != '_id') {
@@ -417,15 +416,12 @@ class ActiveRecord extends BaseActiveRecord
         $this->_version = $response['_version'];
         $this->_score = null;
 
-            $this->afterSave(true);
         $this->setOldAttributes($values);
+        $this->afterSave(true, $values);
 
         return true;
     }
 
-        return false;
-    }
-
     /**
      * Updates all records whos primary keys are given.
      * For example, to change the status to be 1 for all customers whose status is 2:

extensions/mongodb/ActiveRecord.php

@@ -224,8 +224,8 @@ abstract class ActiveRecord extends BaseActiveRecord
         $this->setAttribute('_id', $newId);
         $values['_id'] = $newId;
 
-        $this->afterSave(true);
         $this->setOldAttributes($values);
+        $this->afterSave(true, $values);
 
         return true;
     }
@@ -241,7 +241,7 @@ abstract class ActiveRecord extends BaseActiveRecord
         }
         $values = $this->getDirtyAttributes($attributes);
         if (empty($values)) {
-            $this->afterSave(false);
+            $this->afterSave(false, $values);
             return 0;
         }
         $condition = $this->getOldPrimaryKey(true);
@@ -260,10 +260,10 @@ abstract class ActiveRecord extends BaseActiveRecord
             throw new StaleObjectException('The object being updated is outdated.');
         }
 
-        $this->afterSave(false);
         foreach ($values as $name => $value) {
             $this->setOldAttribute($name, $this->getAttribute($name));
         }
+        $this->afterSave(false, $values);
 
         return $rows;
     }

extensions/mongodb/file/ActiveRecord.php

@@ -127,8 +127,8 @@ abstract class ActiveRecord extends \yii\mongodb\ActiveRecord
         $this->setAttribute('_id', $newId);
         $values['_id'] = $newId;
 
-        $this->afterSave(true);
         $this->setOldAttributes($values);
+        $this->afterSave(true, $values);
 
         return true;
     }
@@ -144,7 +144,7 @@ abstract class ActiveRecord extends \yii\mongodb\ActiveRecord
         }
         $values = $this->getDirtyAttributes($attributes);
         if (empty($values)) {
-            $this->afterSave(false);
+            $this->afterSave(false, $values);
             return 0;
         }
 
@@ -196,10 +196,10 @@ abstract class ActiveRecord extends \yii\mongodb\ActiveRecord
             }
         }
 
-        $this->afterSave(false);
         foreach ($values as $name => $value) {
             $this->setOldAttribute($name, $this->getAttribute($name));
         }
+        $this->afterSave(false, $values);
 
         return $rows;
     }

extensions/redis/ActiveRecord.php

@@ -99,11 +99,12 @@ class ActiveRecord extends BaseActiveRecord
         if ($runValidation && !$this->validate($attributes)) {
             return false;
         }
-        if ($this->beforeSave(true)) {
+        if (!$this->beforeSave(true)) {
+            return false;
+        }
         $db = static::getDb();
         $values = $this->getDirtyAttributes($attributes);
         $pk = [];
-            //			if ($values === []) {
         foreach ($this->primaryKey() as $key) {
             $pk[$key] = $values[$key] = $this->getAttribute($key);
             if ($pk[$key] === null) {
@@ -111,7 +112,6 @@ class ActiveRecord extends BaseActiveRecord
                 $this->setAttribute($key, $values[$key]);
             }
         }
-            //			}
         // save pk in a findall pool
         $db->executeCommand('RPUSH', [static::keyPrefix(), static::buildKey($pk)]);
 
@@ -124,15 +124,12 @@ class ActiveRecord extends BaseActiveRecord
         }
         $db->executeCommand('HMSET', $args);
 
-            $this->afterSave(true);
         $this->setOldAttributes($values);
+        $this->afterSave(true, $values);
 
         return true;
     }
 
-        return false;
-    }
-
     /**
      * Updates the whole table using the provided attribute values and conditions.
      * For example, to change the status to be 1 for all customers whose status is 2:

extensions/sphinx/ActiveRecord.php

@@ -395,8 +395,8 @@ abstract class ActiveRecord extends BaseActiveRecord
             return false;
         }
 
-        $this->afterSave(true);
         $this->setOldAttributes($values);
+        $this->afterSave(true, $values);
 
         return true;
     }
@@ -488,7 +488,7 @@ abstract class ActiveRecord extends BaseActiveRecord
         }
         $values = $this->getDirtyAttributes($attributes);
         if (empty($values)) {
-            $this->afterSave(false);
+            $this->afterSave(false, $values);
             return 0;
         }
 
@@ -530,10 +530,10 @@ abstract class ActiveRecord extends BaseActiveRecord
             }
         }
 
-        $this->afterSave(false);
         foreach ($values as $name => $value) {
             $this->setOldAttribute($name, $this->getAttribute($name));
         }
+        $this->afterSave(false, $values);
 
         return $rows;
     }

framework/CHANGELOG.md

@@ -21,6 +21,7 @@ Yii Framework 2 Change Log
 - Bug #3194: Date formatter works only for timestamps in the year range 1970 to 2038 (kartik-v)
 - Bug #3204: `yii\di\Container` did not handle the `$config` parameter well in case when it does not have a default value (qiangxue)
 - Bug #3216: Fixed the bug that `yii.activeForm.destroy()` did not remove `submit` event handlers (qiangxue)
+- Bug #3233: Ensure consistent behavior in ActiveRecord::afterSave() (cebe, qiangxue)
 - Bug #3236: Return value for DateTime->format('U') casted to double to allow correct date formatting (pgaultier)
 - Bug #3268: Fixed the bug that the schema name in a table name was not respected by `yii\db\mysql\Schema` (terazoid, qiangxue)
 - Bug #3311: Fixed the bug that `yii\di\Container::has()` did not return correct value (mgrechanik, qiangxue)

framework/UPGRADE.md

@@ -55,3 +55,7 @@ Upgrade from Yii 2.0 Beta
   This change is needed because `yii\web\View` no longer automatically generates CSRF meta tags due to issue #3358.
 
 * If your model code is using the `file` validation rule, you should rename its `types` option to `extensions`.
+
+* The behavior and signature of `ActiveRecord::afterSave()` has changed. `ActiveRecord::$isNewRecord` will now always be
+  false in afterSave and also dirty attributes are not available. This change has been made to have a more consistent and
+  expected behavior. The changed attributes are now available in the new parameter of afterSave() `$changedAttributes`.
\ No newline at end of file

framework/db/ActiveRecord.php

@@ -431,8 +431,8 @@ class ActiveRecord extends BaseActiveRecord
             }
         }
 
-        $this->afterSave(true);
         $this->setOldAttributes($values);
+        $this->afterSave(true, $values);
 
         return true;
     }

framework/db/BaseActiveRecord.php

@@ -680,7 +680,7 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
         }
         $values = $this->getDirtyAttributes($attributes);
         if (empty($values)) {
-            $this->afterSave(false);
+            $this->afterSave(false, $values);
             return 0;
         }
         $condition = $this->getOldPrimaryKey(true);
@@ -699,10 +699,10 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
             throw new StaleObjectException('The object being updated is outdated.');
         }
 
-        $this->afterSave(false);
         foreach ($values as $name => $value) {
             $this->_oldAttributes[$name] = $this->_attributes[$name];
         }
+        $this->afterSave(false, $values);
 
         return $rows;
     }
@@ -859,11 +859,14 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
      * When overriding this method, make sure you call the parent implementation so that
      * the event is triggered.
      * @param boolean $insert whether this method called while inserting a record.
+     * @param array $changedAttributes the attribute values that have changed during save.
      * If false, it means the method is called while updating a record.
      */
-    public function afterSave($insert)
+    public function afterSave($insert, $changedAttributes)
     {
-        $this->trigger($insert ? self::EVENT_AFTER_INSERT : self::EVENT_AFTER_UPDATE);
+        $this->trigger($insert ? self::EVENT_AFTER_INSERT : self::EVENT_AFTER_UPDATE, new ModelEvent([
+            'data' => ['changedAttributes' => $changedAttributes]
+        ]));
     }
 
     /**