codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.3k stars 1.89k forks source link

Bug: hasChanged() always returns true even if a field of boolean type is reassigned with the same value without change. #5678

Closed xonipatino closed 2 years ago

xonipatino commented 2 years ago

PHP Version

7.4

CodeIgniter4 Version

4.1.8

CodeIgniter4 Installation Method

Manual (zip or tar.gz)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

MySQL 8.0.27

What happened?

I am currently working on a project where there is a boolean type column (state) in a table, I am working on this with an Entity, when I make a post to update a record in that table, I make the query with ->find() and I reassign the information of the inputs through the function ->fill(), example:

-- In Agent_Entity.php :

namespace App\Entities;
use CodeIgniter\Entity;
class Agent_Entity extends Entity
{
    protected $attributes = [
      ..
      ..
      "status" => true, //default in the DB table
      "name" => null
    ];

    protected $casts = [
      ..
      ..
      "status" => "boolean",
      "name" => "?string"
    ];
}

-- In Agent_Model.php :

namespace App\Models;
use CodeIgniter\Model;
class Agent_Model extends Model
{
    protected $table = "agent";
    protected $primaryKey = "id_agent";
    ..
    ..
    protected $returnType = \App\Entities\Agent_Entity::class
}

-- In Agent_Controller.php :

namespace App\Controllers;
class Agent_Controller extends Base_Controller
{
    function update($_id) {
        $name = trim($this->request->getPost("name", FILTER_SANITIZE_STRING));
        $status = (bool) $this->request->getPost("status", FILTER_UNSAFE_RAW);
        ..
        ..
        $agent = model("agent_model")->find($_id); //this returns an Agent_Entity()
        ..
        ..
        $agent->fill([
            ..
            ..
            "name" => $name,
            "status" => $status
        ]);

        //here is the bug
        //here it always returns true, even if the same information that is registered in the database is assigned
        if ( $agent->hasChanged() ) {
             //send to save to database
        }
        else {
            echo "without changes"; exit;
        }
   } 
}

When I call the function ->hasChange() making it ->fill() with the same data that is registered (without changes), ->hasChanged() always returns true, but when I comment it, it means:

//in this case, ->hasChanged() it returns false (what is expected)
$agent->fill([
    ..
    ..
    "name" => $name,
    //"status" => $status
]);

sending the same registered information (without making changes) returns false. The error is in this latest version because this project was working very well in version 4.1.5 or 4.1.6 I don't remember which one it had

Steps to Reproduce

->hasChange() It must return false since sometimes changes are not made in the name or in the state and neither in the other fields.

Expected Output

false when there is no change even when a boolean field is included

Anything else?

No response

iRedds commented 2 years ago

Call dd($agent); after filling. The attributes and original properties must be the same.

kenjis commented 2 years ago

I'm not sure this is a bug.

In the current implementation, it seems $attributes has raw values from the database when fetched from the database. https://github.com/codeigniter4/CodeIgniter4/blob/90eef161c1e7cb4af6165dfa87e8d4be074092fa/system/Database/MySQLi/Result.php#L144-L146

xonipatino commented 2 years ago

good morning friends, the validation problem still persists, I don't know if I managed to make myself understood with the error and if you can help me to give a solution.

kenjis commented 2 years ago

What's the validation problem?

xonipatino commented 2 years ago

Hello everyone, look at the problem is the following:

When I send a POST form with the same information registered in the database, that is, without changes, it is supposed that when doing the validation with hasChanged() it should throw me False, because no change was made, this is not happening since the hasChanged() function takes the Status field (boolean) as if it always changed, but when I comment the Status field, that is, I don't add it in the Fill() function works correctly.

iRedds commented 2 years ago

MySQL does not have a boolean data type. Any boolean value is converted to 1 or 0. MySQL php driver returns all data as a string. This means that the "status" field received from the database will have the value "1" or "0".

Here you convert the value to boolean

 $status = (bool) $this->request->getPost("status", FILTER_UNSAFE_RAW);
        $agent->fill([
            //...
            "status" => $status
        ]);

Can you guess what the result of the expression bool === string will be?

xonipatino commented 2 years ago

@iRedds Thanks for your clarification, but in the model it returns an Entity (Agent_Entity), which converts this field (Status) into boolean, which I suppose hasChanged() will find of the same data type.

namespace App\Entities;
use CodeIgniter\Entity;
class Agent_Entity extends Entity
{
    protected $attributes = [
      ..
      ..
      "status" => true, //default in the DB table
      "name" => null
    ];

    protected $casts = [
      ..
      ..
      "status" => "boolean",
      "name" => "?string"
    ];
}

protected $returnType = \App\Entities\Agent_Entity::class

xonipatino commented 2 years ago

I performed the following validation before doing the fill() on the Controller and the result was True, this confirms that if there is any problem in the hasChanged()

$agent = model("agent_model")->find($_id); //this returns an Agent_Entity() echo gettype($agent->status); // this returned Boolean echo json_encode($agent->status === (bool) $status); // this returned True, This means that there are no changes, which is correct exit;

iRedds commented 2 years ago

The "boolean" type casting only works for getting, not for setting. That is, when you call $agent->status That is, the original value does not change.

As I said above

Call dd($agent); after filling. The attributes and original properties must be the same.

xonipatino commented 2 years ago

hi @iRedds, You are right, I just assigned in the fill() the state with type string ("1" or "0") and it works correctly.

The code is as follows in the Controller:

$status= (bool) $this->request->getPost("status", FILTER_UNSAFE_RAW); //checkbox value, if checked is true, otherwise (null) is false
..
..
$agent = model("agent_model")->find($_id); //this returns an Agent_Entity()
..
..
$agent->fill([
    ..
    ..
    "name" => $name,
    "status" => ($status) ? "1" : "0"
]);
..
..
if ( $agent->hasChanged() ) { // this returned False, is correct!!
     //send to save to database
}
...

Thank @iRedds, and thanks to all, I close the topic.

iRedds commented 2 years ago

You can extend the CodeIgniter\Entity\Cast\BooleanCast class by adding your own setter. https://codeigniter.com/user_guide/models/entities.html#custom-casting

kenjis commented 2 years ago

But the current behavior is difficult to understand, and inconsistent: