From 77d72c5791389b1e9172b4f3a03e406520f6b8a9 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Fri, 24 May 2024 16:20:54 +0200 Subject: [PATCH] fix(customer): Unique constraint on customer email (#7439) **What** Prevent creating multiple customers with the same email --- .../src/dal/mikro-orm/db-error-mapper.ts | 15 +++- .../__tests__/mikro-orm-repository.spec.ts | 2 +- .../services/customer-module/index.spec.ts | 87 +++++++++++++++++++ .../modules/customer/mikro-orm.config.dev.ts | 4 + packages/modules/customer/package.json | 10 +-- .../migrations/.snapshot-medusa-customer.json | 16 +++- .../src/migrations/Migration20240524123112.ts | 21 +++++ .../modules/customer/src/models/address.ts | 40 +++++---- .../modules/customer/src/models/customer.ts | 17 +++- .../customer/src/repositories/index.ts | 1 - .../integration-tests/__tests__/index.spec.ts | 8 +- 11 files changed, 186 insertions(+), 35 deletions(-) create mode 100644 packages/modules/customer/src/migrations/Migration20240524123112.ts delete mode 100644 packages/modules/customer/src/repositories/index.ts diff --git a/packages/core/utils/src/dal/mikro-orm/db-error-mapper.ts b/packages/core/utils/src/dal/mikro-orm/db-error-mapper.ts index 7d2ff301fe..5d44ec62c5 100644 --- a/packages/core/utils/src/dal/mikro-orm/db-error-mapper.ts +++ b/packages/core/utils/src/dal/mikro-orm/db-error-mapper.ts @@ -7,6 +7,17 @@ import { } from "@mikro-orm/core" import { MedusaError, upperCaseFirst } from "../../common" +function parseValue(value: string) { + switch (value) { + case "t": + return "true" + case "f": + return "false" + default: + return value + } +} + export const dbErrorMapper = (err: Error) => { if (err instanceof NotFoundError) { throw new MedusaError(MedusaError.Types.NOT_FOUND, err.message) @@ -24,8 +35,8 @@ export const dbErrorMapper = (err: Error) => { throw new MedusaError( MedusaError.Types.INVALID_DATA, `${upperCaseFirst(info.table.split("_").join(" "))} with ${info.keys - .map((key, i) => `${key}: ${info.values[i]}`) - .join(", ")} already exists.` + .map((key, i) => `${key}: ${parseValue(info.values[i])}`) + .join(", ")}, already exists.` ) } diff --git a/packages/core/utils/src/dal/mikro-orm/integration-tests/__tests__/mikro-orm-repository.spec.ts b/packages/core/utils/src/dal/mikro-orm/integration-tests/__tests__/mikro-orm-repository.spec.ts index eabd38e74f..20ff7e3d0b 100644 --- a/packages/core/utils/src/dal/mikro-orm/integration-tests/__tests__/mikro-orm-repository.spec.ts +++ b/packages/core/utils/src/dal/mikro-orm/integration-tests/__tests__/mikro-orm-repository.spec.ts @@ -864,7 +864,7 @@ describe("mikroOrmRepository", () => { .upsertWithReplace([entity3]) .catch((e) => e.message) - expect(err).toEqual("Entity3 with title: en3 already exists.") + expect(err).toEqual("Entity3 with title: en3, already exists.") }) it("should map NotNullConstraintViolationException MedusaError on upsertWithReplace", async () => { diff --git a/packages/modules/customer/integration-tests/__tests__/services/customer-module/index.spec.ts b/packages/modules/customer/integration-tests/__tests__/services/customer-module/index.spec.ts index 51046168ff..7201cce550 100644 --- a/packages/modules/customer/integration-tests/__tests__/services/customer-module/index.spec.ts +++ b/packages/modules/customer/integration-tests/__tests__/services/customer-module/index.spec.ts @@ -38,6 +38,93 @@ moduleIntegrationTestRunner({ ) }) + it("should create two customers with the same email but one has an account", async () => { + const customerData = { + company_name: "Acme Corp", + first_name: "John", + last_name: "Doe", + email: "john.doe@acmecorp.com", + phone: "123456789", + created_by: "admin", + metadata: { membership: "gold" }, + } + const customerData2 = { + ...customerData, + has_account: true, + } + const [customer, customer2] = await service.create([ + customerData, + customerData2, + ]) + + expect(customer).toEqual( + expect.objectContaining({ + id: expect.any(String), + company_name: "Acme Corp", + first_name: "John", + last_name: "Doe", + email: "john.doe@acmecorp.com", + phone: "123456789", + created_by: "admin", + metadata: expect.objectContaining({ membership: "gold" }), + }) + ) + expect(customer2).toEqual( + expect.objectContaining({ + id: expect.any(String), + company_name: "Acme Corp", + first_name: "John", + last_name: "Doe", + email: "john.doe@acmecorp.com", + phone: "123456789", + created_by: "admin", + metadata: expect.objectContaining({ membership: "gold" }), + has_account: true, + }) + ) + }) + + it("should fail to create a duplicated guest customers", async () => { + const customerData = { + company_name: "Acme Corp", + first_name: "John", + last_name: "Doe", + email: "john.doe@acmecorp.com", + phone: "123456789", + created_by: "admin", + metadata: { membership: "gold" }, + } + + const err = await service + .create([customerData, customerData]) + .catch((err) => err) + + expect(err.message).toBe( + "Customer with email: john.doe@acmecorp.com, has_account: false, already exists." + ) + }) + + it("should fail to create a duplicated customers", async () => { + const customerData = { + company_name: "Acme Corp", + first_name: "John", + last_name: "Doe", + email: "john.doe@acmecorp.com", + phone: "123456789", + created_by: "admin", + metadata: { membership: "gold" }, + has_account: true, + } + + const err = await service + .create([customerData, customerData]) + .catch((err) => err) + + expect(err.message).toBe( + "Customer with email: john.doe@acmecorp.com, has_account: true, already exists." + ) + }) + it("should create address", async () => { const customerData = { company_name: "Acme Corp", diff --git a/packages/modules/customer/mikro-orm.config.dev.ts b/packages/modules/customer/mikro-orm.config.dev.ts index 6761df13e8..20fc41e5be 100644 --- a/packages/modules/customer/mikro-orm.config.dev.ts +++ b/packages/modules/customer/mikro-orm.config.dev.ts @@ -1,8 +1,12 @@ import * as entities from "./src/models" +import { TSMigrationGenerator } from "@medusajs/utils" module.exports = { entities: Object.values(entities), schema: "public", clientUrl: "postgres://postgres@localhost/medusa-customer", type: "postgresql", + migrations: { + generator: TSMigrationGenerator, + }, } diff --git a/packages/modules/customer/package.json b/packages/modules/customer/package.json index 96c802879d..c0a2f432aa 100644 --- a/packages/modules/customer/package.json +++ b/packages/modules/customer/package.json @@ -30,11 +30,11 @@ "build": "rimraf dist && tsc --build && tsc-alias -p tsconfig.json", "test": "jest --runInBand --bail --passWithNoTests --forceExit -- src", "test:integration": "jest --forceExit -- integration-tests/**/__tests__/**/*.ts", - "migration:generate": " MIKRO_ORM_CLI=./mikro-orm.config.dev.ts mikro-orm migration:generate", - "migration:initial": " MIKRO_ORM_CLI=./mikro-orm.config.dev.ts mikro-orm migration:create --initial", - "migration:create": " MIKRO_ORM_CLI=./mikro-orm.config.dev.ts mikro-orm migration:create", - "migration:up": " MIKRO_ORM_CLI=./mikro-orm.config.dev.ts mikro-orm migration:up", - "orm:cache:clear": " MIKRO_ORM_CLI=./mikro-orm.config.dev.ts mikro-orm cache:clear" + "migration:generate": "MIKRO_ORM_CLI=./mikro-orm.config.dev.ts mikro-orm migration:generate", + "migration:initial": "MIKRO_ORM_CLI=./mikro-orm.config.dev.ts mikro-orm migration:create --initial", + "migration:create": "MIKRO_ORM_CLI=./mikro-orm.config.dev.ts mikro-orm migration:create", + "migration:up": "MIKRO_ORM_CLI=./mikro-orm.config.dev.ts mikro-orm migration:up", + "orm:cache:clear": "MIKRO_ORM_CLI=./mikro-orm.config.dev.ts mikro-orm cache:clear" }, "devDependencies": { "@medusajs/types": "workspace:^", diff --git a/packages/modules/customer/src/migrations/.snapshot-medusa-customer.json b/packages/modules/customer/src/migrations/.snapshot-medusa-customer.json index 7274b8fa9c..0d05e9c4bd 100644 --- a/packages/modules/customer/src/migrations/.snapshot-medusa-customer.json +++ b/packages/modules/customer/src/migrations/.snapshot-medusa-customer.json @@ -124,6 +124,14 @@ "name": "customer", "schema": "public", "indexes": [ + { + "keyName": "IDX_customer_email_has_account_unique", + "columnNames": [], + "composite": false, + "primary": false, + "unique": false, + "expression": "CREATE UNIQUE INDEX IF NOT EXISTS \"IDX_customer_email_has_account_unique\" ON \"customer\" (email, has_account) WHERE deleted_at IS NULL" + }, { "keyName": "customer_pkey", "columnNames": [ @@ -321,20 +329,20 @@ "unique": false }, { - "keyName": "IDX_customer_address_unqiue_customer_billing", + "keyName": "IDX_customer_address_unique_customer_billing", "columnNames": [], "composite": false, "primary": false, "unique": false, - "expression": "create unique index \"IDX_customer_address_unqiue_customer_billing\" on \"customer_address\" (\"customer_id\") where \"is_default_billing\" = true" + "expression": "CREATE UNIQUE INDEX IF NOT EXISTS \"IDX_customer_address_unique_customer_billing\" ON \"customer_address\" (customer_id) WHERE \"is_default_billing\" = true" }, { - "keyName": "IDX_customer_address_unqiue_customer_shipping", + "keyName": "IDX_customer_address_unique_customer_shipping", "columnNames": [], "composite": false, "primary": false, "unique": false, - "expression": "create unique index \"IDX_customer_address_unique_customer_shipping\" on \"customer_address\" (\"customer_id\") where \"is_default_shipping\" = true" + "expression": "CREATE UNIQUE INDEX IF NOT EXISTS \"IDX_customer_address_unique_customer_shipping\" ON \"customer_address\" (customer_id) WHERE \"is_default_shipping\" = true" }, { "keyName": "customer_address_pkey", diff --git a/packages/modules/customer/src/migrations/Migration20240524123112.ts b/packages/modules/customer/src/migrations/Migration20240524123112.ts new file mode 100644 index 0000000000..199ba6c529 --- /dev/null +++ b/packages/modules/customer/src/migrations/Migration20240524123112.ts @@ -0,0 +1,21 @@ +import { Migration } from '@mikro-orm/migrations'; + +export class Migration20240524123112 extends Migration { + async up(): Promise { + this.addSql('CREATE UNIQUE INDEX IF NOT EXISTS "IDX_customer_email_has_account_unique" ON "customer" (email, has_account) WHERE deleted_at IS NULL;'); + + this.addSql('drop index if exists "IDX_customer_address_unqiue_customer_billing";'); + this.addSql('drop index if exists "IDX_customer_address_unqiue_customer_shipping";'); + this.addSql('CREATE UNIQUE INDEX IF NOT EXISTS "IDX_customer_address_unique_customer_billing" ON "customer_address" (customer_id) WHERE "is_default_billing" = true;'); + this.addSql('CREATE UNIQUE INDEX IF NOT EXISTS "IDX_customer_address_unique_customer_shipping" ON "customer_address" (customer_id) WHERE "is_default_shipping" = true;'); + } + + async down(): Promise { + this.addSql('drop index if exists "IDX_customer_email_has_account_unique";'); + + this.addSql('drop index if exists "IDX_customer_address_unique_customer_billing";'); + this.addSql('drop index if exists "IDX_customer_address_unique_customer_shipping";'); + this.addSql('create unique index if not exists "IDX_customer_address_unqiue_customer_billing" on "customer_address" ("customer_id") where "is_default_billing" = true;'); + this.addSql('create unique index if not exists "IDX_customer_address_unique_customer_shipping" on "customer_address" ("customer_id") where "is_default_shipping" = true;'); + } +} diff --git a/packages/modules/customer/src/models/address.ts b/packages/modules/customer/src/models/address.ts index fdfb3acac1..89d8e2dd86 100644 --- a/packages/modules/customer/src/models/address.ts +++ b/packages/modules/customer/src/models/address.ts @@ -1,10 +1,13 @@ import { DAL } from "@medusajs/types" -import { Searchable, generateEntityId } from "@medusajs/utils" +import { + createPsqlIndexStatementHelper, + generateEntityId, + Searchable, +} from "@medusajs/utils" import { BeforeCreate, Cascade, Entity, - Index, ManyToOne, OnInit, OptionalProps, @@ -15,22 +18,27 @@ import Customer from "./customer" type OptionalAddressProps = DAL.EntityDateColumns // TODO: To be revisited when more clear -export const UNIQUE_CUSTOMER_SHIPPING_ADDRESS = - "IDX_customer_address_unique_customer_shipping" -export const UNIQUE_CUSTOMER_BILLING_ADDRESS = - "IDX_customer_address_unique_customer_billing" +const CustomerAddressUniqueCustomerShippingAddress = + createPsqlIndexStatementHelper({ + name: "IDX_customer_address_unique_customer_shipping", + tableName: "customer_address", + columns: "customer_id", + unique: true, + where: '"is_default_shipping" = true', + }) + +const CustomerAddressUniqueCustomerBillingAddress = + createPsqlIndexStatementHelper({ + name: "IDX_customer_address_unique_customer_billing", + tableName: "customer_address", + columns: "customer_id", + unique: true, + where: '"is_default_billing" = true', + }) @Entity({ tableName: "customer_address" }) -@Index({ - name: UNIQUE_CUSTOMER_SHIPPING_ADDRESS, - expression: - 'create unique index "IDX_customer_address_unique_customer_shipping" on "customer_address" ("customer_id") where "is_default_shipping" = true', -}) -@Index({ - name: UNIQUE_CUSTOMER_BILLING_ADDRESS, - expression: - 'create unique index "IDX_customer_address_unique_customer_billing" on "customer_address" ("customer_id") where "is_default_billing" = true', -}) +@CustomerAddressUniqueCustomerShippingAddress.MikroORMIndex() +@CustomerAddressUniqueCustomerBillingAddress.MikroORMIndex() export default class Address { [OptionalProps]: OptionalAddressProps diff --git a/packages/modules/customer/src/models/customer.ts b/packages/modules/customer/src/models/customer.ts index 06e64b77fd..d222e5b58f 100644 --- a/packages/modules/customer/src/models/customer.ts +++ b/packages/modules/customer/src/models/customer.ts @@ -1,5 +1,10 @@ import { DAL } from "@medusajs/types" -import { DALUtils, Searchable, generateEntityId } from "@medusajs/utils" +import { + createPsqlIndexStatementHelper, + DALUtils, + generateEntityId, + Searchable, +} from "@medusajs/utils" import { BeforeCreate, Cascade, @@ -7,8 +12,8 @@ import { Entity, Filter, ManyToMany, - OnInit, OneToMany, + OnInit, OptionalProps, PrimaryKey, Property, @@ -22,8 +27,16 @@ type OptionalCustomerProps = | "addresses" | DAL.SoftDeletableEntityDateColumns +const CustomerUniqueEmail = createPsqlIndexStatementHelper({ + tableName: "customer", + columns: ["email", "has_account"], + unique: true, + where: "deleted_at IS NULL", +}) + @Entity({ tableName: "customer" }) @Filter(DALUtils.mikroOrmSoftDeletableFilterOptions) +@CustomerUniqueEmail.MikroORMIndex() export default class Customer { [OptionalProps]?: OptionalCustomerProps diff --git a/packages/modules/customer/src/repositories/index.ts b/packages/modules/customer/src/repositories/index.ts deleted file mode 100644 index 147c9cc259..0000000000 --- a/packages/modules/customer/src/repositories/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { MikroOrmBaseRepository as BaseRepository } from "@medusajs/utils" diff --git a/packages/modules/tax/integration-tests/__tests__/index.spec.ts b/packages/modules/tax/integration-tests/__tests__/index.spec.ts index 40e6cc2347..c52d85b274 100644 --- a/packages/modules/tax/integration-tests/__tests__/index.spec.ts +++ b/packages/modules/tax/integration-tests/__tests__/index.spec.ts @@ -711,7 +711,7 @@ moduleIntegrationTestRunner({ ], }) ).rejects.toThrowError( - /Tax rate rule with tax_rate_id: .*?, reference_id: product_id_1 already exists./ + /Tax rate rule with tax_rate_id: .*?, reference_id: product_id_1, already exists./ ) const rate = await service.create({ @@ -729,7 +729,7 @@ moduleIntegrationTestRunner({ reference_id: "product_id_1", }) ).rejects.toThrowError( - /Tax rate rule with tax_rate_id: .*?, reference_id: product_id_1 already exists./ + /Tax rate rule with tax_rate_id: .*?, reference_id: product_id_1, already exists./ ) }) @@ -764,7 +764,7 @@ moduleIntegrationTestRunner({ province_code: "QC", }) ).rejects.toThrowError( - "Tax region with country_code: ca, province_code: qc already exists." + "Tax region with country_code: ca, province_code: qc, already exists." ) }) @@ -797,7 +797,7 @@ moduleIntegrationTestRunner({ is_default: true, }) ).rejects.toThrowError( - /Tax rate with tax_region_id: .*? already exists./ + /Tax rate with tax_region_id: .*?, already exists./ ) })