fix(utils): DML hasOne - belongsTo not behaving correctly (#8813)

FIXES FRMW-2676

**What**
ref: https://discord.com/channels/876835651130097704/1023889804544458752/threads/1276979858781503528

Currently, when providing the following

```ts
const user1 = manager.create(User, {
  username: "User 1",
  team: {
    name: "Team 1",
  },
})
```

It would result in an error inserting into the database because the foreign key will be sent twice as part of the insert, one for the relation and one for the foreign key that both relate to the foreign key property.

To fix that and allow both approaches (providing the entity to cascade persist or just providing the foreign key in case of another side nullable relation) we need to handle it a bit differently.

now both approaches would be valid. the entities for the example might not be the best ones but it is just to illustrate
option 1 - we create both the user and the team:
```ts
const user1 = manager.create(User, {
  username: "User 1",
  team: {
    name: "Team 1",
  },
})
```
option 2 - the team already exists (for example the previous user have been detached from the team but we kept the team alive and assign a new user to that team) :
```ts
const user1 = manager.create(User, {
  username: "User 1",
  team_id: team.id
})
```
This commit is contained in:
Adrien de Peretti
2024-08-28 17:45:48 +02:00
committed by GitHub
parent 6ea5a15762
commit 52e394055c
3 changed files with 186 additions and 8 deletions

View File

@@ -2361,6 +2361,7 @@ describe("Entity builder", () => {
setter: false,
type: "string",
isForeignKey: true,
persist: false,
},
created_at: {
reference: "scalar",
@@ -3242,6 +3243,7 @@ describe("Entity builder", () => {
getter: false,
setter: false,
isForeignKey: true,
persist: false,
},
created_at: {
reference: "scalar",
@@ -3422,6 +3424,7 @@ describe("Entity builder", () => {
getter: false,
setter: false,
isForeignKey: true,
persist: false,
},
created_at: {
reference: "scalar",
@@ -4026,6 +4029,7 @@ describe("Entity builder", () => {
getter: false,
setter: false,
isForeignKey: true,
persist: false,
},
created_at: {
reference: "scalar",
@@ -4214,6 +4218,7 @@ describe("Entity builder", () => {
getter: false,
setter: false,
isForeignKey: true,
persist: false,
},
created_at: {
reference: "scalar",

View File

@@ -13,6 +13,7 @@ import {
OneToOne,
OnInit,
Property,
rel,
} from "@mikro-orm/core"
import { camelToSnakeCase, pluralize } from "../../../common"
import { ForeignKey } from "../../../dal/mikro-orm/decorators/foreign-key"
@@ -116,6 +117,23 @@ export function defineBelongsToRelationship(
* Hook to handle foreign key assignation
*/
MikroORMEntity.prototype[hookName] = function () {
/**
* In case of has one relation, in order to be able to have both ways
* to associate a relation (through the relation or the foreign key) we need to handle it
* specifically
*/
if (HasOne.isHasOne(otherSideRelation)) {
const relationMeta = this.__meta.relations.find(
(relation) => relation.name === relationship.name
).targetMeta
this[relationship.name] ??= rel(
relationMeta.class,
this[foreignKeyName]
)
this[relationship.name] ??= this[relationship.name]?.id
return
}
this[relationship.name] ??= this[foreignKeyName]
this[foreignKeyName] ??= this[relationship.name]?.id
}
@@ -179,19 +197,18 @@ export function defineBelongsToRelationship(
onDelete: shouldCascade ? "cascade" : undefined,
})(MikroORMEntity.prototype, relationship.name)
if (relationship.nullable) {
Object.defineProperty(MikroORMEntity.prototype, foreignKeyName, {
value: null,
configurable: true,
enumerable: true,
writable: true,
})
}
Object.defineProperty(MikroORMEntity.prototype, foreignKeyName, {
value: null,
configurable: true,
enumerable: true,
writable: true,
})
Property({
type: "string",
columnType: "text",
nullable: relationship.nullable,
persist: false,
})(MikroORMEntity.prototype, foreignKeyName)
ForeignKey()(MikroORMEntity.prototype, foreignKeyName)

View File

@@ -0,0 +1,156 @@
import { MetadataStorage, MikroORM } from "@mikro-orm/core"
import { model } from "../../entity-builder"
import { toMikroOrmEntities } from "../../helpers/create-mikro-orm-entity"
import { createDatabase, dropDatabase } from "pg-god"
import { CustomTsMigrationGenerator, mikroOrmSerializer } from "../../../dal"
import { EntityConstructor } from "@medusajs/types"
import { pgGodCredentials } from "../utils"
import { FileSystem } from "../../../common"
import { join } from "path"
export const fileSystem = new FileSystem(
join(__dirname, "../../integration-tests-migrations-has-one-belongs-to")
)
describe("hasOne - belongTo", () => {
const dbName = "EntityBuilder-HasOneBelongsTo"
let orm!: MikroORM
let Team: EntityConstructor<any>, User: EntityConstructor<any>
afterAll(async () => {
await fileSystem.cleanup()
})
beforeEach(async () => {
MetadataStorage.clear()
const team = model.define("team", {
id: model.id().primaryKey(),
name: model.text(),
user: model.belongsTo(() => user, { mappedBy: "team" }),
})
const user = model.define("user", {
id: model.id().primaryKey(),
username: model.text(),
team: model.hasOne(() => team, { mappedBy: "user" }).nullable(),
})
;[User, Team] = toMikroOrmEntities([user, team])
await createDatabase({ databaseName: dbName }, pgGodCredentials)
orm = await MikroORM.init({
entities: [Team, User],
tsNode: true,
dbName,
password: pgGodCredentials.password,
host: pgGodCredentials.host,
user: pgGodCredentials.user,
type: "postgresql",
migrations: {
generator: CustomTsMigrationGenerator,
path: fileSystem.basePath,
},
})
const migrator = orm.getMigrator()
await migrator.createMigration()
await migrator.up()
})
afterEach(async () => {
await orm.close()
await dropDatabase(
{ databaseName: dbName, errorIfNonExist: false },
pgGodCredentials
)
})
it(`should handle the relation properly`, async () => {
let manager = orm.em.fork()
const user1 = manager.create(User, {
username: "User 1",
team: {
name: "Team 1",
},
})
await manager.persistAndFlush([user1])
manager = orm.em.fork()
const user = await manager.findOne(
User,
{
id: user1.id,
},
{
populate: ["team"],
}
)
expect(mikroOrmSerializer<InstanceType<typeof User>>(user)).toEqual({
id: user1.id,
username: "User 1",
created_at: expect.any(Date),
updated_at: expect.any(Date),
deleted_at: null,
team: {
id: expect.any(String),
name: "Team 1",
created_at: expect.any(Date),
updated_at: expect.any(Date),
deleted_at: null,
user_id: user1.id,
},
})
})
it(`should handle the relation properly with just the relation id`, async () => {
let manager = orm.em.fork()
const user1 = manager.create(User, {
username: "User 1",
})
await manager.persistAndFlush([user1])
manager = orm.em.fork()
const team1 = manager.create(Team, {
name: "Team 1",
user_id: user1.id,
})
await manager.persistAndFlush([team1])
manager = orm.em.fork()
const user = await manager.findOne(
User,
{
id: user1.id,
},
{
populate: ["team"],
}
)
expect(mikroOrmSerializer<InstanceType<typeof User>>(user)).toEqual({
id: user1.id,
username: "User 1",
created_at: expect.any(Date),
updated_at: expect.any(Date),
deleted_at: null,
team: {
id: expect.any(String),
name: "Team 1",
created_at: expect.any(Date),
updated_at: expect.any(Date),
deleted_at: null,
user_id: user1.id,
},
})
})
})