Skip to content

Commit daa9214

Browse files
authored
fix(dashboard): sorting countries by name or code no longer throws error (#16020)
## Summary **What** — What changes are introduced in this PR? Fixes an error thrown when sorting the countries table in the admin dashboard ("The key display_name is not a valid order key"). Affects the region detail page's country section, the add-countries modal, the region-create modal, and the geo-zone country picker under shipping/locations. Also fixing string interpolation on console log for the error - ${key} was not being evaluated. **Why** — Why are these changes relevant or necessary? All four `_DataTable` instances that render a sortable countries list defined their own `orderBy` prop independently of the table's column definitions, and had copied the column ids (`display_name`, `iso_2`) instead of the values useCountries' `acceptedOrderKeys` allowlist actually accepted (name, code). `display_name` was rejected outright by the hook, throwing on sort. Additionally, "code" never corresponded to a real field anywhere in the data model, it existed only as a magic string inside the hook's internal `key === "code" ? "iso_2" : "name"` translation. **How** — How have these changes been implemented? Aligned both sides of the boundary on the real field names already present on `RegionCountryDTO`/`StaticCountry`:`name` and `iso_2`. - `use-countries.tsx`: changed `acceptedOrderKeys` to ["name", "iso_2"] and removed the "code" → "iso_2" translation, since the order key now maps directly to a real field. Also using back-tick instead of `""` for the console log. - Updated the `orderBy` arrays in `region-country-section.tsx`, `add-countries-form.tsx`, `create-region-form.tsx`, and `geo-zone-form.tsx`, `add-countries-form.tsx`, `create-region-form.tsx`, and `region-country-section.tsx` to pass { key: "name" } / { key: "iso_2" }. - This is enforced by the type system going forward, `DataTableOrderByKey<TData>.key` is typed as keyof TData, so any key that isn't a real `StaticCountry` field (like the old "code") fails to compile. **Testing** — How have these changes been tested, or how can the reviewer test the feature? Added `packages/admin/dashboard/src/routes/regions/common/hooks/__tests__/use-countries.spec.tsx`, covering: - default sort by name - descending sort via -name/-iso_2 - ascending sort by iso_2 - an invalid order key throwing - q filtering - limit/offset pagination. Manually verified in the dashboard: sorting by Name and Code (both directions) no longer errors, on all four affected screens (region detail country section, add-countries modal, region-create modal, geo-zone form). --- ## Checklist Please ensure the following before requesting a review: - [x] I have added a **changeset** for this PR - Every non-breaking change should be marked as a **patch** - To add a changeset, run `yarn changeset` and follow the prompts - [x] The changes are covered by relevant **tests** - [x] I have verified the code works as intended locally - [x] I have linked the related issue(s) if applicable --- ## Additional Context This is my first PR here so please lmk if something needs to be done differently. Fixes: #16017
1 parent 008d415 commit daa9214

7 files changed

Lines changed: 106 additions & 7 deletions

File tree

.changeset/swift-areas-flow.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@medusajs/dashboard": patch
3+
---
4+
5+
fix(dashboard): sorting countries by name or code no longer throws error

packages/admin/dashboard/src/routes/locations/common/components/geo-zone-form/geo-zone-form.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ const AreaStackedModal = <TForm extends UseFormReturn<any>>({
247247
pagination
248248
layout="fill"
249249
orderBy={[
250-
{ key: "display_name", label: t("fields.name") },
250+
{ key: "name", label: t("fields.name") },
251251
{ key: "iso_2", label: t("fields.code") },
252252
]}
253253
queryObject={raw}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import { describe, expect, it } from "vitest"
2+
3+
import { StaticCountry } from "../../../../../lib/data/countries"
4+
import { useCountries } from "../use-countries"
5+
6+
const buildCountry = (
7+
overrides: Partial<StaticCountry> = {}
8+
): StaticCountry => ({
9+
iso_2: "us",
10+
iso_3: "usa",
11+
num_code: "840",
12+
name: "UNITED STATES",
13+
display_name: "United States",
14+
...overrides,
15+
})
16+
17+
const countries: StaticCountry[] = [
18+
buildCountry({ iso_2: "dk", iso_3: "dnk", name: "DENMARK", display_name: "Denmark" }),
19+
buildCountry({ iso_2: "us", iso_3: "usa", name: "UNITED STATES", display_name: "United States" }),
20+
buildCountry({ iso_2: "ca", iso_3: "can", name: "CANADA", display_name: "Canada" }),
21+
]
22+
23+
describe("useCountries", () => {
24+
it("sorts by name ascending by default", () => {
25+
const { countries: result } = useCountries({
26+
countries: [...countries],
27+
limit: 10,
28+
})
29+
30+
expect(result.map((c) => c.iso_2)).toEqual(["ca", "dk", "us"])
31+
})
32+
33+
it("sorts by name descending when order is prefixed with -", () => {
34+
const { countries: result } = useCountries({
35+
countries: [...countries],
36+
limit: 10,
37+
order: "-name",
38+
})
39+
40+
expect(result.map((c) => c.iso_2)).toEqual(["us", "dk", "ca"])
41+
})
42+
43+
it("sorts by iso_2 ascending", () => {
44+
const { countries: result } = useCountries({
45+
countries: [...countries],
46+
limit: 10,
47+
order: "iso_2",
48+
})
49+
50+
expect(result.map((c) => c.iso_2)).toEqual(["ca", "dk", "us"])
51+
})
52+
53+
it("sorts by iso_2 descending when order is prefixed with -", () => {
54+
const { countries: result } = useCountries({
55+
countries: [...countries],
56+
limit: 10,
57+
order: "-iso_2",
58+
})
59+
60+
expect(result.map((c) => c.iso_2)).toEqual(["us", "dk", "ca"])
61+
})
62+
63+
it("throws when the order key is not accepted", () => {
64+
expect(() =>
65+
useCountries({
66+
countries: [...countries],
67+
limit: 10,
68+
order: "should_throw",
69+
})
70+
).toThrow()
71+
})
72+
73+
it("filters by q, matching on name or iso_2, ignoring order and pagination", () => {
74+
const { countries: result, count } = useCountries({
75+
countries: [...countries],
76+
limit: 10,
77+
q: "ca",
78+
})
79+
80+
expect(result.map((c) => c.iso_2)).toEqual(["ca"])
81+
expect(count).toBe(1)
82+
})
83+
84+
it("respects limit and offset when no query is provided", () => {
85+
const { countries: result, count } = useCountries({
86+
countries: [...countries],
87+
limit: 1,
88+
offset: 1,
89+
})
90+
91+
expect(result).toHaveLength(1)
92+
expect(count).toBe(countries.length)
93+
})
94+
})

packages/admin/dashboard/src/routes/regions/common/hooks/use-countries.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { RegionCountryDTO } from "@medusajs/types"
22
import { json } from "react-router-dom"
33
import { StaticCountry } from "../../../../lib/data/countries"
44

5-
const acceptedOrderKeys = ["name", "code"]
5+
const acceptedOrderKeys = ["name", "iso_2"]
66

77
/**
88
* Since countries cannot be retrieved from the API, we need to create a hook
@@ -28,11 +28,11 @@ export const useCountries = ({
2828
const key = order.replace("-", "")
2929

3030
if (!acceptedOrderKeys.includes(key)) {
31-
console.log("The key ${key} is not a valid order key")
31+
console.log(`The key ${key} is not a valid order key`)
3232
throw json(`The key ${key} is not a valid order key`, 500)
3333
}
3434

35-
const sortKey: keyof RegionCountryDTO = key === "code" ? "iso_2" : "name"
35+
const sortKey = key as keyof RegionCountryDTO
3636

3737
data.sort((a, b) => {
3838
if (a[sortKey] === null && b[sortKey] === null) {

packages/admin/dashboard/src/routes/regions/region-add-countries/components/add-countries-form/add-countries-form.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ export const AddCountriesForm = ({ region }: AddCountriesFormProps) => {
144144
pagination
145145
layout="fill"
146146
orderBy={[
147-
{ key: "display_name", label: t("fields.name") },
147+
{ key: "name", label: t("fields.name") },
148148
{ key: "iso_2", label: t("fields.code") },
149149
]}
150150
queryObject={raw}

packages/admin/dashboard/src/routes/regions/region-create/components/create-region-form/create-region-form.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ export const CreateRegionForm = ({ currencies }: CreateRegionFormProps) => {
373373
count={count}
374374
pageSize={PAGE_SIZE}
375375
orderBy={[
376-
{ key: "display_name", label: t("fields.name") },
376+
{ key: "name", label: t("fields.name") },
377377
{ key: "iso_2", label: t("fields.code") },
378378
]}
379379
pagination

packages/admin/dashboard/src/routes/regions/region-detail/components/region-country-section/region-country-section.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ export const RegionCountrySection = ({ region }: RegionCountrySectionProps) => {
122122
pageSize={PAGE_SIZE}
123123
count={count}
124124
orderBy={[
125-
{ key: "display_name", label: t("fields.name") },
125+
{ key: "name", label: t("fields.name") },
126126
{ key: "iso_2", label: t("fields.code") },
127127
]}
128128
search

0 commit comments

Comments
 (0)