From cae47d9e49e9a17d8395f7b390f6ec0a2f9b8dc2 Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Mon, 17 Mar 2025 13:23:18 +0530 Subject: [PATCH] feat: add check for uniqueness when creating links with isList=false (#11767) --- .changeset/late-shirts-begin.md | 7 ++ .../link-modules/define-link.spec.ts | 32 +++-- packages/core/modules-sdk/src/link.ts | 116 ++++++++++++++++-- .../core/utils/src/modules-sdk/define-link.ts | 13 +- .../modules/link-modules/src/services/link.ts | 3 +- 5 files changed, 145 insertions(+), 26 deletions(-) create mode 100644 .changeset/late-shirts-begin.md diff --git a/.changeset/late-shirts-begin.md b/.changeset/late-shirts-begin.md new file mode 100644 index 0000000000..702fe224ce --- /dev/null +++ b/.changeset/late-shirts-begin.md @@ -0,0 +1,7 @@ +--- +"@medusajs/link-modules": patch +"@medusajs/modules-sdk": patch +"@medusajs/utils": patch +--- + +feat: add check for uniqueness when creating links with isList=false diff --git a/integration-tests/modules/__tests__/link-modules/define-link.spec.ts b/integration-tests/modules/__tests__/link-modules/define-link.spec.ts index 82de7f5d6d..045fb18dd4 100644 --- a/integration-tests/modules/__tests__/link-modules/define-link.spec.ts +++ b/integration-tests/modules/__tests__/link-modules/define-link.spec.ts @@ -46,6 +46,7 @@ medusaIntegrationTestRunner({ entity: "Currency", primaryKey: "code", foreignKey: "currency_code", + isList: true, alias: "currency", args: { methodSuffix: "Currencies", @@ -58,6 +59,7 @@ medusaIntegrationTestRunner({ primaryKey: "id", foreignKey: "region_id", alias: "region", + isList: false, args: { methodSuffix: "Regions", }, @@ -88,9 +90,9 @@ medusaIntegrationTestRunner({ serviceName: "region", entity: "Region", fieldAlias: { - currency: { + currencies: { path: "currency_link.currency", - isList: false, + isList: true, forwardArgumentsOnPath: ["currency_link.currency"], }, }, @@ -100,7 +102,7 @@ medusaIntegrationTestRunner({ primaryKey: "region_id", foreignKey: "id", alias: "currency_link", - isList: false, + isList: true, }, }, ], @@ -145,6 +147,7 @@ medusaIntegrationTestRunner({ entity: "ProductVariant", primaryKey: "id", foreignKey: "product_variant_id", + isList: true, alias: "product_variant", args: { methodSuffix: "ProductVariants", @@ -156,6 +159,7 @@ medusaIntegrationTestRunner({ entity: "Region", primaryKey: "id", foreignKey: "region_id", + isList: false, alias: "region", args: { methodSuffix: "Regions", @@ -187,9 +191,9 @@ medusaIntegrationTestRunner({ serviceName: "region", entity: "Region", fieldAlias: { - product_variant: { + product_variants: { path: "product_variant_link.product_variant", - isList: false, + isList: true, forwardArgumentsOnPath: [ "product_variant_link.product_variant", ], @@ -201,7 +205,7 @@ medusaIntegrationTestRunner({ primaryKey: "region_id", foreignKey: "id", alias: "product_variant_link", - isList: false, + isList: true, }, }, ], @@ -249,6 +253,7 @@ medusaIntegrationTestRunner({ entity: "Currency", primaryKey: "code", foreignKey: "currency_code", + isList: true, alias: "currency", args: { methodSuffix: "Currencies", @@ -260,6 +265,7 @@ medusaIntegrationTestRunner({ entity: "Region", primaryKey: "id", foreignKey: "region_id", + isList: false, alias: "region", args: { methodSuffix: "Regions", @@ -291,9 +297,9 @@ medusaIntegrationTestRunner({ serviceName: "region", entity: "Region", fieldAlias: { - currency: { + currencies: { path: "currency_link.currency", - isList: false, + isList: true, forwardArgumentsOnPath: ["currency_link.currency"], }, }, @@ -303,7 +309,7 @@ medusaIntegrationTestRunner({ primaryKey: "region_id", foreignKey: "id", alias: "currency_link", - isList: false, + isList: true, }, }, ], @@ -347,6 +353,7 @@ medusaIntegrationTestRunner({ entity: "Currency", primaryKey: "code", foreignKey: "currency_code", + isList: true, alias: "currency", args: { methodSuffix: "Currencies", @@ -358,6 +365,7 @@ medusaIntegrationTestRunner({ entity: "Region", primaryKey: "id", foreignKey: "region_id", + isList: true, alias: "region", args: { methodSuffix: "Regions", @@ -389,9 +397,9 @@ medusaIntegrationTestRunner({ serviceName: "region", entity: "Region", fieldAlias: { - currency: { + currencies: { path: "currency_link.currency", - isList: false, + isList: true, forwardArgumentsOnPath: ["currency_link.currency"], }, }, @@ -401,7 +409,7 @@ medusaIntegrationTestRunner({ primaryKey: "region_id", foreignKey: "id", alias: "currency_link", - isList: false, + isList: true, }, }, ], diff --git a/packages/core/modules-sdk/src/link.ts b/packages/core/modules-sdk/src/link.ts index ee6f7dcc75..4165db8a37 100644 --- a/packages/core/modules-sdk/src/link.ts +++ b/packages/core/modules-sdk/src/link.ts @@ -5,7 +5,13 @@ import { ModuleJoinerRelationship, } from "@medusajs/types" -import { isObject, Modules, promiseAll, toPascalCase } from "@medusajs/utils" +import { + isObject, + MedusaError, + Modules, + promiseAll, + toPascalCase, +} from "@medusajs/utils" import { MedusaModule } from "./medusa-module" import { convertRecordsToLinkDefinition } from "./utils/convert-data-to-link-definition" import { linkingErrorMessage } from "./utils/linking-error" @@ -380,18 +386,88 @@ export class Link { const allLinks = Array.isArray(link) ? link : [link] const serviceLinks = new Map< string, - [string | string[], string, Record?][] + { + linksToCreate: [string | string[], string, Record?][] + linksToValidateForUniqueness: { + filters: { [key: string]: string }[] + services: string[] + } + } >() for (const link of allLinks) { const service = this.getLinkModuleOrThrow(link) + const relationships = service.__joinerConfig.relationships const { moduleA, moduleB, moduleBKey, primaryKeys } = this.getLinkDataConfig(link) if (!serviceLinks.has(service.__definition.key)) { - serviceLinks.set(service.__definition.key, []) + serviceLinks.set(service.__definition.key, { + /** + * Tuple of foreign key and the primary keys that must be + * persisted to the pivot table for representing the + * link + */ + linksToCreate: [], + + /** + * An array of objects to validate for uniqueness before persisting + * data to the pivot table. When a link uses "isList: false", we + * have to limit a relationship with this entity to be a one-to-one + * or one-to-many + */ + linksToValidateForUniqueness: { + filters: [], + services: [], + }, + }) } + relationships?.forEach((relationship) => { + const linksToValidateForUniqueness = serviceLinks.get( + service.__definition.key + )!.linksToValidateForUniqueness! + + linksToValidateForUniqueness.services.push(relationship.serviceName) + + /** + * When isList is set on false on the relationship, then it means + * we have a one-to-one or many-to-one relationship with the + * other side and we have limit duplicate entries from other + * entity. For example: + * + * - A brand has a many to one relationship with a product. + * - A product can have only one brand. Aka (brand.isList = false) + * - A brand can have multiple products. Aka (products.isList = true) + * + * A result of this, we have to ensure that a product_id can only appear + * once in the pivot table that is used for tracking "brand<>products" + * relationship. + */ + if (relationship.isList === false) { + const otherSide = relationships.find( + (other) => other.foreignKey !== relationship.foreignKey + ) + if (!otherSide) { + return + } + + if (moduleBKey === otherSide.foreignKey) { + linksToValidateForUniqueness.filters.push({ + [otherSide.foreignKey]: link[moduleB][moduleBKey], + }) + } else { + primaryKeys.forEach((pk) => { + if (pk === otherSide.foreignKey) { + linksToValidateForUniqueness.filters.push({ + [otherSide.foreignKey]: link[moduleA][pk], + }) + } + }) + } + } + }) + const pkValue = primaryKeys.length === 1 ? link[moduleA][primaryKeys[0]] @@ -403,15 +479,39 @@ export class Link { fields.push(link.data) } - serviceLinks.get(service.__definition.key)?.push(fields as any) + serviceLinks + .get(service.__definition.key) + ?.linksToCreate.push(fields as any) + } + + for (const [serviceName, data] of serviceLinks) { + if (data.linksToValidateForUniqueness.filters.length) { + const service = this.modulesMap.get(serviceName)! + const existingLinks = await service.list( + { + $or: data.linksToValidateForUniqueness.filters, + }, + { + take: 1, + } + ) + + if (existingLinks.length > 0) { + const serviceA = data.linksToValidateForUniqueness.services[0] + const serviceB = data.linksToValidateForUniqueness.services[1] + + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `Cannot create multiple links between '${serviceA}' and '${serviceB}'` + ) + } + } } const promises: Promise[] = [] - - for (const [serviceName, links] of serviceLinks) { + for (const [serviceName, data] of serviceLinks) { const service = this.modulesMap.get(serviceName)! - - promises.push(service.create(links)) + promises.push(service.create(data.linksToCreate)) } return (await promiseAll(promises)).flat() diff --git a/packages/core/utils/src/modules-sdk/define-link.ts b/packages/core/utils/src/modules-sdk/define-link.ts index 6ecc1c432d..8aadc0b4f8 100644 --- a/packages/core/utils/src/modules-sdk/define-link.ts +++ b/packages/core/utils/src/modules-sdk/define-link.ts @@ -125,7 +125,8 @@ function buildFieldAlias(fieldAliases?: Shortcut | Shortcut[]) { } function prepareServiceConfig( - input: DefineLinkInputSource | DefineReadOnlyLinkInputSource + input: DefineLinkInputSource | DefineReadOnlyLinkInputSource, + defaultOptions?: { isList?: boolean } ) { let serviceConfig = {} as ModuleLinkableKeyConfig @@ -137,7 +138,7 @@ function prepareServiceConfig( alias: source.alias ?? camelToSnakeCase(source.field ?? ""), field: input.field ?? source.field, primaryKey: source.primaryKey, - isList: false, + isList: defaultOptions?.isList ?? false, deleteCascade: false, module: source.serviceName, entity: source.entity, @@ -152,7 +153,7 @@ function prepareServiceConfig( alias: source.alias ?? camelToSnakeCase(source.field ?? ""), field: input.field ?? source.field, primaryKey: source.primaryKey, - isList: input.isList ?? false, + isList: input.isList ?? defaultOptions?.isList ?? false, deleteCascade: input.deleteCascade ?? false, module: source.serviceName, entity: source.entity, @@ -183,8 +184,8 @@ export function defineLink( rightService: DefineLinkInputSource | DefineReadOnlyLinkInputSource, linkServiceOptions?: ExtraOptions | ReadOnlyExtraOptions ): DefineLinkExport { - const serviceAObj = prepareServiceConfig(leftService) - const serviceBObj = prepareServiceConfig(rightService) + const serviceAObj = prepareServiceConfig(leftService, { isList: true }) + const serviceBObj = prepareServiceConfig(rightService, { isList: false }) if (linkServiceOptions?.readOnly) { return defineReadOnlyLink( @@ -373,6 +374,7 @@ ${serviceBObj.module}: { methodSuffix: serviceAMethodSuffix, }, deleteCascade: serviceAObj.deleteCascade, + isList: serviceAObj.isList, }, { serviceName: serviceBObj.module, @@ -384,6 +386,7 @@ ${serviceBObj.module}: { methodSuffix: serviceBMethodSuffix, }, deleteCascade: serviceBObj.deleteCascade, + isList: serviceBObj.isList, }, ], extends: [ diff --git a/packages/modules/link-modules/src/services/link.ts b/packages/modules/link-modules/src/services/link.ts index cf7f752ada..0593da6aed 100644 --- a/packages/modules/link-modules/src/services/link.ts +++ b/packages/modules/link-modules/src/services/link.ts @@ -3,11 +3,12 @@ import { InjectManager, InjectTransactionManager, MedusaContext, + MikroOrmBaseRepository, ModulesSdkUtils, } from "@medusajs/framework/utils" type InjectedDependencies = { - linkRepository: any + linkRepository: MikroOrmBaseRepository } export default class LinkService {