diff --git a/integration-tests/plugins/__tests__/regions/admin/regions.spec.ts b/integration-tests/plugins/__tests__/regions/admin/regions.spec.ts index f55d6f17ec..33862a1a22 100644 --- a/integration-tests/plugins/__tests__/regions/admin/regions.spec.ts +++ b/integration-tests/plugins/__tests__/regions/admin/regions.spec.ts @@ -52,6 +52,8 @@ describe("Regions - Admin", () => { { name: "Test Region", currency_code: "usd", + countries: ["us", "ca"], + metadata: { foo: "bar" }, }, adminHeaders ) @@ -62,14 +64,21 @@ describe("Regions - Admin", () => { id: created.data.region.id, name: "Test Region", currency_code: "usd", + metadata: { foo: "bar" }, }) ) + expect(created.data.region.countries.map((c) => c.iso_2)).toEqual([ + "us", + "ca", + ]) const updated = await api.post( - `/admin/regions`, + `/admin/regions/${created.data.region.id}`, { name: "United States", currency_code: "usd", + countries: ["us"], + metadata: { foo: "baz" }, }, adminHeaders ) @@ -78,9 +87,12 @@ describe("Regions - Admin", () => { expect(updated.data.region).toEqual( expect.objectContaining({ id: updated.data.region.id, + name: "United States", currency_code: "usd", + metadata: { foo: "baz" }, }) ) + expect(updated.data.region.countries.map((c) => c.iso_2)).toEqual(["us"]) const deleted = await api.delete( `/admin/regions/${updated.data.region.id}`, @@ -129,7 +141,7 @@ describe("Regions - Admin", () => { expect(error.response.status).toEqual(400) expect(error.response.data.message).toEqual( - "Currency with code: foo was not found" + 'Currencies with codes: "foo" were not found' ) }) @@ -180,6 +192,8 @@ describe("Regions - Admin", () => { { name: "Test", currency_code: "usd", + countries: ["jp"], + metadata: { foo: "bar" }, }, ]) @@ -192,8 +206,12 @@ describe("Regions - Admin", () => { id: expect.any(String), name: "Test", currency_code: "usd", + metadata: { foo: "bar" }, }), ]) + expect(response.data.regions[0].countries.map((c) => c.iso_2)).toEqual([ + "jp", + ]) }) it("should get a region", async () => { @@ -201,6 +219,8 @@ describe("Regions - Admin", () => { { name: "Test", currency_code: "usd", + countries: ["jp"], + metadata: { foo: "bar" }, }, ]) @@ -213,7 +233,9 @@ describe("Regions - Admin", () => { id: region.id, name: "Test", currency_code: "usd", + metadata: { foo: "bar" }, }) ) + expect(response.data.region.countries.map((c) => c.iso_2)).toEqual(["jp"]) }) }) diff --git a/packages/core-flows/src/region/steps/update-regions.ts b/packages/core-flows/src/region/steps/update-regions.ts index 1e30f1344c..daec4ad790 100644 --- a/packages/core-flows/src/region/steps/update-regions.ts +++ b/packages/core-flows/src/region/steps/update-regions.ts @@ -48,6 +48,7 @@ export const updateRegionsStep = createStep( name: r.name, currency_code: r.currency_code, metadata: r.metadata, + countries: r.countries.map((c) => c.iso_2), })) ) } diff --git a/packages/medusa/src/api-v2/admin/regions/validators.ts b/packages/medusa/src/api-v2/admin/regions/validators.ts index 96ef914928..8fd977f5d9 100644 --- a/packages/medusa/src/api-v2/admin/regions/validators.ts +++ b/packages/medusa/src/api-v2/admin/regions/validators.ts @@ -1,6 +1,12 @@ import { OperatorMap } from "@medusajs/types" import { Type } from "class-transformer" -import { IsOptional, IsString, ValidateNested } from "class-validator" +import { + IsArray, + IsObject, + IsOptional, + IsString, + ValidateNested, +} from "class-validator" import { FindParams, extendedFindParamsMixin } from "../../../types/common" import { OperatorMapValidator } from "../../../types/validators/operator-map" @@ -76,6 +82,14 @@ export class AdminPostRegionsReq { @IsString() currency_code: string + + @IsArray() + @IsOptional() + countries?: string[] + + @IsObject() + @IsOptional() + metadata?: Record } export class AdminPostRegionsRegionReq { @@ -86,4 +100,12 @@ export class AdminPostRegionsRegionReq { @IsString() @IsOptional() currency_code?: string + + @IsArray() + @IsOptional() + countries?: string[] + + @IsObject() + @IsOptional() + metadata?: Record } diff --git a/packages/region/integration-tests/__tests__/region-module.spec.ts b/packages/region/integration-tests/__tests__/region-module.spec.ts index cf43c02bee..42c8a365cd 100644 --- a/packages/region/integration-tests/__tests__/region-module.spec.ts +++ b/packages/region/integration-tests/__tests__/region-module.spec.ts @@ -127,7 +127,7 @@ describe("Region Module Service", () => { currency_code: "USD", countries: ["neverland"], }) - ).rejects.toThrowError("Countries with codes neverland does not exist") + ).rejects.toThrowError('Countries with codes: "neverland" do not exist') }) it("should throw when country is already assigned to a region", async () => { @@ -144,7 +144,7 @@ describe("Region Module Service", () => { countries: ["us"], }) ).rejects.toThrowError( - "Country with code us is already assigned to a region" + 'Countries with codes: "us" are already assigned to a region' ) }) @@ -163,7 +163,163 @@ describe("Region Module Service", () => { }, ]) ).rejects.toThrowError( - "Country with code us is already assigned to a region" + 'Countries with codes: "us" are already assigned to a region' + ) + }) + + it("should update the region successfully", async () => { + const createdRegion = await service.create({ + name: "North America", + currency_code: "USD", + countries: ["us", "ca"], + }) + + await service.update(createdRegion.id, { + name: "Americas", + currency_code: "MXN", + countries: ["us", "mx"], + }) + + const latestRegion = await service.retrieve(createdRegion.id, { + relations: ["currency", "countries"], + }) + + expect(latestRegion).toMatchObject({ + id: createdRegion.id, + name: "Americas", + currency_code: "mxn", + }) + + expect(latestRegion.countries.map((c) => c.iso_2)).toEqual(["mx", "us"]) + }) + + it("should update the region without affecting countries if countries are undefined", async () => { + const createdRegion = await service.create({ + name: "North America", + currency_code: "USD", + countries: ["us", "ca"], + }) + + await service.update(createdRegion.id, { + name: "Americas", + currency_code: "MXN", + }) + + const updatedRegion = await service.retrieve(createdRegion.id, { + relations: ["currency", "countries"], + }) + + expect(updatedRegion).toMatchObject({ + id: createdRegion.id, + name: "Americas", + currency_code: "mxn", + }) + + expect(updatedRegion.countries.map((c) => c.iso_2)).toEqual(["ca", "us"]) + }) + + it("should remove the countries in a region successfully", async () => { + const createdRegion = await service.create({ + name: "North America", + currency_code: "USD", + countries: ["us", "ca"], + }) + + await service.update(createdRegion.id, { + name: "Americas", + currency_code: "MXN", + countries: [], + }) + + const updatedRegion = await service.retrieve(createdRegion.id, { + relations: ["currency", "countries"], + }) + + expect(updatedRegion).toMatchObject({ + id: createdRegion.id, + name: "Americas", + currency_code: "mxn", + }) + + expect(updatedRegion.countries).toHaveLength(0) + }) + + it("should fail updating the region currency to a non-existent one", async () => { + const createdRegion = await service.create({ + name: "North America", + currency_code: "USD", + countries: ["us", "ca"], + }) + + await expect( + service.update( + { id: createdRegion.id }, + { + currency_code: "DOGECOIN", + } + ) + ).rejects.toThrowError('Currencies with codes: "dogecoin" were not found') + }) + + it("should fail updating the region countries to non-existent ones", async () => { + const createdRegion = await service.create({ + name: "North America", + currency_code: "USD", + countries: ["us", "ca"], + }) + + await expect( + service.update( + { id: createdRegion.id }, + { + countries: ["us", "neverland"], + } + ) + ).rejects.toThrowError('Countries with codes: "neverland" do not exist') + }) + + it("should fail updating the region if there are duplicate countries", async () => { + const createdRegion = await service.create({ + name: "North America", + currency_code: "USD", + countries: ["us", "ca"], + }) + + await expect( + service.update( + { id: createdRegion.id }, + { + countries: ["us", "us"], + } + ) + ).rejects.toThrowError( + 'Countries with codes: "us" are already assigned to a region' + ) + }) + + it("should fail updating the region if country is already used", async () => { + const [createdRegion] = await service.create([ + { + name: "North America", + currency_code: "USD", + countries: ["us", "ca"], + }, + { + name: "Americas", + currency_code: "USD", + countries: ["mx"], + }, + ]) + + await expect( + service.update( + { id: createdRegion.id }, + { + countries: ["us", "mx"], + } + ) + ).rejects.toThrowError( + 'Countries with codes: "mx" are already assigned to a region' ) }) @@ -173,6 +329,6 @@ describe("Region Module Service", () => { name: "Europe", currency_code: "DOGECOIN", }) - ).rejects.toThrowError("Currency with code: DOGECOIN was not found") + ).rejects.toThrowError('Currencies with codes: "dogecoin" were not found') }) }) diff --git a/packages/region/src/migrations/.snapshot-medusa-region.json b/packages/region/src/migrations/.snapshot-medusa-region.json index 83539e71fc..4aeb715cba 100644 --- a/packages/region/src/migrations/.snapshot-medusa-region.json +++ b/packages/region/src/migrations/.snapshot-medusa-region.json @@ -135,7 +135,7 @@ "indexes": [ { "columnNames": [ - "currency_id" + "currency_code" ], "composite": false, "keyName": "IDX_region_currency_code", @@ -163,7 +163,7 @@ ], "checks": [], "foreignKeys": { - "region_currency_id_foreign": { + "region_currency_code_foreign": { "constraintName": "region_currency_code_foreign", "columnNames": [ "currency_code" @@ -173,7 +173,7 @@ "code" ], "referencedTableName": "public.region_currency", - "deleteRule": "cascade", + "deleteRule": "set null", "updateRule": "cascade" } } @@ -247,15 +247,6 @@ "name": "region_country", "schema": "public", "indexes": [ - { - "columnNames": [ - "region_id" - ], - "composite": false, - "keyName": "IDX_country_region_id", - "primary": false, - "unique": false - }, { "keyName": "region_country_pkey", "columnNames": [ @@ -278,10 +269,10 @@ "id" ], "referencedTableName": "public.region", - "deleteRule": "cascade", + "deleteRule": "set null", "updateRule": "cascade" } } } ] -} \ No newline at end of file +} diff --git a/packages/region/src/migrations/RegionModuleSetup20240205173216.ts b/packages/region/src/migrations/RegionModuleSetup20240205173216.ts index c60010a279..71546c8106 100644 --- a/packages/region/src/migrations/RegionModuleSetup20240205173216.ts +++ b/packages/region/src/migrations/RegionModuleSetup20240205173216.ts @@ -51,7 +51,7 @@ CREATE TABLE IF NOT EXISTS "region_country" ( "region_id" text NULL, CONSTRAINT "region_country_pkey" PRIMARY KEY ("id") ); -CREATE INDEX IF NOT EXISTS "IDX_country_region_id" ON "region_country" ("region_id"); +CREATE UNIQUE INDEX IF NOT EXISTS "IDX_region_country_region_id_iso_2_unique" ON "region_country" (region_id, iso_2); -- Adjust foreign keys for "region_country" ALTER TABLE "region_country" DROP CONSTRAINT IF EXISTS "FK_91f88052197680f9790272aaf5b"; diff --git a/packages/region/src/models/country.ts b/packages/region/src/models/country.ts index 9df8d781c6..65a565149f 100644 --- a/packages/region/src/models/country.ts +++ b/packages/region/src/models/country.ts @@ -1,15 +1,30 @@ import { BeforeCreate, Entity, + Unique, ManyToOne, OnInit, PrimaryKey, Property, + Index, } from "@mikro-orm/core" -import { generateEntityId } from "@medusajs/utils" +import { + createPsqlIndexStatementHelper, + generateEntityId, +} from "@medusajs/utils" import Region from "./region" +// We don't need a partial index on deleted_at here since we don't support soft deletes on countries +const regionIdIsoIndexName = "IDX_region_country_region_id_iso_2_unique" +const RegionIdIsoIndexStatement = createPsqlIndexStatementHelper({ + name: regionIdIsoIndexName, + tableName: "region_country", + columns: ["region_id", "iso_2"], + unique: true, +}) + +RegionIdIsoIndexStatement.MikroORMIndex() @Entity({ tableName: "region_country" }) export default class Country { @PrimaryKey({ columnType: "text" }) @@ -35,7 +50,6 @@ export default class Country { @ManyToOne({ entity: () => Region, - index: "IDX_country_region_id", nullable: true, }) region?: Region | null diff --git a/packages/region/src/services/region-module.ts b/packages/region/src/services/region-module.ts index 485565ddb9..ad779ee581 100644 --- a/packages/region/src/services/region-module.ts +++ b/packages/region/src/services/region-module.ts @@ -14,6 +14,7 @@ import { UpdateRegionDTO, } from "@medusajs/types" import { + arrayDifference, InjectManager, InjectTransactionManager, isObject, @@ -27,11 +28,7 @@ import { import { Country, Currency, Region } from "@models" import { DefaultsUtils } from "@medusajs/utils" -import { - CreateCountryDTO, - CreateCurrencyDTO, - CreateRegionDTO as InternalCreateRegionDTO, -} from "@types" +import { CreateCountryDTO, CreateCurrencyDTO } from "@types" import { entityNameToLinkableKeysMap, joinerConfig } from "../joiner-config" const COUNTRIES_LIMIT = 1000 @@ -117,78 +114,31 @@ export default class RegionModuleService< data: CreateRegionDTO[], @MedusaContext() sharedContext: Context = {} ): Promise { - let currencies = await this.currencyService_.list( - { code: data.map((d) => d.currency_code.toLowerCase()) }, - {}, - sharedContext + let normalizedInput = RegionModuleService.normalizeInput(data) + + const validations = [ + this.validateCurrencies( + normalizedInput.map((r) => r.currency_code), + sharedContext + ), + this.validateCountries( + normalizedInput.map((r) => r.countries ?? []).flat(), + sharedContext + ), + ] as const + + // Assign the full country object so the ORM updates the relationship + const [, dbCountries] = await promiseAll(validations) + const dbCountriesMap = new Map(dbCountries.map((d) => [d.iso_2, d])) + let normalizedDbRegions = normalizedInput.map((region) => + removeUndefined({ + ...region, + countries: region.countries?.map((c) => dbCountriesMap.get(c)), + }) ) - const countriesInDb = await this.countryService_.list( - { iso_2: data.map((d) => d.countries).flat() }, - { select: ["iso_2", "region_id"] }, - sharedContext - ) - - const countriesInDbMap = new Map( - countriesInDb.map((c) => [c.iso_2, c]) - ) - - const currencyMap = new Map( - currencies.map((c) => [c.code.toLowerCase(), c]) - ) - - const toCreate: InternalCreateRegionDTO[] = [] - const seenCountries = new Set() - for (const region of data) { - const reg = { ...region } as InternalCreateRegionDTO - const countriesToAdd = region.countries || [] - - if (countriesToAdd) { - const notInDb = countriesToAdd.filter( - (c) => !countriesInDbMap.has(c.toLowerCase()) - ) - - if (notInDb.length) { - throw new MedusaError( - MedusaError.Types.INVALID_DATA, - `Countries with codes ${countriesToAdd.join(", ")} does not exist` - ) - } - - const regionCountries = countriesToAdd.map((code) => { - const country = countriesInDbMap.get(code.toLowerCase()) - - if (country?.region_id || seenCountries.has(code.toLowerCase())) { - throw new MedusaError( - MedusaError.Types.INVALID_DATA, - `Country with code ${code} is already assigned to a region` - ) - } - - seenCountries.add(code.toLowerCase()) - - return country - }) as unknown as TCountry[] - - reg.countries = regionCountries - } - - const lowerCasedCurrency = region.currency_code.toLowerCase() - if (!currencyMap.has(lowerCasedCurrency)) { - throw new MedusaError( - MedusaError.Types.INVALID_DATA, - `Currency with code: ${region.currency_code} was not found` - ) - } - - reg.currency_code = lowerCasedCurrency - - toCreate.push(reg as InternalCreateRegionDTO) - } - - const result = await this.regionService_.create(toCreate, sharedContext) - - return result + // Create the regions and update the country region_id + return await this.regionService_.create(normalizedDbRegions, sharedContext) } async update( @@ -208,11 +158,15 @@ export default class RegionModuleService< data?: UpdatableRegionFields, @MedusaContext() sharedContext: Context = {} ): Promise { - const result = await this.update_(idOrSelectorOrData, data, sharedContext) + const updateResult = await this.update_( + idOrSelectorOrData, + data, + sharedContext + ) const regions = await this.baseRepository_.serialize< RegionDTO[] | RegionDTO - >(result) + >(updateResult) return isString(idOrSelectorOrData) ? regions[0] : regions } @@ -223,37 +177,171 @@ export default class RegionModuleService< data?: UpdatableRegionFields, @MedusaContext() sharedContext: Context = {} ): Promise { - let result: Region[] = [] + let normalizedInput: UpdateRegionDTO[] = [] if (isString(idOrSelectorOrData)) { - result = await this.regionService_.update( - [{ id: idOrSelectorOrData, ...data }], - sharedContext - ) + normalizedInput = [{ id: idOrSelectorOrData, ...data }] } if (Array.isArray(idOrSelectorOrData)) { - result = await this.regionService_.update( - idOrSelectorOrData, - sharedContext - ) + normalizedInput = idOrSelectorOrData } if (isObject(idOrSelectorOrData)) { - let toUpdate: Partial[] = [] const regions = await this.regionService_.list( - { ...idOrSelectorOrData }, + idOrSelectorOrData, {}, sharedContext ) - regions.forEach((region) => { - toUpdate.push({ id: region.id, ...data }) - }) + normalizedInput = regions.map((region) => ({ + id: region.id, + ...data, + })) + } + normalizedInput = RegionModuleService.normalizeInput(normalizedInput) - result = await this.regionService_.update(toUpdate, sharedContext) + // If countries are being updated for a region, first make previously set countries' region to null to get to a clean slate. + // Somewhat less efficient, but region operations will be very rare, so it is better to go with a simple solution + const regionsWithCountryUpdate = normalizedInput + .filter((region) => !!region.countries) + .map((region) => region.id) + .flat() + + if (regionsWithCountryUpdate.length) { + await this.countryService_.update( + { + selector: { + region_id: regionsWithCountryUpdate, + }, + data: { region_id: null }, + }, + sharedContext + ) } - return result + const validations = [ + this.validateCurrencies( + normalizedInput.map((d) => d.currency_code), + sharedContext + ), + this.validateCountries( + normalizedInput.map((d) => d.countries ?? []).flat(), + sharedContext + ), + ] as const + + // Assign the full country object so the ORM updates the relationship + const [, dbCountries] = await promiseAll(validations) + const dbCountriesMap = new Map(dbCountries.map((d) => [d.iso_2, d])) + let normalizedDbRegions = normalizedInput.map((region) => + removeUndefined({ + ...region, + countries: region.countries?.map((c) => dbCountriesMap.get(c)), + }) + ) + + return await this.regionService_.update(normalizedDbRegions, sharedContext) + } + + private static normalizeInput( + regions: T[] + ): T[] { + return regions.map((region) => + removeUndefined({ + ...region, + currency_code: region.currency_code?.toLowerCase(), + name: region.name?.trim(), + countries: region.countries?.map((country) => country.toLowerCase()), + }) + ) + } + + private async validateCurrencies( + currencyCodes: (string | undefined)[] | undefined, + sharedContext: Context + ): Promise { + const normalizedCurrencyCodes = currencyCodes + ?.filter((c) => c !== undefined) + .map((c) => c!.toLowerCase()) + + if (!normalizedCurrencyCodes?.length) { + return + } + + const uniqueCurrencyCodes = Array.from(new Set(normalizedCurrencyCodes)) + const dbCurrencies = await this.currencyService_.list( + { code: uniqueCurrencyCodes }, + {}, + sharedContext + ) + const dbCurrencyCodes = dbCurrencies.map((c) => c.code.toLowerCase()) + + if (uniqueCurrencyCodes.length !== dbCurrencyCodes.length) { + const missingCurrencies = arrayDifference( + uniqueCurrencyCodes, + dbCurrencyCodes + ) + + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `Currencies with codes: "${missingCurrencies.join( + ", " + )}" were not found` + ) + } + } + + private async validateCountries( + countries: string[] | undefined, + sharedContext: Context + ): Promise { + if (!countries?.length) { + return [] + } + + // The new regions being created have a country conflict + const uniqueCountries = Array.from(new Set(countries)) + if (uniqueCountries.length !== countries.length) { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `Countries with codes: "${getDuplicateEntries(countries).join( + ", " + )}" are already assigned to a region` + ) + } + + const countriesInDb = await this.countryService_.list( + { iso_2: uniqueCountries }, + { select: ["iso_2", "region_id"] }, + sharedContext + ) + const countryCodesInDb = countriesInDb.map((c) => c.iso_2.toLowerCase()) + + // Countries missing in the database + if (countriesInDb.length !== uniqueCountries.length) { + const missingCountries = arrayDifference( + uniqueCountries, + countryCodesInDb + ) + + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `Countries with codes: "${missingCountries.join(", ")}" do not exist` + ) + } + + // Countries that already have a region already assigned to them + const countriesWithRegion = countriesInDb.filter((c) => !!c.region_id) + if (countriesWithRegion.length) { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `Countries with codes: "${countriesWithRegion + .map((c) => c.iso_2) + .join(", ")}" are already assigned to a region` + ) + } + + return countriesInDb } @InjectManager("baseRepository_") @@ -325,3 +413,25 @@ export default class RegionModuleService< } } } + +// microORM complains if undefined fields are present in the passed data object +const removeUndefined = >(obj: T): T => { + return Object.fromEntries( + Object.entries(obj).filter(([_, v]) => v !== undefined) + ) as T +} + +const getDuplicateEntries = (collection: string[]): string[] => { + const uniqueElements = new Set() + const duplicates: string[] = [] + + collection.forEach((item) => { + if (uniqueElements.has(item)) { + duplicates.push(item) + } else { + uniqueElements.add(item) + } + }) + + return duplicates +} diff --git a/packages/region/src/types/index.ts b/packages/region/src/types/index.ts index c1841fd471..e4f28fb1c6 100644 --- a/packages/region/src/types/index.ts +++ b/packages/region/src/types/index.ts @@ -24,9 +24,3 @@ export type CreateCountryDTO = { name: string display_name: string } - -export type CreateRegionDTO = { - name: string - currency_code: string - countries?: Country[] -} \ No newline at end of file diff --git a/packages/types/src/region/mutations.ts b/packages/types/src/region/mutations.ts index 88ad80be69..1a2ff1c55e 100644 --- a/packages/types/src/region/mutations.ts +++ b/packages/types/src/region/mutations.ts @@ -7,21 +7,15 @@ export interface CreateRegionDTO { export interface UpdateRegionDTO { id: string + name?: string currency_code?: string countries?: string[] - name?: string metadata?: Record } export interface UpdatableRegionFields { - currency_code?: string name?: string + currency_code?: string + countries?: string[] metadata?: Record } - -export interface AddCountryToRegionDTO { - region_id: string - country_id: string -} - -export interface RemoveCountryFromRegionDTO extends AddCountryToRegionDTO {}