fix(core): self-referencing relations `Not unique table/alias` #2740

monrostar · 2024-03-15T15:58:36Z

Description

This code bypasses the typeorm bug related to an error in duplicate aliases when linking to a parent table
Related issue: #2738

Breaking changes

Does this PR include any breaking changes we should be aware of?

no

Checklist

📌 Always:

I have set a clear title
My PR is small and contains a single feature
I have checked my own PR

👍 Most of the time:

I have added or updated test cases
I have updated the README if needed

monrostar · 2024-03-15T16:02:44Z

packages/core/src/service/helpers/list-query-builder/list-query-builder.ts

-            if (!currentParentIsTreeType && !currentMetadataIsTreeType) {
+            let currentMetadataHasOneOrMoreSelfReferencingRelations = false;
+            // Check if the current entity has one or more self-referencing relations
+            // to determine if it is a tree type or has tree relations.
+            for (const relation of currentMetadata.relations) {
+                if (relation.inverseEntityMetadata === currentMetadata) {
+                    currentMetadataHasOneOrMoreSelfReferencingRelations = true;
+                    break;
+                }
+            }
+
+            if (!currentParentIsTreeType && !currentMetadataIsTreeType && !currentMetadataHasOneOrMoreSelfReferencingRelations) {
                return;
            }


This is the most important code. It is enough to determine whether it is necessary to do join manually or leave this job to typeorm

monrostar · 2024-03-15T16:03:47Z

packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts

+
+    @ManyToOne(() => TestEntity, (type) => type.parent)
+    parent: TestEntity | null;
+
+    @Column('int', { nullable: true })
+    parentId: ID | null;


Here I had to update the model a bit to repeat the error and take it into account in the tests

above fix i think need consider more behavior
if we have translations like

@OneToMany( () => LensProcessOptionTranslation, (translation) => translation.base, { eager: true } ) translations: Array<Translation<LensProcessOption>>;

the parent should be auto load translations otherwise it will result translateEntity failed

function translateLeaf( object: { [key: string]: any } | undefined, property: string, languageCode: LanguageCode | [LanguageCode, ...LanguageCode[]], ): any { if (object && object[property]) { if (Array.isArray(object[property])) { return object[property].map((nested2: any) => translateEntity(nested2, languageCode)); } else if (object[property]) { return translateEntity(object[property], languageCode); } } }

if parent existed but it have auto loaded translations it will broken here

please see my latest updated repo https://github.com/tianyingchun/next-issue/tree/vendure_issue_2738

InternalServerError [GraphQLError]: error.entity-has-no-translation-in-language at new I18nError (/Users/tianyingchun/Documents/coding/kzfoo/vendure-issue/node_modules/@vendure/core/dist/i18n/i18n-error.js:21:9) at new InternalServerError (/Users/tianyingchun/Documents/coding/kzfoo/vendure-issue/node_modules/@vendure/core/dist/common/error/errors.js:15:9) at translateEntity (/Users/tianyingchun/Documents/coding/kzfoo/vendure-issue/node_modules/@vendure/core/dist/service/helpers/utils/translate-entity.js:33:15) at translateLeaf (/Users/tianyingchun/Documents/coding/kzfoo/vendure-issue/node_modules/@vendure/core/dist/service/helpers/utils/translate-entity.js:104:20) at translateDeep (/Users/tianyingchun/Documents/coding/kzfoo/vendure-issue/node_modules/@vendure/core/dist/service/helpers/utils/translate-entity.js:89:21) at TranslatorService.translate (/Users/tianyingchun/Documents/coding/kzfoo/vendure-issue/node_modules/@vendure/core/dist/service/helpers/translator/translator.service.js:54:53) at file:///Users/tianyingchun/Documents/coding/kzfoo/vendure-issue/packages/plugin-issue/src/services/menu.service.ts:82:76 at Array.map (<anonymous>) at file:///Users/tianyingchun/Documents/coding/kzfoo/vendure-issue/packages/plugin-issue/src/services/menu.service.ts:82:37 at process.processTicksAndRejections (node:internal/process/task_queues:95:5) { path: undefined, locations: undefined, extensions: { code: 'INTERNAL_SERVER_ERROR' }, variables: { entityName: 'Menu', languageCode: ',,en' }, code: 'INTERNAL_SERVER_ERROR', logLevel: 0 }

Interesting. I thought I had accounted for translations for all self-related relations. I will look into this and add join for eager relations

above fix i think need consider more behavior if we have translations like

@OneToMany( () => LensProcessOptionTranslation, (translation) => translation.base, { eager: true } ) translations: Array<Translation<LensProcessOption>>;

the parent should be auto load translations otherwise it will result translateEntity failed

function translateLeaf( object: { [key: string]: any } | undefined, property: string, languageCode: LanguageCode | [LanguageCode, ...LanguageCode[]], ): any { if (object && object[property]) { if (Array.isArray(object[property])) { return object[property].map((nested2: any) => translateEntity(nested2, languageCode)); } else if (object[property]) { return translateEntity(object[property], languageCode); } } }

if parent existed but it have auto loaded translations it will broken here

monrostar · 2024-03-16T15:24:49Z

@michaelbromley
10c0798
Added a fix. And refactored the code a bit.

I'm not sure if we should look for translation in nested objects automatically because that would lead to uncontrolled execution of this method.

translateDeep works fine.

tianyingchun · 2024-03-17T01:44:08Z

oh it seems that this change also have some issue here if we have relation comes from inherits entity consider hehavior as bellow:

// module.entity.ts
@Entity('module')
@TableInheritance({ column: { type: 'varchar', name: 'type' } })
export class Module extends VendureEntity implements ModuleChannelAware {
  constructor(input?: DeepPartial<Module>) {
    super(input);
  }

  @Column()
  type: string;


  name: LocaleString;

  @OneToMany(() => ModuleAcl, (moduleAcl) => moduleAcl.module)
  moduleAcls: ModuleAcl[];


  @ManyToMany(() => ModuleChannel)
  @JoinTable()
  channels: ModuleChannel[];

  @OneToMany(() => ModuleTranslation, (translation) => translation.base, {
    eager: true,
  })
  translations: Array<Translation<Module>>;
}

// module-translation.entity.ts


@Entity('module_translation')
export class ModuleTranslation
  extends VendureEntity
  implements Translation<Module>
{
  constructor(input?: DeepPartial<Translation<Module>>) {
    super(input);
  }

  @Column('varchar')
  languageCode: LanguageCode;

  @Column()
  name: string;

  @ManyToOne(() => Module, (base) => base.translations, {
    onDelete: 'CASCADE',
  })
  base: Relation<Module>;
}

// module-menu.entity.ts

@ChildEntity('module_menu')
export class ModuleMenu extends Module {
  constructor(input: DeepPartial<ModuleMenu>) {
    super(input);
  }


  @Column({ default: false })
  invisible: boolean;

  @Column({ type: 'varchar', nullable: true })
  iconImgActive: string | null;

  @Column({ default: '_self' })
  target: string;

  @TreeParent()
  parent: ModuleMenu | null;

  @Column('int', { nullable: true })
  parentId: ID | null;

  @TreeChildren()
  children: ModuleMenu[];
}

// module-acl.entity.ts
@Entity('module_acl')
export class ModuleAcl extends VendureEntity {
  constructor(input?: DeepPartial<ModuleAcl>) {
    super(input);
  }

  aclTagName: LocaleString;


  @ManyToOne(() => Module)
  module: Relation<Module>;

  @Column('int')
  moduleId: ID;


  @OneToMany(() => ModuleAclTranslation, (translation) => translation.base, {
    eager: true,
  })
  translations: Array<Translation<ModuleAcl>>;
}

// module-acl-translation.entity.ts

@Entity('module_acl_translation')
export class ModuleAclTranslation
  extends VendureEntity
  implements Translation<ModuleAcl>
{
  constructor(input?: DeepPartial<Translation<ModuleAcl>>) {
    super(input);
  }

  @Column('varchar')
  languageCode: LanguageCode;

  @Column({ unique: true })
  aclTagName: string;

  @ManyToOne(() => ModuleAcl, (base) => base.translations, {
    onDelete: 'CASCADE',
  })
  base: Relation<ModuleAcl>;
}

// module-channel.entity.ts
@Entity('module_channel')
export class ModuleChannel extends VendureEntity {
  constructor(input?: DeepPartial<ModuleChannel>) {
    super(input);
  }

  @Column({ unique: true })
  code: string;

  @Column({ unique: true })
  token: string;

  description: LocaleString;


  @OneToMany(
    () => ModuleChannelTranslation,
    (translation) => translation.base,
    {
      eager: true,
    }
  )
  translations: Array<Translation<ModuleChannel>>;
}

// module-channel-translation.entity.ts

@Entity('module_channel_translation')
export class ModuleChannelTranslation
  extends VendureEntity
  implements Translation<ModuleChannel>
{
  constructor(input?: DeepPartial<Translation<ModuleChannel>>) {
    super(input);
  }

  @Column('varchar')
  languageCode: LanguageCode;


  @Column({ default: '', nullable: true })
  description: string;

  @ManyToOne(() => ModuleChannel, (base) => base.translations, {
    onDelete: 'CASCADE',
  })
  base: Relation<ModuleChannel>;
}

  public async findAll(
    ctx: RequestContext,
    options?: ListQueryOptions<ModuleMenu>,
  ) {
    return this.listQueryBuilder
      .build(ModuleMenu, options, {
        ctx,
        relations: ['moduleAcls', 'channels']
      })
      .getManyAndCount()
      .then(([modules, totalItems]) => {
        const items = modules.map((moduleItem) =>
          this.translator.translate(moduleItem, ctx, ['moduleAcls', 'channels'])
        );

        return {
          items,
          totalItems,
        };
      });
  }

it will lose translations property on channels or moduleAcls if they have records it will throw

InternalServerError [GraphQLError]: error.entity-has-no-translation-in-language
...
  path: undefined,
  locations: undefined,
  extensions: { code: 'INTERNAL_SERVER_ERROR' },
  variables: { entityName: 'ModuleChannel', languageCode: ',,en' },
  code: 'INTERNAL_SERVER_ERROR',
  logLevel: 0

monrostar · 2024-03-17T06:39:56Z

Can you also provide the missing code from ModuleChannelAware?
At the moment I can see that there might be an error due to missing ReverseSide for relations, but I guess that can be bypassed too.

export class Module extends VendureEntity implements ModuleChannelAware {

tianyingchun · 2024-03-17T06:45:00Z

/**
 * @description
 * Entities which can be assigned to Module Channels should implement this interface.
 */
export interface ModuleChannelAware {
  channels: ModuleChannel[];
}

monrostar · 2024-03-17T07:21:11Z

@michaelbromley
I think the attempt to fix the next bug should be done in a separate PR because there are too many changes here already

In this PR, the first bug has been fixed. I'll open a new PR from #2744

michaelbromley · 2024-03-18T10:03:32Z

@monrostar yep, makes sense to split the other changes.

@tianyingchun thank you very much for the ongoing feedback!

fix(core): self-referencing relations Not unique table/alias

4034445

monrostar commented Mar 15, 2024

View reviewed changes

fix(core): update self-related manual joins with deep eager relations

10c0798

monrostar added 2 commits March 16, 2024 16:25

chore(core): clean up code

4824d86

fix(core): update test cases for new TestEntities order

5eb6391

michaelbromley merged commit 357ba49 into vendure-ecommerce:minor Mar 18, 2024
9 of 12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(core): self-referencing relations `Not unique table/alias` #2740

fix(core): self-referencing relations `Not unique table/alias` #2740

monrostar commented Mar 15, 2024 •

edited

Loading

monrostar Mar 15, 2024

monrostar Mar 15, 2024

tianyingchun Mar 16, 2024

tianyingchun Mar 16, 2024

tianyingchun Mar 16, 2024

monrostar Mar 16, 2024 •

edited

Loading

monrostar commented Mar 16, 2024 •

edited

Loading

tianyingchun commented Mar 17, 2024 •

edited

Loading

monrostar commented Mar 17, 2024

tianyingchun commented Mar 17, 2024 •

edited

Loading

monrostar commented Mar 17, 2024

michaelbromley commented Mar 18, 2024

fix(core): self-referencing relations Not unique table/alias #2740

fix(core): self-referencing relations Not unique table/alias #2740

Conversation

monrostar commented Mar 15, 2024 • edited Loading

Description

Breaking changes

Checklist

monrostar Mar 15, 2024

Choose a reason for hiding this comment

monrostar Mar 15, 2024

Choose a reason for hiding this comment

tianyingchun Mar 16, 2024

Choose a reason for hiding this comment

tianyingchun Mar 16, 2024

Choose a reason for hiding this comment

tianyingchun Mar 16, 2024

Choose a reason for hiding this comment

monrostar Mar 16, 2024 • edited Loading

Choose a reason for hiding this comment

monrostar commented Mar 16, 2024 • edited Loading

tianyingchun commented Mar 17, 2024 • edited Loading

monrostar commented Mar 17, 2024

tianyingchun commented Mar 17, 2024 • edited Loading

monrostar commented Mar 17, 2024

michaelbromley commented Mar 18, 2024

fix(core): self-referencing relations `Not unique table/alias` #2740

fix(core): self-referencing relations `Not unique table/alias` #2740

monrostar commented Mar 15, 2024 •

edited

Loading

monrostar Mar 16, 2024 •

edited

Loading

monrostar commented Mar 16, 2024 •

edited

Loading

tianyingchun commented Mar 17, 2024 •

edited

Loading

tianyingchun commented Mar 17, 2024 •

edited

Loading