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

[BUG] Cannot override relation with Omit #142

Closed
lnquy065 opened this issue Oct 31, 2022 · 4 comments
Closed

[BUG] Cannot override relation with Omit #142

lnquy065 opened this issue Oct 31, 2022 · 4 comments

Comments

@lnquy065
Copy link

Describe the bug
I want to override countries to ManyToMany.
But it didn't work, I have to create a new field country_list instead to pass this case

MedusaRegionEntity

@Entity()
export class Region extends SoftDeletableEntity {
    @OneToMany(() => Country, (c) => c.region)
     countries: Country[]
}
MedusaExRegionEntity

@MedusaEntity({ override: MedusaRegion })
@Entity()
export class Region extends Omit(MedusaRegion, ["countries"]) {

  @ManyToMany(() => Country, (country) => country.regions, {
    cascade: true,
  })
  @JoinTable({
    name: "region_country",
    joinColumn: {
      name: "region_id",
      referencedColumnName: "id",
    },
    inverseJoinColumn: {
      name: "country_id",
      referencedColumnName: "id",
    },
  })
  countries: Country[];
}
@adrien2p
Copy link
Owner

Can you give is the error you get or the behavior you had? Also, if you could share your migration 😀

@lnquy065
Copy link
Author

hi, it has no error when running, but when I add a new Country into countries and save, it will become one-to-many like Medusa default entity. It seems like my many-to-many countries can't override the one-to-many countries of Medusa entity.

Then, I try to rename many-to-many countries in MedusaExRegionEntity to country_list, and it works perfectly:

MedusaExRegionEntity

@MedusaEntity({ override: MedusaRegion })
@Entity()
export class Region extends Omit(MedusaRegion, ["countries"]) {

  @ManyToMany(() => Country, (country) => country.regions, {
    cascade: true,
  })
  @JoinTable({
    name: "region_country",
    joinColumn: {
      name: "region_id",
      referencedColumnName: "id",
    },
    inverseJoinColumn: {
      name: "country_id",
      referencedColumnName: "id",
    },
  })
  country_list: Country[]; // <=== rename here, then it works!!
}

And my migration file:

import {
  MigrationInterface,
  QueryRunner,
  Table,
  TableForeignKey,
  TableIndex,
} from "typeorm";
import { Migration } from "medusa-extender";

@Migration()
export class RegionMigration1667183653406 implements MigrationInterface {
  name = "RegionMigration1667183653406";
  TABLE_NAME = "region_country";
  COL_REGION_ID = "region_id";
  COL_COUNTRY_ID = "country_id";
  REGION_FK = "regionIdFK";
  COUNTRY_FK = "countryIdFK";
  INDEX_NAME = "regionCountryIdUniqueIndex";

  public async up(queryRunner: QueryRunner): Promise<void> {
    // CREATE JOIN TABLE
    await queryRunner.createTable(
      new Table({
        name: this.TABLE_NAME,
        columns: [
          {
            name: this.COL_REGION_ID,
            type: "text",
          },
          {
            name: this.COL_COUNTRY_ID,
            type: "int",
          },
        ],
      }),
      true
    );

    //  CREATE FK - REGION
    await queryRunner.createForeignKey(
      this.TABLE_NAME,
      new TableForeignKey({
        name: this.REGION_FK,
        columnNames: [this.COL_REGION_ID],
        referencedColumnNames: ["id"],
        referencedTableName: "region",
        onDelete: "CASCADE",
        onUpdate: "CASCADE",
      })
    );

    //  CREATE FK - COUNTRY
    await queryRunner.createForeignKey(
      this.TABLE_NAME,
      new TableForeignKey({
        name: this.COUNTRY_FK,
        columnNames: [this.COL_COUNTRY_ID],
        referencedColumnNames: ["id"],
        referencedTableName: "country",
        onDelete: "CASCADE",
        onUpdate: "CASCADE",
      })
    );

    // CREATE INDEX
    await queryRunner.createIndex(
      this.TABLE_NAME,
      new TableIndex({
        name: this.INDEX_NAME,
        columnNames: [this.COL_REGION_ID, this.COL_COUNTRY_ID],
      })
    );
  }

  public async down(queryRunner: QueryRunner): Promise<void> {
    await queryRunner.dropIndex(this.TABLE_NAME, this.INDEX_NAME);
    await queryRunner.dropForeignKey(this.TABLE_NAME, this.REGION_FK);
    await queryRunner.dropForeignKey(this.TABLE_NAME, this.COUNTRY_FK);
    await queryRunner.dropTable(this.TABLE_NAME);
  }
}

Now it okay with country_list, but I want to re-use the countries field

@adrien2p
Copy link
Owner

adrien2p commented Oct 31, 2022

I see thank you for the details.

The thing is that the omit has its own limitation, it is only a type omitting which simplify typings when you override a field. It is very useful for simple type like primitive, but would require far more for this type of changes in terms of relations. The fact is that changing from one to many to many to many would impact the core as well and would require to take that into account. This can't be done automatically. I think the easiest way would have like you did a separate attribute, unless you want to rewrite the parts of the core depending on this attribute. In the later case you have to do the core changes by overriding the appropriate places (services, validators for the endpoints, etc...), you should undo the actual foreign key and other constraints and then add your new constraints.

Does that make sense?

@lnquy065
Copy link
Author

lnquy065 commented Nov 1, 2022

Ahh, I got it. Thanks for your explanation. I think I'd better leave medusa's constraints and create new ones of my own.

@adrien2p adrien2p closed this as completed Nov 1, 2022
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

No branches or pull requests

2 participants