Skip to content

Commit 7ef1726

Browse files
abuyukyi101198abuyukyi101198
authored
refactor(machines): CloneStorageTable to GenericTable MAASENG-5687 (#5952)
1 parent 29a350b commit 7ef1726

File tree

7 files changed

+214
-198
lines changed

7 files changed

+214
-198
lines changed

‎src/app/machines/components/MachineForms/MachineActionFormWrapper/CloneForm/CloneForm.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import {
2323
} from "@/app/store/machine/utils/hooks";
2424
import { NodeActions } from "@/app/store/types/node";
2525

26+
import "./_index.scss";
27+
2628
type CloneMachineProps = {
2729
isViewingDetails: boolean;
2830
searchFilter?: string;
@@ -122,6 +124,7 @@ export const CloneForm = ({
122124
</ExternalLink>
123125
</p>
124126
}
127+
className="clone-form"
125128
initialValues={{
126129
interfaces: false,
127130
source: "",

‎src/app/machines/components/MachineForms/MachineActionFormWrapper/CloneForm/CloneFormFields/CloneFormFields.tsx‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export const CloneFormFields = ({
3333
}: Props): React.ReactElement => {
3434
const { setFieldValue, values } = useFormikContext<CloneFormValues>();
3535
const cloneNetworkTableContainerRef = useRef<HTMLDivElement>(null);
36+
const cloneStorageTableContainerRef = useRef<HTMLDivElement>(null);
3637

3738
const machineInState = useSelector((state: RootState) =>
3839
machineSelectors.getById(state, values.source)
@@ -126,25 +127,25 @@ export const CloneFormFields = ({
126127
/>
127128
</div>
128129
</div>
129-
</div>
130-
</Col>
131-
</Row>
132-
<Row>
133-
<Col size={12}>
134-
<div className="clone-table-card">
135-
<FormikField
136-
disabled={!selectedMachine}
137-
label="Clone storage configuration"
138-
name="storage"
139-
type="checkbox"
140-
wrapperClassName="u-sv2"
141-
/>
142-
<div className="clone-table-container">
143-
<CloneStorageTable
144-
loadingMachineDetails={loadingMachineDetails}
145-
machine={selectedMachine}
146-
selected={values.storage}
130+
<div className="clone-table-card">
131+
<FormikField
132+
disabled={!selectedMachine}
133+
label="Clone storage configuration"
134+
name="storage"
135+
type="checkbox"
136+
wrapperClassName="u-sv2"
147137
/>
138+
<div
139+
className="clone-table-container"
140+
ref={cloneStorageTableContainerRef}
141+
>
142+
<CloneStorageTable
143+
containerRef={cloneStorageTableContainerRef}
144+
loadingMachineDetails={loadingMachineDetails}
145+
machine={selectedMachine}
146+
selected={values.storage}
147+
/>
148+
</div>
148149
</div>
149150
</div>
150151
</Col>

‎src/app/machines/components/MachineForms/MachineActionFormWrapper/CloneForm/CloneFormFields/CloneStorageTable/CloneStorageTable.test.tsx‎

Lines changed: 22 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,23 @@
11
import CloneStorageTable from "./CloneStorageTable";
22

33
import * as factory from "@/testing/factories";
4-
import { screen, within, renderWithProviders } from "@/testing/utils";
4+
import { renderWithProviders, screen, within } from "@/testing/utils";
55

66
describe("CloneStorageTable", () => {
7-
it("renders empty table if neither loading details nor machine provided", () => {
7+
it("renders the table", () => {
88
renderWithProviders(<CloneStorageTable machine={null} selected={false} />);
9-
expect(screen.getAllByRole("row")).toHaveLength(1);
10-
expect(screen.queryByTestId("placeholder")).not.toBeInTheDocument();
9+
expect(screen.getByTestId("p-generic-table")).toBeInTheDocument();
1110
});
1211

13-
it("renders placeholder content while machine details are loading", () => {
12+
it("renders a loading spinner while machine details are loading", () => {
1413
renderWithProviders(
1514
<CloneStorageTable
1615
loadingMachineDetails
1716
machine={null}
1817
selected={false}
1918
/>
2019
);
21-
const tableBody = screen.getAllByRole("rowgroup")[1];
22-
const rows = within(tableBody).getAllByRole("row");
23-
24-
rows.forEach((row) => {
25-
const placeholders = within(row).getAllByTestId("placeholder");
26-
expect(placeholders[0]).toHaveTextContent("Disk name");
27-
expect(placeholders[1]).toHaveTextContent("Model");
28-
expect(placeholders[2]).toHaveTextContent("1.0.0");
29-
expect(placeholders[3]).toHaveTextContent("Disk type");
30-
expect(placeholders[4]).toHaveTextContent("X, X");
31-
expect(placeholders[5]).toHaveTextContent("1.23 GB");
32-
expect(placeholders[6]).toHaveTextContent("Icon");
33-
});
20+
expect(screen.getByRole("alert")).toHaveTextContent("Loading...");
3421
});
3522

3623
it("renders machine storage details if machine is provided", () => {
@@ -40,8 +27,7 @@ describe("CloneStorageTable", () => {
4027
renderWithProviders(
4128
<CloneStorageTable machine={machine} selected={false} />
4229
);
43-
expect(screen.queryByTestId("placeholder")).not.toBeInTheDocument();
44-
30+
expect(screen.queryByRole("alert")).not.toBeInTheDocument();
4531
expect(screen.getByText("Disk 1")).toBeInTheDocument();
4632
});
4733

@@ -54,10 +40,10 @@ describe("CloneStorageTable", () => {
5440
renderWithProviders(
5541
<CloneStorageTable machine={machine} selected={false} />
5642
);
57-
expect(screen.getByTestId("disk-available")).toHaveClass("p-icon--tick");
58-
expect(screen.getByTestId("disk-available")).toHaveAccessibleName(
59-
"available"
60-
);
43+
const availableCell = within(screen.getAllByRole("row")[1]).getAllByRole(
44+
"cell"
45+
)[4];
46+
expect(availableCell.querySelector("i")).toHaveClass("p-icon--tick");
6147
});
6248

6349
it("shows a cross for unavailable disks", () => {
@@ -67,10 +53,10 @@ describe("CloneStorageTable", () => {
6753
renderWithProviders(
6854
<CloneStorageTable machine={machine} selected={false} />
6955
);
70-
expect(screen.getByTestId("disk-available")).toHaveClass("p-icon--close");
71-
expect(screen.getByTestId("disk-available")).toHaveAccessibleName(
72-
"not available"
73-
);
56+
const availableCell = within(screen.getAllByRole("row")[1]).getAllByRole(
57+
"cell"
58+
)[4];
59+
expect(availableCell.querySelector("i")).toHaveClass("p-icon--close");
7460
});
7561

7662
it("shows a tick for available partitions", () => {
@@ -84,12 +70,10 @@ describe("CloneStorageTable", () => {
8470
renderWithProviders(
8571
<CloneStorageTable machine={machine} selected={false} />
8672
);
87-
expect(screen.getByTestId("partition-available")).toHaveClass(
88-
"p-icon--tick"
89-
);
90-
expect(screen.getByTestId("partition-available")).toHaveAccessibleName(
91-
"available"
92-
);
73+
const availableCell = within(screen.getAllByRole("row")[2]).getAllByRole(
74+
"cell"
75+
)[4];
76+
expect(availableCell.querySelector("i")).toHaveClass("p-icon--tick");
9377
});
9478

9579
it("shows a cross for unavailable partitions", () => {
@@ -105,11 +89,9 @@ describe("CloneStorageTable", () => {
10589
renderWithProviders(
10690
<CloneStorageTable machine={machine} selected={false} />
10791
);
108-
expect(screen.getByTestId("partition-available")).toHaveClass(
109-
"p-icon--close"
110-
);
111-
expect(screen.getByTestId("partition-available")).toHaveAccessibleName(
112-
"not available"
113-
);
92+
const availableCell = within(screen.getAllByRole("row")[2]).getAllByRole(
93+
"cell"
94+
)[4];
95+
expect(availableCell.querySelector("i")).toHaveClass("p-icon--close");
11496
});
11597
});

‎src/app/machines/components/MachineForms/MachineActionFormWrapper/CloneForm/CloneFormFields/CloneStorageTable/CloneStorageTable.tsx‎

Lines changed: 41 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -1,161 +1,78 @@
1-
import type { ReactNode } from "react";
1+
import type { ReactElement, RefObject } from "react";
22

3-
import { Icon, MainTable } from "@canonical/react-components";
4-
import type { MainTableRow } from "@canonical/react-components/dist/components/MainTable/MainTable";
3+
import { GenericTable } from "@canonical/maas-react-components";
54
import classNames from "classnames";
65

7-
import DoubleRow from "@/app/base/components/DoubleRow";
8-
import Placeholder from "@/app/base/components/Placeholder";
9-
import DiskNumaNodes from "@/app/base/components/node/DiskNumaNodes";
6+
import type { CloneStorage } from "./useCloneStorageTableColumns/useCloneStorageTableColumns";
7+
import useCloneStorageTableColumns from "./useCloneStorageTableColumns/useCloneStorageTableColumns";
8+
109
import type { MachineDetails } from "@/app/store/machine/types";
10+
import type { Disk, Partition } from "@/app/store/types/node";
1111
import {
1212
diskAvailable,
1313
formatSize,
1414
formatType,
1515
partitionAvailable,
1616
} from "@/app/store/utils";
1717

18-
type Props = {
18+
type CloneStorageTableProps = {
19+
containerRef?: RefObject<HTMLElement | null>;
1920
loadingMachineDetails?: boolean;
2021
machine: MachineDetails | null;
2122
selected: boolean;
2223
};
2324

24-
const normaliseColumns = ({
25-
available,
26-
model,
27-
name,
28-
size,
29-
type,
30-
}: {
31-
available: ReactNode;
32-
model: ReactNode;
33-
name: ReactNode;
34-
size: ReactNode;
35-
type: ReactNode;
36-
}) => [
37-
{ className: "name-col", content: name },
38-
{ className: "model-col", content: model },
39-
{ className: "type-col", content: type },
40-
{ className: "size-col", content: size },
41-
{ className: "available-col u-align--center", content: available },
42-
];
43-
4425
export const CloneStorageTable = ({
26+
containerRef,
4527
loadingMachineDetails = false,
4628
machine,
4729
selected,
48-
}: Props): React.ReactElement => {
49-
let rows: MainTableRow[] = [];
30+
}: CloneStorageTableProps): ReactElement => {
31+
const columns = useCloneStorageTableColumns();
32+
const data: CloneStorage[] = [];
5033

51-
if (loadingMachineDetails) {
52-
rows = Array.from(Array(4)).map(() => ({
53-
columns: normaliseColumns({
54-
available: <Placeholder>Icon</Placeholder>,
55-
model: (
56-
<DoubleRow
57-
primary={<Placeholder>Model</Placeholder>}
58-
secondary={<Placeholder>1.0.0</Placeholder>}
59-
/>
60-
),
61-
name: <Placeholder>Disk name</Placeholder>,
62-
size: <Placeholder>1.23 GB</Placeholder>,
63-
type: (
64-
<DoubleRow
65-
primary={<Placeholder>Disk type</Placeholder>}
66-
secondary={<Placeholder>X, X</Placeholder>}
67-
/>
68-
),
69-
}),
70-
}));
71-
} else if (machine) {
72-
rows = [];
73-
machine.disks.forEach((disk) => {
74-
rows.push({
75-
columns: normaliseColumns({
76-
available: (
77-
<Icon
78-
aria-label={diskAvailable(disk) ? "available" : "not available"}
79-
data-testid="disk-available"
80-
name={diskAvailable(disk) ? "tick" : "close"}
81-
/>
82-
),
83-
model: (
84-
<DoubleRow
85-
primary={disk.model || "—"}
86-
primaryTitle={disk.model}
87-
secondary={disk.firmware_version}
88-
secondaryTitle={disk.firmware_version}
89-
/>
90-
),
91-
name: <DoubleRow primary={disk.name} primaryTitle={disk.name} />,
92-
size: formatSize(disk.size),
93-
type: (
94-
<DoubleRow
95-
primary={formatType(disk)}
96-
primaryTitle={formatType(disk)}
97-
secondary={<DiskNumaNodes disk={disk} />}
98-
/>
99-
),
100-
}),
34+
if (machine) {
35+
machine.disks.forEach((disk: Disk) => {
36+
data.push({
37+
id: `disk-${disk.id}`,
38+
name: disk.name,
39+
model: disk.model || "—",
40+
firmwareVersion: disk.firmware_version,
41+
type: formatType(disk),
42+
numaNodesDisk: disk,
43+
size: formatSize(disk.size),
44+
available: diskAvailable(disk),
10145
});
10246

10347
if (disk.partitions) {
104-
disk.partitions.forEach((partition) => {
105-
rows.push({
106-
columns: normaliseColumns({
107-
available: (
108-
<Icon
109-
aria-label={
110-
partitionAvailable(partition)
111-
? "available"
112-
: "not available"
113-
}
114-
data-testid="partition-available"
115-
name={partitionAvailable(partition) ? "tick" : "close"}
116-
/>
117-
),
118-
model: "—",
119-
name: partition.name,
120-
size: formatSize(partition.size),
121-
type: (
122-
<DoubleRow
123-
primary={formatType(partition)}
124-
primaryTitle={formatType(partition)}
125-
secondary={<DiskNumaNodes disk={disk} />}
126-
/>
127-
),
128-
}),
48+
disk.partitions.forEach((partition: Partition) => {
49+
data.push({
50+
id: `partition-${partition.id}`,
51+
name: partition.name,
52+
model: "—",
53+
firmwareVersion: "",
54+
type: formatType(partition),
55+
numaNodesDisk: disk,
56+
size: formatSize(partition.size),
57+
available: partitionAvailable(partition),
12958
});
13059
});
13160
}
13261
});
13362
}
13463

13564
return (
136-
<MainTable
65+
<GenericTable
66+
aria-label="Clone storage"
13767
className={classNames("clone-table--storage", {
13868
"not-selected": !selected,
13969
})}
140-
emptyStateMsg={machine ? "No storage information detected." : null}
141-
headers={normaliseColumns({
142-
available: "Available",
143-
model: (
144-
<>
145-
<div>Model</div>
146-
<div>Firmware</div>
147-
</>
148-
),
149-
name: "Name",
150-
size: "Size",
151-
type: (
152-
<>
153-
<div>Type</div>
154-
<div>NUMA node</div>
155-
</>
156-
),
157-
})}
158-
rows={rows}
70+
columns={columns}
71+
containerRef={containerRef}
72+
data={data}
73+
isLoading={loadingMachineDetails}
74+
noData={machine ? "No storage information detected." : null}
75+
variant="full-height"
15976
/>
16077
);
16178
};

0 commit comments

Comments
 (0)