TriPSs / nestjs-query

Easy CRUD for GraphQL.
https://tripss.github.io/nestjs-query/
MIT License
161 stars 43 forks source link

Nested items wrongly returned as null #307

Open rostaingc opened 3 weeks ago

rostaingc commented 3 weeks ago

Describe the bug

Nested items are returned as null, when they are not.

Example query

todoItems {
  id
  name
  toTags {
    tag {
      id
      name
    }
  }
}

You have two tables todoItems and tags as well as a many2many join table todo-to-tag. If there are soft deleted records inside the join table and you try to query the todoItems with their linked tags, all todoItems will be returned with the right number of toTags (join column) records but some of the tags inside the toTags will be returned as null.

Have you read the Contributing Guidelines?

Yes

To Reproduce Steps to reproduce the behavior:

I have created a fork of this repo and added an example and failling test that should pass once the issue is fixed. https://github.com/rostaingc/nestjs-query

Expected behavior All the items in the nested objects should be returned and not be null.

Additional context

When analyzing the SQL queries made by the nestjs query, what happens is:

The issue here is that in query C, there is no check on deleted for the join table ("todos-to-tags"."deleted" IS NULL), thus leading to deleted join records being returned, and non deleted join items being left out because of the LIMIT.

When the data is then aggregated and put inside each nest item, some of the tags are null as they were cut off by the LIMIT.

The solution would be to make sure that there is a check for deleted IS NULL on all tables of the query in query C.

TriPSs commented 3 weeks ago

Thanks for the detailed ticket, if I check the repo I do not see any additional commits? Could you create a PR to this repo with the test? That way I can more easily checkout that code locally.

rostaingc commented 3 weeks ago

Sorry I didnt realize i had not pushed the last commit

Here is the PR for it.

https://github.com/TriPSs/nestjs-query/pull/308

rostaingc commented 3 weeks ago

@TriPSs Let me know if you have enough info with the example in the PR or if you want any additional details Thanks

TriPSs commented 2 weeks ago

So after investigating your PR (thanks for that!) I found what caused it, changes made in #99 are the cause (this thread more specifically).

Sadly I do not yet see a solution to the issue that will make sure both cases work, we would need to investigate if there is any, if not what behavior we want/expect to be correct.

Maybe @hedwiggggg can also weigh in here?