laravel-auditing
laravel-auditing copied to clipboard
Value Object Casting issue. + Recommendation.
Q | A |
---|---|
Bug? | yes |
New Feature? | yes |
Framework | Laravel |
Framework version | 7.13.0 |
Package version | 10.0.0 |
PHP version | 7.4.5 |
Actual Behaviour
Following your documentation: "The getModified() method honors all attribute mutators and/or casts defined in the Auditable model, transforming the data accordingly." The issue is when casting to value objects. See laravel documentation. When this method is calling "getFormattedValue" laravel will get the value from the classCastCache property. See: https://github.com/laravel/framework/blob/master/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php#L562
$modified['old'] will be equal to $modified['new'] if your attribute is using a custom cast.
@foreach ($audit->getModified() as $attribute => $modified)
<ul>
<li>OLD: {{ $modified['old'] }}</li>
<li>NEW: {{ $modified['new'] }}</li>
</ul>
@endforeach
Expected Behaviour
$modified['old'] should NOT be equal to $modified['new'] if the attribute is using a custom cast.
Steps to Reproduce
1- First create a cast class
<?php
namespace App\Casts;
use App\Address;
use Illuminate\Contracts\Database\Eloquent\CastsAttributes;
class Address implements CastsAttributes
{
/**
* Cast the given value.
*
* @param \Illuminate\Database\Eloquent\Model $model
* @param string $key
* @param mixed $value
* @param array $attributes
* @return \App\Address
*/
public function get($model, $key, $value, $attributes)
{
return new Address(
$attributes['address_line_one'],
$attributes['address_line_two']
);
}
/**
* Prepare the given value for storage.
*
* @param \Illuminate\Database\Eloquent\Model $model
* @param string $key
* @param \App\Address $value
* @param array $attributes
* @return array
*/
public function set($model, $key, $value, $attributes)
{
return [
'address_line_one' => $value->lineOne,
'address_line_two' => $value->lineTwo,
];
}
}
2- Add the cast to the model
protected $casts = [
'address' => \App\Casts\Address::class,
];
3- Update the model
$user = new App\User();
$user->address->lineOne = 'My Address Value';
$user->save();
$user = App\User::find(1);
$user->address->lineOne = 'Updated Address Value';
$user->save();
4- In your view call this.
@foreach ($audit->getModified() as $attribute => $modified)
<?php dd($attribute, $modified['old'], $modified['new'] ); ?>
@endforeach
and you will see: 'My Address Value' instead of 'Updated Address Value'.
Possible Solutions
This package should not call mutators and casters to alter the values. Every Auditable model should be able to transform itself when you create a new instance of the model passing the new attributes.
This is what I'm doing. See gist. Basically creating a class that will create the oldAuditableModel and newAuditableModel. And laravel will take care of mutators and casting. The setRawAttributes function does the trick here.
...
$this->oldModel = new $auditableClassName();
$this->oldModel->setRawAttributes($audit->old_values, true);
$this->newModel = new $auditableClassName();
$this->newModel->setRawAttributes($audit->new_values, true);
...
<?php
namespace App;
use OwenIt\Auditing\Contracts\Audit;
use Illuminate\Database\Eloquent\Model;
class SmartAudit
{
/**
* @var array
*/
public $modifiedKeys;
/**
* @var array
*/
public $modified;
/**
* @var \Illuminate\Database\Eloquent\Model
*/
public $oldModel;
/**
* @var \Illuminate\Database\Eloquent\Model
*/
public $newModel;
/**
* @var \OwenIt\Auditing\Contracts\Audit
*/
public $audit;
/**
* @param \OwenIt\Auditing\Contracts\Audit $audit
*/
public function __construct(Audit $audit)
{
$this->audit = $audit;
$auditableClassName = get_class($audit->auditable);
// Modified Auditable attributes
$this->modifiedKeys = [];
$this->modified = [];
if (count($audit->new_values)) {
$this->newModel = new $auditableClassName();
$this->newModel->setRawAttributes($audit->new_values, true);
foreach ($audit->new_values as $attribute => $value) {
$this->modified[$attribute]['new'] = $value;
}
}
if (count($audit->old_values)) {
$this->oldModel = new $auditableClassName();
$this->newModel->setRawAttributes($audit->old_values, true);
foreach ($audit->old_values as $attribute => $value) {
$this->modified[$attribute]['old'] = $value;
}
}
$this->modifiedKeys = array_keys($this->modified);
}
}
2- Add a method displayAuditField to the Auditable model to return the proper value. This method is way to sanitize the input and avoid errors.
public function displayAuditField($field)
{
$resut = $this->{$field};
switch ($field) {
// boolean
case 'active':
$resut = $this->{$field} ? __('yes') : __('no');
break;
// dates
case 'created_at':
case 'updated_at':
case 'deleted_at':
$resut = optional($this->{$field})->format('F j, Y g:i A');
break;
// decimal
case 'price':
$resut = '$ ' . number_format($this->{$field}, 2);
break;
// objects
case 'created_by':
$resut = $this->creator ? optional($this->creator)->name : $this->{$field};
break;
case 'agency_id':
$resut = $this->agency ? optional($this->agency)->name : $this->{$field};
break;
case 'address':
$resut = optional($this->address)->displayAsAuditField();
break;
}
return $resut;
}
3- Then in your view you can do this
<?php
$smartAudit = new \App\SmartAudit($audit);
?>
<tbody>
@foreach ($smartAudit->modifiedKeys as $attribute)
<tr>
<td>
<strong>@lang('auditable.field.'.$attribute)</strong>
</td>
<td>
<table class="w-100">
<td style="width: 50%; background: #ffe9e9;">
{{ optional($smartAudit->oldModel)->displayAuditField($attribute) }}
</td>
<td style="background: #e9ffe9;">
{{ optional($smartAudit->newModel)->displayAuditField($attribute) }}
</td>
</table>
</td>
</tr>
@endforeach
</tbody>
Image:
Is there a fix for this?
I hit a very similar problem.
I opened a bug report with laravel because IMHO the it's sub-optimal that there method returns different results based on a (IMHO) wrong cache-hit.
https://github.com/laravel/framework/issues/43091
IMO its not ideal to store casted values in audits at all. We are working on a solution, but have some backwards compatibility to consider as well.
Note, related to this: https://github.com/owen-it/laravel-auditing/discussions/799