Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Model::hasChanged() and getChangedFields() return false values #14376

Closed
zsilbi opened this issue Sep 10, 2019 · 2 comments
Labels
Projects

Comments

@zsilbi
Copy link
Contributor

@zsilbi zsilbi commented Sep 10, 2019

When snapshot data is saved to a Model, all attributes are being saved as string.
There can be issues regardless orm.cast_on_hydrate is turned on or off.

Here is a quick example:

class Robots extends \Phalcon\Mvc\Model
{
    public $id; // unsigned int(10) in MySQL
    public $year; // unsigned int(10) in MySQL

    public function initialize()
    {
        $this->keepSnapshots(true);
    }
}

/****************************************/
ini_set('phalcon.orm.cast_on_hydrate', 0);

$robot = Robots::findFirst();

var_dump($robot->toArray()); // array(2) { ["id"] => string(1) "1" ["year"] => string(4) "1900" }

var_dump($robot->year); // string(4) "1900"

var_dump($robot->getSnapshotData()['year']); // string(4) "1900"

var_dump($robot->hasChanged('year')); // bool(false)

$robot->year = 1900;
var_dump($robot->hasChanged('year')); // bool(true)

$robot->year = "1900";
var_dump($robot->hasChanged('year')); // bool(false)

/****************************************/
ini_set('phalcon.orm.cast_on_hydrate', 1);

$robot = Robots::findFirst();

var_dump($robot->toArray()); // array(2) { ["id"] => int(1) ["year"] => int(1900) }

var_dump($robot->year); // int(1900)

var_dump($robot->getSnapshotData()['year']); // string(4) "1900"

var_dump($robot->hasChanged('year')); // bool(true)

$robot->year = 1900;
var_dump($robot->hasChanged('year')); // bool(true)

$robot->year = "1900";
var_dump($robot->hasChanged('year')); // bool(false)

When orm.cast_on_hydrate is on, I think Model::cloneResultMap() should save the casted values to the snapshot data.
If it's off, then Model::getChangedFields() should not use === to check if the field has changed.

Any thoughts?

Details

  • Phalcon version: 4.0.x branch
@zsilbi zsilbi referenced this issue Sep 10, 2019
3 of 5 tasks complete
@ruudboon

This comment has been minimized.

Copy link
Member

@ruudboon ruudboon commented Sep 12, 2019

cloneResultMap should save casted values when cast_on_hydrate is on.

I'm not sure if I agree not to use === when cast_on_hydrate is off.
When cast_on_hydrate is off you should accept that all your model properties are strings and should set them as string. If you set it as int I'm expecting that hasChanged will detect a change.

@ruudboon ruudboon added this to To do in 4.0 Release via automation Sep 13, 2019
@ruudboon ruudboon moved this from To do to In progress in 4.0 Release Sep 16, 2019
@zsilbi

This comment has been minimized.

Copy link
Contributor Author

@zsilbi zsilbi commented Sep 18, 2019

Resolved in #14377

@zsilbi zsilbi closed this Sep 18, 2019
4.0 Release automation moved this from In progress to Done Sep 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4.0 Release
  
Done
2 participants
You can’t perform that action at this time.