Skip to content

Commit 638f0c4

Browse files
steverydzsteverydz
authored
fix: Only show remodels to users with permissions (#5669)
1 parent c0711ed commit 638f0c4

7 files changed

Lines changed: 268 additions & 47 deletions

File tree

‎static/js/publisher/hooks/__tests__/useRemodels.test.tsx‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,18 @@ const remodelsResponse = {
3434
"to-model": "test-to-model",
3535
},
3636
],
37+
"next-cursor": null,
3738
};
3839

3940
const handlers = [
4041
http.get("/api/store/test-brand-id/models/remodel-allowlist", () => {
4142
return HttpResponse.json({
4243
data: remodelsResponse,
43-
message: "",
4444
success: true,
4545
});
4646
}),
4747
http.get("/api/store/test-brand-id-fail/models/remodel-allowlist", () => {
4848
return HttpResponse.json({
49-
data: [],
5049
message: "There was a problem fetching remodels",
5150
success: false,
5251
});
@@ -84,7 +83,10 @@ describe("useRemodels", () => {
8483
expect(result.current.isSuccess).toBe(true);
8584
});
8685

87-
expect(result.current.data).toEqual(remodelsResponse.allowlist);
86+
expect(result.current.data).toEqual({
87+
data: remodelsResponse,
88+
success: true,
89+
});
8890
});
8991

9092
test("returns error if request fails", async () => {
@@ -96,10 +98,13 @@ describe("useRemodels", () => {
9698
);
9799

98100
await waitFor(() => {
99-
expect(result.current.isError).toBe(true);
101+
expect(result.current.isSuccess).toBe(true);
100102
});
101103

102-
expect(result.current.data).toBeUndefined();
104+
expect(result.current.data).toEqual({
105+
message: "There was a problem fetching remodels",
106+
success: false,
107+
});
103108
});
104109

105110
test("returns error if network error", async () => {

‎static/js/publisher/hooks/useRemodels.ts‎

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,45 @@
11
import { useQuery, UseQueryResult } from "react-query";
2-
import type { Remodel } from "../types/shared";
2+
import type { Remodel, RemodelResponse, ApiResponse } from "../types/shared";
33

44
const useRemodels = (
55
brandId: string | undefined,
66
modelId: string | undefined,
7-
): UseQueryResult<Remodel[], Error> => {
8-
return useQuery<Remodel[], Error>({
7+
): UseQueryResult<ApiResponse<RemodelResponse>, Error> => {
8+
return useQuery<ApiResponse<RemodelResponse>, Error>({
99
queryKey: ["remodels", brandId, modelId],
1010
queryFn: async () => {
1111
const response = await fetch(
1212
`/api/store/${brandId}/models/remodel-allowlist`,
1313
);
1414

15-
if (!response.ok) {
16-
throw new Error("There was a problem fetching remodels");
15+
const responseData = await response.json();
16+
17+
if (responseData.data?.allowlist) {
18+
const remodelsForCurrentModel = responseData.data.allowlist.filter(
19+
(remodel: Remodel) => {
20+
return (
21+
remodel["from-model"] === modelId ||
22+
remodel["to-model"] === modelId
23+
);
24+
},
25+
);
26+
27+
responseData.data.allowlist = remodelsForCurrentModel.sort(
28+
(a: Remodel, b: Remodel) => {
29+
if (a["created-at"] > b["created-at"]) {
30+
return -1;
31+
}
32+
33+
if (a["created-at"] < b["created-at"]) {
34+
return 1;
35+
}
36+
37+
return 0;
38+
},
39+
);
1740
}
1841

19-
const data = await response.json();
20-
21-
if (!data.success) {
22-
throw new Error(data.message);
23-
}
24-
25-
const remodelsForCurrentModel = data.data.allowlist.filter(
26-
(remodel: Remodel) => {
27-
return (
28-
remodel["from-model"] === modelId || remodel["to-model"] === modelId
29-
);
30-
},
31-
);
32-
33-
return remodelsForCurrentModel.sort((a: Remodel, b: Remodel) => {
34-
if (a["created-at"] > b["created-at"]) {
35-
return -1;
36-
}
37-
38-
if (a["created-at"] < b["created-at"]) {
39-
return 1;
40-
}
41-
42-
return 0;
43-
});
42+
return responseData;
4443
},
4544
enabled: !!brandId,
4645
});

‎static/js/publisher/layouts/ModelDetailsPageLayout/ModelNav.tsx‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
import { Link, useParams } from "react-router-dom";
2+
import { useAtomValue } from "jotai";
3+
4+
import { useRemodels } from "../../hooks";
5+
import { brandIdState } from "../../state/brandStoreState";
26

37
function ModelNav({ sectionName }: { sectionName: string }): React.JSX.Element {
48
const { id, modelId } = useParams();
9+
const brandId = useAtomValue(brandIdState);
10+
const { data } = useRemodels(brandId, modelId);
511

612
return (
713
<nav className="p-tabs">
@@ -26,6 +32,18 @@ function ModelNav({ sectionName }: { sectionName: string }): React.JSX.Element {
2632
Policies
2733
</Link>
2834
</li>
35+
{data?.success && (
36+
<li className="p-tabs__item">
37+
<Link
38+
to={`/admin/${id}/models/${modelId}/remodel`}
39+
className="p-tabs__link"
40+
aria-selected={sectionName === "remodel"}
41+
role="tab"
42+
>
43+
Remodel
44+
</Link>
45+
</li>
46+
)}
2947
</ul>
3048
</nav>
3149
);

‎static/js/publisher/layouts/ModelDetailsPageLayout/__tests__/ModelNav.test.tsx‎

Lines changed: 95 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,118 @@
11
import { BrowserRouter } from "react-router-dom";
2+
import { QueryClient, QueryClientProvider } from "react-query";
23
import { render, screen } from "@testing-library/react";
4+
import { vi } from "vitest";
35
import "@testing-library/jest-dom";
46

57
import ModelNav from "../ModelNav";
8+
import { useRemodels } from "../../../hooks";
69

7-
const renderComponent = () => {
10+
import type { UseQueryResult } from "react-query";
11+
import type { ApiResponse, RemodelResponse } from "../../../types/shared";
12+
13+
vi.mock("../../../hooks", () => ({
14+
useRemodels: vi.fn(),
15+
}));
16+
17+
vi.mock("../../../state/brandStoreState", () => ({
18+
brandIdState: "mock-brand-id",
19+
}));
20+
21+
vi.mock("jotai", () => ({
22+
useAtomValue: vi.fn(() => "mock-brand-id"),
23+
}));
24+
25+
const queryClient = new QueryClient();
26+
27+
const renderComponent = (sectionName: string) => {
828
return render(
929
<BrowserRouter>
10-
<ModelNav sectionName="policies" />
30+
<QueryClientProvider client={queryClient}>
31+
<ModelNav sectionName={sectionName} />
32+
</QueryClientProvider>
1133
</BrowserRouter>,
1234
);
1335
};
1436

1537
describe("ModelNav", () => {
1638
it("highlights the correct navigation item", () => {
17-
renderComponent();
39+
const mockUseRemodels = vi.mocked(useRemodels);
40+
mockUseRemodels.mockReturnValue({
41+
data: {
42+
success: true,
43+
data: {
44+
allowlist: [],
45+
"next-cursor": null,
46+
},
47+
},
48+
} as unknown as UseQueryResult<ApiResponse<RemodelResponse>, Error>);
49+
50+
renderComponent("policies");
1851
const currentLink = screen.getByRole("tab", { name: "Policies" });
1952
expect(currentLink.getAttribute("aria-selected")).toBe("true");
2053
});
2154

2255
it("doesn't highlight other navigation links", () => {
23-
renderComponent();
56+
const mockUseRemodels = vi.mocked(useRemodels);
57+
mockUseRemodels.mockReturnValue({
58+
data: {
59+
success: true,
60+
data: {
61+
allowlist: [],
62+
"next-cursor": null,
63+
},
64+
},
65+
} as unknown as UseQueryResult<ApiResponse<RemodelResponse>, Error>);
66+
67+
renderComponent("policies");
2468
const currentLink = screen.getByRole("tab", { name: "Overview" });
2569
expect(currentLink.getAttribute("aria-selected")).toBe("false");
2670
});
71+
72+
it("shows Remodel tab when useRemodels returns success: true", () => {
73+
const mockUseRemodels = vi.mocked(useRemodels);
74+
mockUseRemodels.mockReturnValue({
75+
data: {
76+
success: true,
77+
data: {
78+
allowlist: [],
79+
"next-cursor": null,
80+
},
81+
},
82+
} as unknown as UseQueryResult<ApiResponse<RemodelResponse>, Error>);
83+
84+
renderComponent("overview");
85+
expect(screen.getByRole("tab", { name: "Remodel" })).toBeInTheDocument();
86+
});
87+
88+
it("hides Remodel tab when useRemodels returns success: false", () => {
89+
const mockUseRemodels = vi.mocked(useRemodels);
90+
mockUseRemodels.mockReturnValue({
91+
data: {
92+
success: false,
93+
message: "Remodeling not available",
94+
data: {
95+
allowlist: [],
96+
"next-cursor": null,
97+
},
98+
},
99+
} as unknown as UseQueryResult<ApiResponse<RemodelResponse>, Error>);
100+
101+
renderComponent("overview");
102+
expect(
103+
screen.queryByRole("tab", { name: "Remodel" }),
104+
).not.toBeInTheDocument();
105+
});
106+
107+
it("hides Remodel tab when useRemodels returns no data", () => {
108+
const mockUseRemodels = vi.mocked(useRemodels);
109+
mockUseRemodels.mockReturnValue({
110+
data: undefined,
111+
} as unknown as UseQueryResult<ApiResponse<RemodelResponse>, Error>);
112+
113+
renderComponent("overview");
114+
expect(
115+
screen.queryByRole("tab", { name: "Remodel" }),
116+
).not.toBeInTheDocument();
117+
});
27118
});

‎static/js/publisher/pages/Remodel/Remodel.tsx‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import RemodelTable from "./RemodelTable";
2222
import ConfigureRemodelForm from "./ConfigureRemodelForm";
2323

2424
import type { UseQueryResult } from "react-query";
25-
import type { Remodel } from "../../types/shared";
25+
import type { Remodel, RemodelResponse, ApiResponse } from "../../types/shared";
2626

2727
function Remodel(): React.JSX.Element {
2828
const { id, modelId } = useParams();
@@ -33,7 +33,10 @@ function Remodel(): React.JSX.Element {
3333
error,
3434
data,
3535
refetch,
36-
}: UseQueryResult<Remodel[], Error> = useRemodels(brandId, modelId);
36+
}: UseQueryResult<ApiResponse<RemodelResponse>, Error> = useRemodels(
37+
brandId,
38+
modelId,
39+
);
3740
const setRemodels = useSetAtom(remodelsListState);
3841
const setFilter = useSetAtom(remodelsListFilterState);
3942
const [showNotification, setShowNotification] = useState(false);
@@ -49,7 +52,7 @@ function Remodel(): React.JSX.Element {
4952

5053
useEffect(() => {
5154
if (!isLoading && !isError && data) {
52-
setRemodels(data);
55+
setRemodels(data.data?.allowlist || []);
5356
setFilter(searchParams.get("filter") || "");
5457
}
5558
}, [isLoading, error, data, brandId, id]);
@@ -67,6 +70,10 @@ function Remodel(): React.JSX.Element {
6770
<Icon name="spinner" className="u-animation--spin" />
6871
&nbsp;Fetching remodels...
6972
</p>
73+
) : data && data.success === false ? (
74+
<Notification severity="caution">
75+
{data.message || "Unable to fetch remodels"}
76+
</Notification>
7077
) : (
7178
<>
7279
<Row>

0 commit comments

Comments
 (0)
⚡