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

Closed
wants to merge 1 commit into 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.

@jaulz
Copy link
Contributor Author

jaulz commented Nov 15, 2024

I guess you are right... I had that REST pattern in mind that you return the entity that you deleted so you can restore it afterwards. With GraphQL we could potentially run the query to read it before the mutation happens in the same request.

@jaulz jaulz closed this Nov 15, 2024
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