Open jwadhams opened 2 years ago
Turned on query logging, this PHP:
\App\Models\AccountIntegrations\TP_TradeEstimate::onlyTrashed()->count();
generates this SQL:
select count(*) as aggregate from `account_integrations` where `account_integrations`.`deleted_at` is not null and `account_integrations`.`type` in (?) ["TP_TradeEstimate"]
but this php:
\App\Models\AccountIntegrations\TP_TradeEstimate::onlyTrashed()->forceDelete();
generates this SQL
delete from `account_integrations` where `account_integrations`.`deleted_at` is not null
... which suggests the problem is somehow specifically with forceDelete
not a something obvious like STI scopes being cleared by the onlyTrashed
scope.
OK, my current hypothesis is that this is a design error in, of all things, MassPrunable.
The implementation and unit tests of \Illuminate\Database\Eloquent\MassPrunable::pruneAll
assume that you can force delete all models that meet the prunable criteria, without either prunable or pruneAll bothering to use something like withTrashed
.
\Illuminate\Database\Eloquent\Builder::forceDelete
, in its commentary, explicitly does NOT apply scopes, but that doesn't seem relevant to its "force" job. What it really needs to ignore is the onDelete
behavior (which turns deletes into updates). If I rebuild forceDelete to basically be a clone of delete
without onDelete
the only tests that fail are related to pruning.
I've got an almost-ready-to-PR branch against Laravel Framework that I'll link here when I submit it. At this rate it looks like no changes to this library will be necessary, but the issue will affect any unpatched versions of Laravel.
I'm planning on working this until I have a proposed patch, but wanted to give a heads up and see if you had any advice.
Here's my reproduction case in my own codebase: (TP_TradeEstimate and Hours are both subclasses of the same thing)
This fails because $affectedRows is actually 2, and if you comment out that assertion, the last assertion fails because $deletedOther has been forceDeleted. Even weirder, obviously onlyTrashed by itself isn't the problem, that count works, and if you
get()
the collection and doeach->forceDelete()
there's no problem.Using single-table-inheritance
v1.0.0
with Laravel framework9.36.2