feat(regions): Add support for updating countries in a region (#6432)

**What**
Add missing support for updating countries on the POST /admin/regions/:id route.
Furthermore, the PR refactors how the normalization and validation was handled in both create and update scenarios.
Finally, the PR adds a unique index on the country entity, which ensures at the DB level that we cannot have a duplicate country in the DB for a single region.
This commit is contained in:
Stevche Radevski
2024-02-20 12:46:42 +01:00
committed by GitHub
parent c319edb8e0
commit cfefd59249
10 changed files with 436 additions and 132 deletions

View File

@@ -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"])
})
})

View File

@@ -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),
}))
)
}

View File

@@ -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<string, unknown>
}
export class AdminPostRegionsRegionReq {
@@ -86,4 +100,12 @@ export class AdminPostRegionsRegionReq {
@IsString()
@IsOptional()
currency_code?: string
@IsArray()
@IsOptional()
countries?: string[]
@IsObject()
@IsOptional()
metadata?: Record<string, unknown>
}

View File

@@ -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')
})
})

View File

@@ -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"
}
}
}
]
}
}

View File

@@ -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";

View File

@@ -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

View File

@@ -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<Region[]> {
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<string, Country>(
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<string>()
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<RegionDTO | RegionDTO[]> {
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<Region[]> {
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<UpdateRegionDTO>[] = []
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<T extends UpdatableRegionFields>(
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<void> {
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<TCountry[]> {
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 = <T extends Record<string, any>>(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
}

View File

@@ -24,9 +24,3 @@ export type CreateCountryDTO = {
name: string
display_name: string
}
export type CreateRegionDTO = {
name: string
currency_code: string
countries?: Country[]
}

View File

@@ -7,21 +7,15 @@ export interface CreateRegionDTO {
export interface UpdateRegionDTO {
id: string
name?: string
currency_code?: string
countries?: string[]
name?: string
metadata?: Record<string, unknown>
}
export interface UpdatableRegionFields {
currency_code?: string
name?: string
currency_code?: string
countries?: string[]
metadata?: Record<string, unknown>
}
export interface AddCountryToRegionDTO {
region_id: string
country_id: string
}
export interface RemoveCountryFromRegionDTO extends AddCountryToRegionDTO {}