Merge pull request #6285 from kenjis/fix-Entity-hasChanged-mapped-property

fix: Entity::hasChanged() returns wrong result to mapped property

Author: kenjis

system/Entity/Cast/BaseCast.php

@@ -19,10 +19,10 @@ abstract class BaseCast implements CastInterface
     /**
      * Get
      *
-     * @param mixed $value  Data
-     * @param array $params Additional param
+     * @param array|bool|float|int|object|string|null $value  Data
+     * @param array                                   $params Additional param
      *
-     * @return mixed
+     * @return array|bool|float|int|object|string|null
      */
     public static function get($value, array $params = [])
     {
@@ -32,10 +32,10 @@ public static function get($value, array $params = [])
     /**
      * Set
      *
-     * @param mixed $value  Data
-     * @param array $params Additional param
+     * @param array|bool|float|int|object|string|null $value  Data
+     * @param array                                   $params Additional param
      *
-     * @return mixed
+     * @return array|bool|float|int|object|string|null
      */
     public static function set($value, array $params = [])
     {

system/Entity/Cast/CastInterface.php

@@ -19,20 +19,20 @@ interface CastInterface
     /**
      * Get
      *
-     * @param mixed $value  Data
-     * @param array $params Additional param
+     * @param array|bool|float|int|object|string|null $value  Data
+     * @param array                                   $params Additional param
      *
-     * @return mixed
+     * @return array|bool|float|int|object|string|null
      */
     public static function get($value, array $params = []);
 
     /**
      * Set
      *
-     * @param mixed $value  Data
-     * @param array $params Additional param
+     * @param array|bool|float|int|object|string|null $value  Data
+     * @param array                                   $params Additional param
      *
-     * @return mixed
+     * @return array|bool|float|int|object|string|null
      */
     public static function set($value, array $params = []);
 }

system/Entity/Cast/DatetimeCast.php

@@ -24,6 +24,8 @@ class DatetimeCast extends BaseCast
      * {@inheritDoc}
      *
      * @throws Exception
+     *
+     * @return Time
      */
     public static function get($value, array $params = [])
     {

system/Entity/Entity.php

@@ -146,7 +146,7 @@ public function fill(?array $data = null)
      * __get() magic method so will have any casts, etc applied to them.
      *
      * @param bool $onlyChanged If true, only return values that have changed since object creation
-     * @param bool $cast        If true, properties will be casted.
+     * @param bool $cast        If true, properties will be cast.
      * @param bool $recursive   If true, inner entities will be casted as array as well.
      */
     public function toArray(bool $onlyChanged = false, bool $cast = true, bool $recursive = false): array
@@ -256,6 +256,8 @@ public function hasChanged(?string $key = null): bool
             return $this->original !== $this->attributes;
         }
 
+        $key = $this->mapProperty($key);
+
         // Key doesn't exist in either
         if (! array_key_exists($key, $this->original) && ! array_key_exists($key, $this->attributes)) {
             return false;
@@ -287,7 +289,7 @@ public function setAttributes(array $data)
      * Checks the datamap to see if this property name is being mapped,
      * and returns the db column name, if any, or the original property name.
      *
-     * @return string
+     * @return string db column name
      */
     protected function mapProperty(string $key)
     {
@@ -480,7 +482,7 @@ public function __get(string $key)
         // Convert to CamelCase for the method
         $method = 'get' . str_replace(' ', '', ucwords(str_replace(['-', '_'], ' ', $key)));
 
-        // if a set* method exists for this key,
+        // if a get* method exists for this key,
         // use that method to insert this value.
         if (method_exists($this, $method)) {
             $result = $this->{$method}();

tests/system/Entity/EntityTest.php

@@ -929,6 +929,24 @@ public function testHasChangedNoChange()
         $this->assertFalse($entity->hasChanged('default'));
     }
 
+    public function testHasChangedMappedNoChange()
+    {
+        $entity = $this->getEntity();
+
+        $entity->createdAt = null;
+
+        $this->assertFalse($entity->hasChanged('createdAt'));
+    }
+
+    public function testHasChangedMappedChanged()
+    {
+        $entity = $this->getEntity();
+
+        $entity->createdAt = '2022-11-11 11:11:11';
+
+        $this->assertTrue($entity->hasChanged('createdAt'));
+    }
+
     public function testHasChangedWholeEntity()
     {
         $entity = $this->getEntity();

user_guide_src/source/models/entities.rst

@@ -144,9 +144,9 @@ As an example, imagine you have the simplified User Entity that is used througho
 .. literalinclude:: entities/008.php
 
 Your boss comes to you and says that no one uses usernames anymore, so you're switching to just use emails for login.
-But they do want to personalize the application a bit, so they want you to change the name field to represent a user's
+But they do want to personalize the application a bit, so they want you to change the ``name`` field to represent a user's
 full name now, not their username like it does currently. To keep things tidy and ensure things continue making sense
-in the database you whip up a migration to rename the `name` field to `full_name` for clarity.
+in the database you whip up a migration to rename the ``name`` field to ``full_name`` for clarity.
 
 Ignoring how contrived this example is, we now have two choices on how to fix the User class. We could modify the class
 property from ``$name`` to ``$full_name``, but that would require changes throughout the application. Instead, we can
@@ -164,6 +164,10 @@ through the original ``$user->full_name``, also, as this is needed for the model
 to the database. However, ``unset()`` and ``isset()`` only work on the mapped property, ``$user->name``, not on the database column name,
 ``$user->full_name``.
 
+.. note:: When you use Data Mapping, you must define ``set*()`` and ``get*()`` method
+    for the database column name. In this example, you must define ``setFullName()`` and
+    ``getFullName()``.
+
 Mutators
 ========
 

user_guide_src/source/models/entities/009.php

@@ -8,14 +8,15 @@ class User extends Entity
 {
     protected $attributes = [
         'id'         => null,
-        'full_name'  => null, // In the $attributes, the key is the column name
+        'full_name'  => null, // In the $attributes, the key is the db column name
         'email'      => null,
         'password'   => null,
         'created_at' => null,
         'updated_at' => null,
     ];
 
     protected $datamap = [
+        // property_name => db_column_name
         'name' => 'full_name',
     ];
 }