Getting properties from deleted records causes an error
Thank you for this package, I installed it today and it seems very powerful!
Bug Report
Q | A |
---|---|
PHP version | 7.2.* |
Package version | 1.1.1 |
Framework | Laravel |
Framework version | 5.7.20 |
Actual Behaviour
When a model is deleted (which does not use SoftDeletes
), the deleted
event is recorded, and a ledger is created, which contains the properties of the said model.
I have a controller which fetches all the Ledger
models, and a view where I populate them along with the properties of the model using $ledger->getData(true)
.
This call causes a fatal error: Call to a member function getCiphers() on null
.
This happens because it's calling $this->getDecipheredProperties(false)
. This methods calls foreach ($this->recordable->getCiphers()
but $this->recordable
is obviously null
.
Expected Behaviour
$ledger->getData()
true should return all the properties of the deleted model, which are present on the properties
column in the database.
Steps to Reproduce
- Setup a model
namespace App\Models;
use Altek\Accountant\Contracts\Recordable;
use Illuminate\Database\Eloquent\Model;
class Article extends Model implements Recordable
{
use \Altek\Accountant\Recordable;
}
- Create and delete a model
$article = Article::create(['title' => 'foo');
$article->delete();
- Get all the ledgers and attempt to get the recordable data
$ledgers = \Altek\Accountant\Models\Ledger::get();
foreach ($ledgers as $ledger) {
$ledger->getData(true);
}
Possible Solutions
In the Altek\Accountant\Ledger
, maybe check if $this->recordable
is set before iterating the ciphers in the getDecipheredProperties
method. Otherwise, skip the foreach
iteration at all.
It also seems that $this->recordable
is called in some other methods in the class without any check, e.g. in the extract()
method, so that would fail as well.
So maybe we should instantiate a new empty instance of the recordable model if $this->recordable
is not set, to allow us to work with its methods.
If you want, I can submit a merge request with a fix, but I'd like to have your input first.
Thanks!