[9.x] Optimize destroy method 🏃🏻‍♂️

[imanghafoori1]

Since the whereKey is optimized to use the whereIntegerRaw whenever is possible, we can use it in the destroy method as well.

The only little change in the underlying select query is that now it uses the fully qualified key.

• (where "table"."key" in ... ) instead of key alone (where "key" in ... )

• It also adds a few missing tests for edge cases.

[leonardocustodio]

I don't know if this is intended or not, but now I can't destroy an array of models created with factory, here is an example:

$collection = Collection::factory([
            'owner_wallet_id' => $this->wallet,
            'token_count' => 1,
            'attribute_count' => 1,
])->create();

Collection::destroy([$collection]);

Now we get the error: ErrorException : Object of class Platform\Core\Models\Collection could not be converted to int. This did not use to happen before this update.

Idk if this is related to our environment or if it happens with everybody. But we only started to get this issue after the laravel update.

[imanghafoori1]

@leonardocustodio
Thanks for your report. I will look into it.
It seems that you are passing an array of model instances into the destroy method.

[taylorotwell]

Reverting.

[imanghafoori1]

@leonardocustodio
It seems that you are passing an array of models into the destroy method.
Are you sure that it really deletes the model from the database?
And please also mention your database driver.

Thanks

[leonardocustodio]

Yep, you are right. It accepts and throws no error, but it doesn't delete them from the database. It deletes if we pass an array of IDs or a Collection of models. But it doesn't delete if it is an array of models.

This is probably because of these lines in the code of the destroy function prior to this PR:

        if ($ids instanceof EloquentCollection) {
            $ids = $ids->modelKeys();
        }

If this is the intended behavior we can change everything to be a collection. Though at least in my opinion if it supports a Collection of models why not support an array of models?

But I see in the docs it never mentioned that it would accept an array or a collection of models.

We can map everything to just get the ID instead of passing the whole model but I must admit that it would be nice to not need to map those every single time.