laravel-auditing icon indicating copy to clipboard operation
laravel-auditing copied to clipboard

Omit casting on a per attribute basis - open to a PR?

Open ultrono opened this issue 3 years ago • 6 comments

Hi,

Would the package maintainer be open to a PR for the following?

Allow an array of attributes that shouldn't be cast, to be specified on a per model basis?

From http://www.laravel-auditing.com/docs/9.0/audit-transformation I can see here a new key role_name has been specified, but it seems odd to introduce keys outside the scope of your model.

Example, assume I have a task model with a status_id column. status_id is cast to an integer.

On the task model the transformAudit() method has been specified as follows:

public function transformAudit(array $data): array
{
    if (Arr::has($data, 'new_values.status_id')) {
        $data['old_values']['status_id'] = optional(Status::query()->find($this->getOriginal('status_id')))->title;
        $data['new_values']['status_id'] = Status::query()->find($this->getAttribute('status_id'))->title;
     }
     
     return $data;
}

Note here, as the documentation suggests I could simply specify a different key, other than status_id. However, doing so causes both keys to be present in the database old and new columns. To only store the new key I have to omit the old key i.e.

public function transformAudit(array $data): array
{
    if (Arr::has($data, 'new_values.status_id')) {
        $data['old_values']['status'] = optional(Status::query()->find($this->getOriginal('status_id')))->title;
        $data['new_values']['status'] = Status::query()->find($this->getAttribute('status_id'))->title;
        unset($data['old_values']['status_id'], $data['new_values']['status_id']);
     }
     
     return $data;
}

Instead of storing the old an new status_id integers, the status title is being stored as a string.

Within the audits table:

// old values
{"status_id":"Awaiting Feedback"}

// new values
{"status_id":"Current Jobs"}

As this package now honours Eloquent casts, within my auditable view, the old an new status_id values are always zero - as expected due to the model casts.

I could remove the casts within the tasks model. However, this is an existing project that makes use of strict comparisons.

I'd propose a very simple PR to accommodate this.

Within the package Audit model, adjust the getDataValue method to account for an excluded attribute. If excluded, simply the return the raw value:

/**
   * {@inheritdoc}
   */
public function getDataValue(string $key)
{
    if (!array_key_exists($key, $this->data)) {
       return;
     }

     $value = $this->data[$key];

     // if the attribute should not be casts, simple return the raw value
     if (in_array($key, $this->excludedAuditableAttributeCats, true) {
       return $value;
     }

     ...

Would this be something the package owner would consider?

Thanks

ultrono avatar Mar 01 '21 10:03 ultrono

This is over a year old but no comments, I definitely think this would benefit the package.

ale1981 avatar May 25 '22 15:05 ale1981

Not only do I have the same requirements, but also the current version has a bug on this. After transforming the data, it doest get stored correctly. However, when retrieving getModified(), it doesn't return the data from the audit table, it fetch the current information from the database. Meaning the status we transformed before won't be even showing when you use getModified to show the audit data.

jasonfish568 avatar Mar 12 '23 10:03 jasonfish568

Had completely forgotten about this. Since I raised this I've unfortunately been using the unset approach above. Not end of the world 😮‍💨

ultrono avatar Mar 13 '23 08:03 ultrono

We are working on a solution, but have to be careful with regards to backwards compatibility.

Related to this: https://github.com/owen-it/laravel-auditing/discussions/799

MortenDHansen avatar Mar 16 '23 08:03 MortenDHansen

Right. To get back to this, i am sorry to say i don't understand what the desired outcome is.

If possible, please outline what the desired behaviour is, or describe how to reproduce the problem.

MortenDHansen avatar Mar 30 '23 10:03 MortenDHansen