adrien2p / medusa-extender

:syringe: Medusa on steroid, take your medusa project to the next level with some badass features :rocket:
https://adrien2p.github.io/medusa-extender/
MIT License
320 stars 38 forks source link

[BUG] Cannot override relation with Omit #142

Closed lnquy065 closed 1 year ago

lnquy065 commented 1 year ago

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 commented 1 year ago

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

lnquy065 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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