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

[BUG]: Fatal error on call $model->getChangedFields() #14466

Closed
ekmst opened this issue Oct 13, 2019 · 5 comments
Closed

[BUG]: Fatal error on call $model->getChangedFields() #14466

ekmst opened this issue Oct 13, 2019 · 5 comments
Labels
enhancement Enhancement to the framework

Comments

@ekmst
Copy link
Contributor

ekmst commented Oct 13, 2019

Describe the bug
Fatal error on call $model->getChangedFields()

To Reproduce

    $user = Users::findFirst();
    
    var_dump($user->getSnapshotData());  // output: null
    
    print_r($user->getChangedFields()); // fatal error

Steps to reproduce the behavior:

Fatal error: Uncaught Phalcon\Mvc\Model\Exception: The record doesn't have a valid data snapshot in phalcon/Mvc/Model.zep on line 1520

Expected behavior

empty array output

Details

  • Phalcon version: 4.0.0-rc.1
  • PHP Version: 7.3.9
  • Operating System: linux
  • Installation type: installing via package manager
  • Other related info (Database, table schema): mysql
@niden
Copy link
Member

niden commented Oct 13, 2019

This is the intended behavior. If there is no snapshot, an exception is thrown.

We can change this to return an empty array. This will break backwards compatibility.

@phalcon/core-team Thoughts?

@ekmst
Copy link
Contributor Author

ekmst commented Oct 13, 2019

Thanks @niden!

In my opinion this is not the case for throwing an exception.
It would be logical to return an empty array

@niden niden added 4.0 enhancement Enhancement to the framework discussion Request for comments and discussion and removed Bug - Unverified labels Oct 13, 2019
@ruudboon
Copy link
Member

For getChangedFields to work snapshot is required. If we sent back an empty array the user doesn't know the method isn't working.
If keepSnapshots is true we can return the correct value.

<?php

use Phalcon\Mvc\Model;

class Robots extends Model
{
    public function initialize()
    {
        $this->keepSnapshots(true);
    }
}

Maybe we should update the error. "keepSnapshots must be enabled to track changes"

@sergeyklay
Copy link
Contributor

I agree with @ruudboon

@niden niden removed the discussion Request for comments and discussion label Oct 14, 2019
@niden niden mentioned this issue Oct 14, 2019
5 tasks
niden added a commit that referenced this issue Oct 14, 2019
niden added a commit that referenced this issue Oct 14, 2019
@niden
Copy link
Member

niden commented Oct 14, 2019

Implemented

@niden niden closed this as completed Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to the framework
Projects
None yet
Development

No branches or pull requests

4 participants