[Bug] Cannot show entries with both SoftDeletes and regular Deletes in the same List operation

[tabacitu]

Bug report

What I did

I've used ActivityLog on backpackforlaravel.com - over there some models have SoftDeletes, some do not. I changed the spatie config subject_returns_soft_deleted_models to true, to show the softdeleted activities too. All good - when the table only contained activities pointing to soft-deletin models, everything worked ok.

But then... I used one model that did NOT have softdeletes.

What I expected to happen

The entry to show up in the list operation.

What happened

Big fat error

What I've already tried to fix it

I've tried to see what is going wrong. And it looks to me like the problem is in the subject column. If I remove that, everything's working. That column is doing $entry->subject which is defined by spatie on Activity.php:62 as

    public function subject(): MorphTo
    {
        if (config('activitylog.subject_returns_soft_deleted_models')) {
            return $this->morphTo()->withTrashed();
        }

        return $this->morphTo();
    }

So from what I can tell... it's a spatie bug, basically. If the config is set to use soft deletes, it will try to use withTrashed() on EVERYTHING. It should be something like:

    public function subject(): MorphTo
    {
        if (config('activitylog.subject_returns_soft_deleted_models') && method_exists($this->morphTo(), 'withTrashed')) {
            return $this->morphTo()->withTrashed();
        }

        return $this->morphTo();
    }

[tabacitu]

Ok so apparently Spatie is aware of the issue - spatie/laravel-activitylog#456 - and the fix isn't as easy as I thought it would be.

The last comment says this is resolved in Laravel 10.20.0. But we should test it before we close this.

[promatik]

Wow! Just in time of our release 😅
Should we make that version a minimum requirement?

[tabacitu]

We could... in fact we should. But let's please also make a branch with everything BUT the L10 requirement - so we can have it working on backpackforlaravel.com too 😅 Haven't upgraded to L10 yet... 👀😔

[promatik]

Hey @tabacitu, getting back to this, I think we don't need to enforce users to use Laravel v10.20.0...
That version fixes a bug in Laravel, users using Laravel v10.19.0 will have the issue either in activity log, either in their apps, that's why one should keep dependencies up to date 🤷‍♂️

Also Laravel v11 has been released...
I'll close for now, but almost sure we shouldn't don't need do it...