From 908b1dc3a23ed8dd3cae3fd7616931f06e2bdba2 Mon Sep 17 00:00:00 2001 From: Sebastian Rindom Date: Mon, 4 Mar 2024 18:02:11 +0100 Subject: [PATCH] fix(tax): improve error handling (#6563) --- .../integration-tests/__tests__/index.spec.ts | 318 ++++-------------- .../utils/setup-tax-structure.ts | 242 +++++++++++++ packages/tax/src/models/tax-rate-rule.ts | 3 +- packages/tax/src/models/tax-rate.ts | 2 +- packages/tax/src/models/tax-region.ts | 9 +- .../tax/src/services/tax-module-service.ts | 106 +++++- 6 files changed, 411 insertions(+), 269 deletions(-) create mode 100644 packages/tax/integration-tests/utils/setup-tax-structure.ts diff --git a/packages/tax/integration-tests/__tests__/index.spec.ts b/packages/tax/integration-tests/__tests__/index.spec.ts index e6f1704fc3..197296d4a0 100644 --- a/packages/tax/integration-tests/__tests__/index.spec.ts +++ b/packages/tax/integration-tests/__tests__/index.spec.ts @@ -1,6 +1,7 @@ import { moduleIntegrationTestRunner, SuiteOptions } from "medusa-test-utils" import { ITaxModuleService } from "@medusajs/types" import { Modules } from "@medusajs/modules-sdk" +import { setupTaxStructure } from "../utils/setup-tax-structure" jest.setTimeout(30000) @@ -709,7 +710,27 @@ moduleIntegrationTestRunner({ { reference: "product", reference_id: "product_id_1" }, ], }) - ).rejects.toThrowError() + ).rejects.toThrowError( + /You are trying to create a Tax Rate Rule for a reference that already exists. Tax Rate id: .*?, reference id: product_id_1./ + ) + + const rate = await service.create({ + tax_region_id: region.id, + value: 10, + code: "test", + name: "test", + rules: [{ reference: "product", reference_id: "product_id_1" }], + }) + + await expect( + service.createTaxRateRules({ + tax_rate_id: rate.id, + reference: "product", + reference_id: "product_id_1", + }) + ).rejects.toThrowError( + /You are trying to create a Tax Rate Rule for a reference that already exists. Tax Rate id: .*?, reference id: product_id_1./ + ) }) it("should fail to create province region belonging to a parent with non-matching country", async () => { @@ -722,7 +743,29 @@ moduleIntegrationTestRunner({ parent_id: caRegion.id, province_code: "QC", }) - ).rejects.toThrowError() + ).rejects.toThrowError( + `Province region must belong to a parent region with the same country code. You are trying to create a province region with (country: us, province: qc) but parent expects (country: ca)` + ) + }) + + it("should fail duplicate regions", async () => { + await service.createTaxRegions({ + country_code: "CA", + }) + + await service.createTaxRegions({ + country_code: "CA", + province_code: "QC", + }) + + await expect( + service.createTaxRegions({ + country_code: "CA", + province_code: "QC", + }) + ).rejects.toThrowError( + "You are trying to create a Tax Region for (country_code: ca, province_code: qc) but one already exists." + ) }) it("should fail to create region with non-existing parent", async () => { @@ -732,7 +775,30 @@ moduleIntegrationTestRunner({ country_code: "CA", province_code: "QC", }) - ).rejects.toThrowError() + ).rejects.toThrowError( + `Province region must belong to a parent region. You are trying to create a province region with (country: ca, province: qc) but parent does not exist` + ) + }) + + it("should fail to create two default tax rates", async () => { + const rate = await service.createTaxRegions({ + country_code: "CA", + default_tax_rate: { + name: "Test Rate", + rate: 0.2, + }, + }) + + await expect( + service.create({ + tax_region_id: rate.id, + name: "Shipping Rate", + rate: 8.23, + is_default: true, + }) + ).rejects.toThrowError( + /You are trying to create a default tax rate for region: .*? which already has a default tax rate. Unset the current default rate and try again./ + ) }) it("should delete all child regions when parent region is deleted", async () => { @@ -754,7 +820,7 @@ moduleIntegrationTestRunner({ expect(taxRegions).toEqual([]) }) - it("it should soft delete all child regions when parent region is deleted", async () => { + it("should soft delete all child regions when parent region is deleted", async () => { const region = await service.createTaxRegions({ country_code: "CA", }) @@ -767,9 +833,7 @@ moduleIntegrationTestRunner({ await service.softDeleteTaxRegions([region.id]) const taxRegions = await service.listTaxRegions( - { - id: provinceRegion.id, - }, + { id: provinceRegion.id }, { withDeleted: true } ) @@ -783,243 +847,3 @@ moduleIntegrationTestRunner({ }) }, }) - -const setupTaxStructure = async (service: ITaxModuleService) => { - // Setup for this specific test - // - // Using the following structure to setup tests. - // US - default 2% - // - Region: CA - default 5% - // - Override: Reduced rate (for 3 product ids): 3% - // - Override: Reduced rate (for product type): 1% - // - Region: NY - default: 6% - // - Region: FL - default: 4% - // - // Denmark - default 25% - // - // Germany - default 19% - // - Override: Reduced Rate (for product type) - 7% - // - // Canada - default 5% - // - Override: Reduced rate (for product id) - 3% - // - Override: Reduced rate (for product type) - 3.5% - // - Region: QC - default 2% - // - Override: Reduced rate (for same product type as country reduced rate): 1% - // - Region: BC - default 2% - // - const [us, dk, de, ca] = await service.createTaxRegions([ - { - country_code: "US", - default_tax_rate: { name: "US Default Rate", rate: 2 }, - }, - { - country_code: "DK", - default_tax_rate: { name: "Denmark Default Rate", rate: 25 }, - }, - { - country_code: "DE", - default_tax_rate: { - code: "DE19", - name: "Germany Default Rate", - rate: 19, - }, - }, - { - country_code: "CA", - default_tax_rate: { name: "Canada Default Rate", rate: 5 }, - }, - ]) - - // Create province regions within the US - const [cal, ny, fl, qc, bc] = await service.createTaxRegions([ - { - country_code: "US", - province_code: "CA", - parent_id: us.id, - default_tax_rate: { - rate: 5, - name: "CA Default Rate", - code: "CADEFAULT", - }, - }, - { - country_code: "US", - province_code: "NY", - parent_id: us.id, - default_tax_rate: { - rate: 6, - name: "NY Default Rate", - code: "NYDEFAULT", - }, - }, - { - country_code: "US", - province_code: "FL", - parent_id: us.id, - default_tax_rate: { - rate: 4, - name: "FL Default Rate", - code: "FLDEFAULT", - }, - }, - { - country_code: "CA", - province_code: "QC", - parent_id: ca.id, - default_tax_rate: { - rate: 2, - name: "QC Default Rate", - code: "QCDEFAULT", - }, - }, - { - country_code: "CA", - province_code: "BC", - parent_id: ca.id, - default_tax_rate: { - rate: 2, - name: "BC Default Rate", - code: "BCDEFAULT", - }, - }, - ]) - - const [calProd, calType, deType, canProd, canType, qcType] = - await service.create([ - { - tax_region_id: cal.id, - name: "CA Reduced Rate for Products", - rate: 3, - code: "CAREDUCE_PROD", - }, - { - tax_region_id: cal.id, - name: "CA Reduced Rate for Product Type", - rate: 1, - code: "CAREDUCE_TYPE", - }, - { - tax_region_id: de.id, - name: "Germany Reduced Rate for Product Type", - rate: 7, - code: "DEREDUCE_TYPE", - }, - { - tax_region_id: ca.id, - name: "Canada Reduced Rate for Product", - rate: 3, - code: "CAREDUCE_PROD_CA", - }, - { - tax_region_id: ca.id, - name: "Canada Reduced Rate for Product Type", - rate: 3.5, - code: "CAREDUCE_TYPE_CA", - }, - { - tax_region_id: qc.id, - name: "QC Reduced Rate for Product Type", - rate: 1, - code: "QCREDUCE_TYPE", - }, - ]) - - // Create tax rate rules for specific products and product types - await service.createTaxRateRules([ - { - reference: "product", - reference_id: "product_id_1", - tax_rate_id: calProd.id, - }, - { - reference: "product", - reference_id: "product_id_2", - tax_rate_id: calProd.id, - }, - { - reference: "product", - reference_id: "product_id_3", - tax_rate_id: calProd.id, - }, - { - reference: "product_type", - reference_id: "product_type_id_1", - tax_rate_id: calType.id, - }, - { - reference: "product_type", - reference_id: "product_type_id_2", - tax_rate_id: deType.id, - }, - { - reference: "product", - reference_id: "product_id_4", - tax_rate_id: canProd.id, - }, - { - reference: "product_type", - reference_id: "product_type_id_3", - tax_rate_id: canType.id, - }, - { - reference: "product_type", - reference_id: "product_type_id_3", - tax_rate_id: qcType.id, - }, - ]) - - return { - us: { - country: us, - children: { - cal: { - province: cal, - overrides: { - calProd, - calType, - }, - }, - ny: { - province: ny, - overrides: {}, - }, - fl: { - province: fl, - overrides: {}, - }, - }, - overrides: {}, - }, - dk: { - country: dk, - children: {}, - overrides: {}, - }, - de: { - country: de, - children: {}, - overrides: { - deType, - }, - }, - ca: { - country: ca, - children: { - qc: { - province: qc, - overrides: { - qcType, - }, - }, - bc: { - province: bc, - overrides: {}, - }, - }, - overrides: { - canProd, - canType, - }, - }, - } -} diff --git a/packages/tax/integration-tests/utils/setup-tax-structure.ts b/packages/tax/integration-tests/utils/setup-tax-structure.ts new file mode 100644 index 0000000000..52f16860db --- /dev/null +++ b/packages/tax/integration-tests/utils/setup-tax-structure.ts @@ -0,0 +1,242 @@ +import { ITaxModuleService } from "@medusajs/types" + +/** + * Setup for this specific test + * + * Using the following structure to setup tests. + * US - default 2% + * - Region: CA - default 5% + * - Override: Reduced rate (for 3 product ids): 3% + * - Override: Reduced rate (for product type): 1% + * - Region: NY - default: 6% + * - Region: FL - default: 4% + * + * Denmark - default 25% + * + * Germany - default 19% + * - Override: Reduced Rate (for product type) - 7% + * + * Canada - default 5% + * - Override: Reduced rate (for product id) - 3% + * - Override: Reduced rate (for product type) - 3.5% + * - Region: QC - default 2% + * - Override: Reduced rate (for same product type as country reduced rate): 1% + * - Region: BC - default 2% + */ +export const setupTaxStructure = async (service: ITaxModuleService) => { + const [us, dk, de, ca] = await service.createTaxRegions([ + { + country_code: "US", + default_tax_rate: { name: "US Default Rate", rate: 2 }, + }, + { + country_code: "DK", + default_tax_rate: { name: "Denmark Default Rate", rate: 25 }, + }, + { + country_code: "DE", + default_tax_rate: { + code: "DE19", + name: "Germany Default Rate", + rate: 19, + }, + }, + { + country_code: "CA", + default_tax_rate: { name: "Canada Default Rate", rate: 5 }, + }, + ]) + + // Create province regions within the US + const [cal, ny, fl, qc, bc] = await service.createTaxRegions([ + { + country_code: "US", + province_code: "CA", + parent_id: us.id, + default_tax_rate: { + rate: 5, + name: "CA Default Rate", + code: "CADEFAULT", + }, + }, + { + country_code: "US", + province_code: "NY", + parent_id: us.id, + default_tax_rate: { + rate: 6, + name: "NY Default Rate", + code: "NYDEFAULT", + }, + }, + { + country_code: "US", + province_code: "FL", + parent_id: us.id, + default_tax_rate: { + rate: 4, + name: "FL Default Rate", + code: "FLDEFAULT", + }, + }, + { + country_code: "CA", + province_code: "QC", + parent_id: ca.id, + default_tax_rate: { + rate: 2, + name: "QC Default Rate", + code: "QCDEFAULT", + }, + }, + { + country_code: "CA", + province_code: "BC", + parent_id: ca.id, + default_tax_rate: { + rate: 2, + name: "BC Default Rate", + code: "BCDEFAULT", + }, + }, + ]) + + const [calProd, calType, deType, canProd, canType, qcType] = + await service.create([ + { + tax_region_id: cal.id, + name: "CA Reduced Rate for Products", + rate: 3, + code: "CAREDUCE_PROD", + }, + { + tax_region_id: cal.id, + name: "CA Reduced Rate for Product Type", + rate: 1, + code: "CAREDUCE_TYPE", + }, + { + tax_region_id: de.id, + name: "Germany Reduced Rate for Product Type", + rate: 7, + code: "DEREDUCE_TYPE", + }, + { + tax_region_id: ca.id, + name: "Canada Reduced Rate for Product", + rate: 3, + code: "CAREDUCE_PROD_CA", + }, + { + tax_region_id: ca.id, + name: "Canada Reduced Rate for Product Type", + rate: 3.5, + code: "CAREDUCE_TYPE_CA", + }, + { + tax_region_id: qc.id, + name: "QC Reduced Rate for Product Type", + rate: 1, + code: "QCREDUCE_TYPE", + }, + ]) + + // Create tax rate rules for specific products and product types + await service.createTaxRateRules([ + { + reference: "product", + reference_id: "product_id_1", + tax_rate_id: calProd.id, + }, + { + reference: "product", + reference_id: "product_id_2", + tax_rate_id: calProd.id, + }, + { + reference: "product", + reference_id: "product_id_3", + tax_rate_id: calProd.id, + }, + { + reference: "product_type", + reference_id: "product_type_id_1", + tax_rate_id: calType.id, + }, + { + reference: "product_type", + reference_id: "product_type_id_2", + tax_rate_id: deType.id, + }, + { + reference: "product", + reference_id: "product_id_4", + tax_rate_id: canProd.id, + }, + { + reference: "product_type", + reference_id: "product_type_id_3", + tax_rate_id: canType.id, + }, + { + reference: "product_type", + reference_id: "product_type_id_3", + tax_rate_id: qcType.id, + }, + ]) + + return { + us: { + country: us, + children: { + cal: { + province: cal, + overrides: { + calProd, + calType, + }, + }, + ny: { + province: ny, + overrides: {}, + }, + fl: { + province: fl, + overrides: {}, + }, + }, + overrides: {}, + }, + dk: { + country: dk, + children: {}, + overrides: {}, + }, + de: { + country: de, + children: {}, + overrides: { + deType, + }, + }, + ca: { + country: ca, + children: { + qc: { + province: qc, + overrides: { + qcType, + }, + }, + bc: { + province: bc, + overrides: {}, + }, + }, + overrides: { + canProd, + canType, + }, + }, + } +} diff --git a/packages/tax/src/models/tax-rate-rule.ts b/packages/tax/src/models/tax-rate-rule.ts index a16b08dae1..ec74a9d8ff 100644 --- a/packages/tax/src/models/tax-rate-rule.ts +++ b/packages/tax/src/models/tax-rate-rule.ts @@ -37,7 +37,8 @@ const referenceIdIndexStatement = createPsqlIndexStatementHelper({ where: "deleted_at IS NULL", }) -const uniqueRateReferenceIndexName = "IDX_tax_rate_rule_unique_rate_reference" +export const uniqueRateReferenceIndexName = + "IDX_tax_rate_rule_unique_rate_reference" const uniqueRateReferenceIndexStatement = createPsqlIndexStatementHelper({ name: uniqueRateReferenceIndexName, tableName: TABLE_NAME, diff --git a/packages/tax/src/models/tax-rate.ts b/packages/tax/src/models/tax-rate.ts index cbc853549e..6add3d5693 100644 --- a/packages/tax/src/models/tax-rate.ts +++ b/packages/tax/src/models/tax-rate.ts @@ -24,7 +24,7 @@ type OptionalTaxRateProps = DAL.SoftDeletableEntityDateColumns const TABLE_NAME = "tax_rate" -const singleDefaultRegionIndexName = "IDX_single_default_region" +export const singleDefaultRegionIndexName = "IDX_single_default_region" const singleDefaultRegionIndexStatement = createPsqlIndexStatementHelper({ name: singleDefaultRegionIndexName, tableName: TABLE_NAME, diff --git a/packages/tax/src/models/tax-region.ts b/packages/tax/src/models/tax-region.ts index 6d29f2571e..4d58a6566d 100644 --- a/packages/tax/src/models/tax-region.ts +++ b/packages/tax/src/models/tax-region.ts @@ -25,7 +25,8 @@ type OptionalTaxRegionProps = DAL.SoftDeletableEntityDateColumns const TABLE_NAME = "tax_region" -const countryCodeProvinceIndexName = "IDX_tax_region_unique_country_province" +export const countryCodeProvinceIndexName = + "IDX_tax_region_unique_country_province" const countryCodeProvinceIndexStatement = createPsqlIndexStatementHelper({ name: countryCodeProvinceIndexName, tableName: TABLE_NAME, @@ -33,8 +34,10 @@ const countryCodeProvinceIndexStatement = createPsqlIndexStatementHelper({ unique: true, }) -const taxRegionProviderTopLevelCheckName = "CK_tax_region_provider_top_level" -const taxRegionCountryTopLevelCheckName = "CK_tax_region_country_top_level" +export const taxRegionProviderTopLevelCheckName = + "CK_tax_region_provider_top_level" +export const taxRegionCountryTopLevelCheckName = + "CK_tax_region_country_top_level" @Check({ name: taxRegionProviderTopLevelCheckName, diff --git a/packages/tax/src/services/tax-module-service.ts b/packages/tax/src/services/tax-module-service.ts index caab1eac0b..ac7e24b4d2 100644 --- a/packages/tax/src/services/tax-module-service.ts +++ b/packages/tax/src/services/tax-module-service.ts @@ -22,7 +22,9 @@ import { import { TaxProvider, TaxRate, TaxRegion, TaxRateRule } from "@models" import { entityNameToLinkableKeysMap, joinerConfig } from "../joiner-config" import { TaxRegionDTO } from "@medusajs/types" -import { EntityManager } from "@mikro-orm/postgresql" +import { uniqueRateReferenceIndexName } from "../models/tax-rate-rule" +import { singleDefaultRegionIndexName } from "../models/tax-rate" +import { countryCodeProvinceIndexName } from "../models/tax-region" type InjectedDependencies = { baseRepository: DAL.RepositoryService @@ -105,7 +107,11 @@ export default class TaxModuleService< @MedusaContext() sharedContext: Context = {} ): Promise { const input = Array.isArray(data) ? data : [data] - const rates = await this.create_(input, sharedContext) + const rates = await this.create_(input, sharedContext).catch((err) => { + this.handleCreateError(err) + this.handleCreateRulesError(err) + throw err + }) return Array.isArray(data) ? rates : rates[0] } @@ -177,7 +183,13 @@ export default class TaxModuleService< data: TaxTypes.UpdateTaxRateDTO, @MedusaContext() sharedContext: Context = {} ): Promise { - const rates = await this.update_(selector, data, sharedContext) + const rates = await this.update_(selector, data, sharedContext).catch( + (err) => { + this.handleCreateError(err) + this.handleCreateRulesError(err) + throw err + } + ) const serialized = await this.baseRepository_.serialize< TaxTypes.TaxRateDTO[] >(rates, { populate: true }) @@ -309,6 +321,20 @@ export default class TaxModuleService< async createTaxRegions( data: TaxTypes.CreateTaxRegionDTO | TaxTypes.CreateTaxRegionDTO[], @MedusaContext() sharedContext: Context = {} + ) { + const input = Array.isArray(data) ? data : [data] + const result = await this.createTaxRegions_(input, sharedContext).catch( + (err) => { + this.handleCreateRegionsError(err) + throw err + } + ) + return Array.isArray(data) ? result : result[0] + } + + async createTaxRegions_( + data: TaxTypes.CreateTaxRegionDTO[], + sharedContext: Context = {} ) { const { defaultRates, regionData } = this.prepareTaxRegionInputForCreate(data) @@ -336,13 +362,10 @@ export default class TaxModuleService< await this.create(rates, sharedContext) } - const result = await this.baseRepository_.serialize< - TaxTypes.TaxRegionDTO[] - >(regions, { - populate: true, - }) - - return Array.isArray(data) ? result : result[0] + return await this.baseRepository_.serialize( + regions, + { populate: true } + ) } createTaxRateRules( @@ -360,12 +383,12 @@ export default class TaxModuleService< @MedusaContext() sharedContext: Context = {} ) { const input = Array.isArray(data) ? data : [data] - const rules = await this.taxRateRuleService_.create(input, sharedContext) - const result = await this.baseRepository_.serialize< - TaxTypes.TaxRateRuleDTO[] - >(rules, { - populate: true, - }) + const result = await this.createTaxRateRules_(input, sharedContext).catch( + (err) => { + this.handleCreateRulesError(err) + throw err + } + ) return Array.isArray(data) ? result : result[0] } @@ -374,7 +397,13 @@ export default class TaxModuleService< data: TaxTypes.CreateTaxRateRuleDTO[], @MedusaContext() sharedContext: Context = {} ) { - return await this.taxRateRuleService_.create(data, sharedContext) + const rules = await this.taxRateRuleService_.create(data, sharedContext) + return await this.baseRepository_.serialize( + rules, + { + populate: true, + } + ) } @InjectManager("baseRepository_") @@ -720,6 +749,49 @@ export default class TaxModuleService< return code.toLowerCase() } + private handleCreateRegionsError(err: any) { + if (err.constraint === countryCodeProvinceIndexName) { + const [countryCode, provinceCode] = err.detail + .split("=")[1] + .match(/\(([^)]+)\)/)[1] + .split(",") + + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `You are trying to create a Tax Region for (country_code: ${countryCode.trim()}, province_code: ${provinceCode.trim()}) but one already exists.` + ) + } + } + + private handleCreateError(err: any) { + if (err.constraint === singleDefaultRegionIndexName) { + // err.detail = Key (tax_region_id)=(txreg_01HQX5E8GEH36ZHJWFYDAFY67P) already exists. + const regionId = err.detail.split("=")[1].match(/\(([^)]+)\)/)[1] + + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `You are trying to create a default tax rate for region: ${regionId} which already has a default tax rate. Unset the current default rate and try again.` + ) + } + } + + private handleCreateRulesError(err: any) { + if (err.constraint === uniqueRateReferenceIndexName) { + // err.detail == "Key (tax_rate_id, reference_id)=(txr_01HQWRXTC0JK0F02D977WRR45T, product_id_1) already exists." + // We want to extract the ids from the detail string + // i.e. txr_01HQWRXTC0JK0F02D977WRR45T and product_id_1 + const [taxRateId, referenceId] = err.detail + .split("=")[1] + .match(/\(([^)]+)\)/)[1] + .split(",") + + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `You are trying to create a Tax Rate Rule for a reference that already exists. Tax Rate id: ${taxRateId.trim()}, reference id: ${referenceId.trim()}.` + ) + } + } + // @InjectTransactionManager("baseRepository_") // async createProvidersOnLoad(@MedusaContext() sharedContext: Context = {}) { // const providersToLoad = this.container_["tax_providers"] as ITaxProvider[]