Version 8.0.3 breaks support for non-integer keys

[specialtactics]

Q	A
Bug?	yes
New Feature?	no
Framework	Laravel
Framework version	5.7.25
Package version	>=8.0.3
PHP version	7.2.x

Actual Behaviour

This PR here
https://github.com/owen-it/laravel-auditing/pull/448/files

For this issue (from what I understand):
#432

Will actually break any project using non-numeric (or non-integer) primary key IDs. In our case, we are using UUIDs (which I would think is quite common these days).

The end result is that any update to the underlying model will wipe all audits and it will no longer have any related audits.

Expected Behaviour

As laravel supports non-integer keys, so should this package. Update shouldn't break existing functionality.

Steps to Reproduce

Create a new model in Laravel with the following properties and as a string in the database (for field type)

    /**
     * @var bool Set to false for UUID keys
     */
    public $incrementing = false;

    /**
     * @var string Set to string for UUID keys
     */
    protected $keyType = 'string';

Deploy the auditable trait on it, and see what happens on model update.

Possible Solutions

The auditable_id cast to integer needs to be removed

[jonagoldman]

You can also extend the Audit model and add uuid behaviour:

<?php

namespace App\Models;

use Illuminate\Support\Str;
use Illuminate\Database\Eloquent\Model;
use OwenIt\Auditing\Contracts\Audit as AuditContract;
use OwenIt\Auditing\Audit as BaseAudit;

class Audit extends Model implements AuditContract
{
    use BaseAudit;

    /**
     * The "booting" method of the model.
     *
     * @return void
     */
    protected static function boot()
    {
        parent::boot();

        static::creating(function($model) {

            if (is_null($model->getAttribute('id')))
            {
                $model->id = Str::orderedUuid();
            }
            
        });

    }

    /**
     * The "type" of the auto-incrementing ID.
     *
     * @var string
     */
    protected $keyType = 'string';

    /**
     * Indicates if the IDs are auto-incrementing.
     *
     * @var bool
     */
    public $incrementing = false;

    /**
     * {@inheritdoc}
     */
    protected $guarded = [];

    /**
     * {@inheritdoc}
     */
    protected $casts = [
        'id' => 'string',
        /// ... others
    ];

}

[specialtactics]

I've done exactly that, but I am not sure why it should be necessary to do so - the package should work out of the box to support the functionary laravel provides.

[jonagoldman]

Because regular auto increment ids are automatically generated by the database when a new record is created, so you don't need to do anything, but with uuids you must write that functionality yourself, same goes for all packages or even regular Laravel models. You want something very specific so you must write it yourself. In any case is a good idea to extend the default model anyways so no harm done.

[specialtactics]

@jonagoldman I think you must be misunderstanding what the issue is here, because what you said above does not relate to it in any way. This issue has nothing to do with the generation of UUIDs, please read again.

[jonagoldman]

Ohhh I see now! 😓 You are right, sorry about that. I had a similar problem generating uuids and tought the issue was about generating uuids! Thanks!

[Maixck]

This breaks #448 for people that do use numeric id, can loose comparison be added to fix that error?

[specialtactics]

@Maixck That was a specific issue with sqlite driver, and it's fixable by overriding the model.

What I am describing on the other hand is across many RDBMs, and it is vanilla laravel functionality.

[Maixck]

@specialtactics It is also occurring with pgsql driver. It can be fixed by overriding the model but it also is broken functionality out of the box.

[specialtactics]

I use postgres, and I've never had this issue, how old is your postgres driver?

[parallels999]

closed due to inactivity, #486 (comment) looks the correct answer