Value Object Casting issue. + Recommendation.

rrpadilla commented 4 years ago

Q	A
Bug?	yes
New Feature?	yes
Framework	Laravel	Lumen
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: