ListQueryBuilder with customPropertyMap fails for two or more relations on the same type

[ashitikov]

Describe the bug Image you have an entity:

@Entity()
class SomeEntity {
  ...

  @JoinTable()
  @ManyToMany(type => ProductVariant)
  parentProductVariants!: ProductVariant[]

  @ManyToOne(type => ProductVariant, { onDelete: 'CASCADE' })
  productVariant!: ProductVariant
}

Note there is two relations, where target is ProductVariant entity.

You want to build a list query using custom property map:

this.listQueryBuilder.build(ctx, SomeEntity, options, {
  relations: ['productVariant'],
  customPropertyMap: {
    name: 'product_variant.product_variant_translation.name'
  }
});

Generated query will contain incorrect join on parentProductVariants relation, instead of productVariant. Thats because of these lines:

1. https://github.com/vendure-ecommerce/vendure/blob/a73a4048278f5a38e49ac9376ac29922293f4810/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts#L330 . There we find a first relation on the entity. In our case the first relation is 'parentProductVariants'.
2. https://github.com/vendure-ecommerce/vendure/blob/a73a4048278f5a38e49ac9376ac29922293f4810/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts#L468 . There we find a first alias from expression map available to the entity target 'ProductVariant' which stands for a 'parentProductVariants' relation.

Expected behavior Correct query generation with the right relations to be joined.

Environment (please complete the following information):

• @vendure/core version: 1.7.1
• Nodejs version: 14
• Database (mysql/postgres etc): postgres

Additional context I don't quite understand how we can solve this issue using a table name in customPropertyMap rather than relation name. So the question is why customPropertyMap requires table names?

[xrev87]

Additional context I don't quite understand how we can solve this issue using a table name in customPropertyMap rather than relation name. So the question is why customPropertyMap requires table names?

I was looking at this a few months ago as well and I fully agree with this statement - the logic around this seems to be very complex and unnecessary in the face of relation paths; the current set up also precludes you from using a customPropertyMap for many to many type relations and second level relations.

[michaelbromley]

Would anyone like to build a poc PR which does this better? Is it possible to do in a non-breaking way?

[michaelbromley]

As of v1.8.2, you can (and should) use the relation path rather than table names for your customPropertyMap 👍