Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.x] Where doesnt have nullable morph #54363

Open
wants to merge 10 commits into
base: 11.x
Choose a base branch
from

Conversation

liamduckett
Copy link
Contributor

Hey, this PR aims to resolve this issue.

The problem currently (as described in the issue) is that QueriesRelationships:: excludes Models with a null morph_type value.

This is due to the logic in QueriesRelationships::hasMorph revolving around types (which exludes null):

// src/Illuminate/Database/Eloquent/Concerns/QueriesRelationships.php (231)
if ($types === ['*']) {
    $types = $this->model->newModelQuery()->distinct()->pluck($relation->getMorphType())
        ->filter()
        ->map(fn ($item) => enum_value($item))
        ->all();
}

My proposed solution adds a conditional clause to the query, to also include models with a null morph relation. I made the decision to widen the scope to include other operators - as this felt more complete of a solution than just solving for default whereDoesntHave. I understand this comes with it's own drawbacks, and am happy to partly undo if desired.

This only applies if the chosen type is wildcard, as I understand choosing types currently means any results must have that type of morph relation (and naturally, null cannot acheieve this). Truthfully I'm not particularily confident on this bit - would appreciate any checking over!

I've only added the test case from the issue, as it feels like the majority use case.

Please see my logic for deciding which operators to include below.

operators;
1. >=
  a) positive: doesnt need nulls as is inclusionary
  b) zero:     will always be true...
  c) negative: will always be true...
2. >
  a) positive: doesnt need nulls as is inclusionary
  b) zero:     doesnt need nulls as is inclusionary
  c) negative: will always be true...
3. <=
  a) positive: needs nulls as is exclusionary
  b) zero:     needs nulls as is exclusionary
  c) negative: can never be true...
4. <
  a) positive: needs nulls as is exclusionary
  b) zero:      can never be true...
  c) negative: can never be true...
5. =
  a) positive: doesnt need nulls as is inclusionary
  b) zero:     needs nulls as is exclusionary
  c) negative: can never be true...
6. !=
  a) positive: needs nulls as is exclusionary
  b) zero:     doesnt need nulls as is inclusionary
  c) negative: will always be true...

@liamduckett liamduckett force-pushed the where-doesnt-have-nullable-morph branch from bbcc7ef to 28e67ef Compare January 29, 2025 20:49
@liamduckett liamduckett force-pushed the where-doesnt-have-nullable-morph branch from 28e67ef to 02b7f76 Compare January 29, 2025 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

whereDoesntHave not produce correct query when use with nullableMorphs
2 participants