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

fix: return deleted models that have relations #2619

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jaulz
Copy link
Contributor

@jaulz jaulz commented Oct 11, 2024

  • Added or updated tests
  • Documented user facing changes
  • Updated CHANGELOG.md

Changes

Right now, it's not possible to return deleted models and query relations at the same time. For example, when running a mutation that deletes a model I would like to still return the model that was just deleted but if any of its relations is queried as well it will run into an exception, e.g.:

Error: Call to a member function getAttributes() on null in /app/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Collection.php:96
Stack trace:
#0 /app/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Collection.php(119): Illuminate\Database\Eloquent\Collection->loadAggregate(Array, '*', 'count')
#1 /app/vendor/nuwave/lighthouse/src/Execution/ModelsLoader/CountModelsLoader.php(18): Illuminate\Database\Eloquent\Collection->loadCount(Array)
#2 /app/vendor/nuwave/lighthouse/src/Execution/BatchLoader/RelationBatchLoader.php(68): Nuwave\Lighthouse\Execution\ModelsLoader\CountModelsLoader->load(Object(Illuminate\Database\Eloquent\Collection))
#3 /app/vendor/nuwave/lighthouse/src/Execution/BatchLoader/RelationBatchLoader.php(49): Nuwave\Lighthouse\Execution\BatchLoader\RelationBatchLoader->resolve()
#4 /app/vendor/webonyx/graphql-php/src/Executor/Promise/Adapter/SyncPromise.php(63): Nuwave\Lighthouse\Execution\BatchLoader\RelationBatchLoader->Nuwave\Lighthouse\Execution\BatchLoader\{closure}()

This PR serves as a draft and is open to discussion how to handle for example the following cases:

  • Shall we return null or 0 on related counts?
  • Shall we return null or empty array on related collections?

Breaking changes

tbd

@spawnia
Copy link
Collaborator

spawnia commented Oct 11, 2024

I would like to still return the model that was just deleted

Why though? I am starting to doubt if Lighthouse should do that.

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.

2 participants