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

ListQueryBuilder parallelLoadRelations does not handle multiple customFields correctly #2555

Closed
jnugh opened this issue Nov 29, 2023 · 1 comment · Fixed by #2566
Closed
Assignees
Labels
type: bug 🐛 Something isn't working

Comments

@jnugh
Copy link
Contributor

jnugh commented Nov 29, 2023

Describe the bug
When querying multiple entities with customFields ListQueryBuilder.parallelLoadRelations is being called. The method loads relations that are part of the original GraphQL query to make sure they don't need to be fetched sequentially. When querying multiple customFields relations only the first one will be populated due to the way the fetched relationPaths are being handled.

The issue seems to be in the area:
https://github.com/vendure-ecommerce/vendure/blob/master/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts#L486C9-L499C35

results.map(r => ({ relation: relationPaths[0] as keyof T, entity: r })), only writes the first relation with a common prefix to the entitiesIdsWithRelations array which is fine if the rest of the relation path are nested relations that are already part of the quer result.
In case of customFields the relationsPaths might look similar to this:

["customFields.relationA", "customFields.relationB"]

entitiesIdsWithRelations will only contain customFields.relationA and thus only populate customFields.relationA event though customFields.relationB has been fetched as well.

To Reproduce
Steps to reproduce the behavior:

  1. Create at least two customFields with relations
  2. Use a list query and fetch both relations simultaneously
  3. Inspect the database queries executed to resolve the second relation in customFields

Expected behavior
The relation is not fetched on a per-parent basis but in a single query that fetches all children of all parent entities at once.

Environment (please complete the following information):

  • @vendure/core version: 2.1.2
  • Nodejs version: v18.17.0
  • Database (mysql/postgres etc): postgres

Additional context
A potential fix would be to write all relation paths into entitiesIdsWithRelations and add a loop during the assignment. Please let me know if you would like to accept a PR on this.

@jnugh jnugh added the type: bug 🐛 Something isn't working label Nov 29, 2023
@michaelbromley
Copy link
Member

Hi!
Thanks for the report. A PR (ideally with a test that fails before your changes) would be very welcome :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants