Skip to content

Commit

Permalink
WD-11589 - refactor: remove nested components in StatusGroup (#1765)
Browse files Browse the repository at this point in the history
  • Loading branch information
vladimir-cucu authored Jun 4, 2024
1 parent f5d2751 commit 16a7c52
Show file tree
Hide file tree
Showing 5 changed files with 195 additions and 111 deletions.
48 changes: 2 additions & 46 deletions src/components/ModelTableList/StatusGroup/StatusGroup.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import { act, screen, within } from "@testing-library/react";
import type { UserEvent } from "@testing-library/user-event";
import userEvent from "@testing-library/user-event";
import { vi } from "vitest";
import { screen, within } from "@testing-library/react";

import type { RootState } from "store/store";
import { rootStateFactory } from "testing/factories";
Expand All @@ -16,7 +13,6 @@ import {
modelDataApplicationFactory,
modelDataFactory,
modelDataInfoFactory,
modelDataUnitFactory,
modelListInfoFactory,
} from "testing/factories/juju/juju";
import { renderComponent } from "testing/utils";
Expand All @@ -25,13 +21,8 @@ import StatusGroup from "./StatusGroup";

describe("StatusGroup", () => {
let state: RootState;
let userEventWithTimers: UserEvent;

beforeEach(() => {
vi.useFakeTimers();
userEventWithTimers = userEvent.setup({
advanceTimers: vi.advanceTimersByTime,
});
state = rootStateFactory.build({
juju: jujuStateFactory.build({
modelData: {
Expand Down Expand Up @@ -99,10 +90,6 @@ describe("StatusGroup", () => {
});
});

afterEach(() => {
vi.useRealTimers();
});

it("by default, renders no tables when there is no data", () => {
const state = rootStateFactory.build();
renderComponent(<StatusGroup filters={{}} />, { state });
Expand Down Expand Up @@ -177,50 +164,19 @@ describe("StatusGroup", () => {
);
});

it("displays links to blocked apps and units", async () => {
vi.useFakeTimers();
it("displays links to blocked apps", async () => {
state.juju.modelData.abc123.applications = {
calico: modelDataApplicationFactory.build({
status: detailedStatusFactory.build({
info: "app blocked",
status: "blocked",
}),
}),
etcd: modelDataApplicationFactory.build({
units: {
"etcd/0": modelDataUnitFactory.build({
"agent-status": detailedStatusFactory.build({
info: "unit blocked",
status: "lost",
}),
}),
},
}),
};
renderComponent(<StatusGroup filters={{}} />, { state });
const tables = screen.getAllByRole("grid");
const row = within(tables[0]).getAllByRole("row")[1];
const error = within(row).getByRole("link", { name: "app blocked" });
expect(error).toHaveAttribute("href", "/models/eggman@external/sub-test");
await act(async () => {
await userEventWithTimers.hover(error);
vi.runAllTimers();
});
const tooltip = screen.getAllByRole("tooltip")[0];
expect(error).toHaveAttribute("href", "/models/eggman@external/sub-test");
const appError = within(tooltip).getByRole("link", {
name: "app blocked",
});
expect(appError).toHaveAttribute(
"href",
"/models/eggman@external/sub-test/app/calico",
);
const unitError = within(tooltip).getByRole("link", {
name: "unit blocked",
});
expect(unitError).toHaveAttribute(
"href",
"/models/eggman@external/sub-test/app/etcd/unit/etcd-0",
);
});
});
69 changes: 4 additions & 65 deletions src/components/ModelTableList/StatusGroup/StatusGroup.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { List, MainTable, Tooltip } from "@canonical/react-components";
import type { ListItem } from "@canonical/react-components/dist/components/List/List";
import { MainTable } from "@canonical/react-components";
import type { MainTableRow } from "@canonical/react-components/dist/components/MainTable/MainTable";
import { useSelector } from "react-redux";
import { Link } from "react-router-dom";

import ModelDetailsLink from "components/ModelDetailsLink";
import TruncatedTooltip from "components/TruncatedTooltip";
Expand All @@ -15,13 +13,7 @@ import {
} from "store/juju/selectors";
import type { Controllers, ModelData } from "store/juju/types";
import type { Filters, Status } from "store/juju/utils/models";
import {
canAdministerModel,
extractOwnerName,
getModelStatusGroupData,
} from "store/juju/utils/models";
import urls from "urls";
import { getUserName } from "utils";
import { canAdministerModel, extractOwnerName } from "store/juju/utils/models";

import AccessButton from "../AccessButton/AccessButton";
import CloudCell from "../CloudCell/CloudCell";
Expand All @@ -34,62 +26,9 @@ import {
getLastUpdated,
} from "../shared";

import WarningMessage from "./WarningMessage";
import { TestId } from "./types";

/**
Generates the warning message for the model name cell.
@param model The full model data.
@return The react component for the warning message.
*/
const generateWarningMessage = (model: ModelData) => {
const { messages } = getModelStatusGroupData(model);
if (!messages.length) {
return null;
}
const ownerTag = model?.info?.["owner-tag"] ?? "";
const userName = getUserName(ownerTag);
const modelName = model.model.name;
const link = (
<ModelDetailsLink modelName={modelName} ownerTag={ownerTag}>
{messages[0].message}
</ModelDetailsLink>
);
const list: ListItem[] = messages.slice(0, 5).map((message) => {
const unitId = message.unitId ? message.unitId.replace("/", "-") : null;
const appName = message.appName;
return {
className: "u-truncate",
content: (
<>
{unitId || appName}:{" "}
<Link
to={
unitId
? urls.model.unit({ userName, modelName, appName, unitId })
: urls.model.app.index({ userName, modelName, appName })
}
>
{message.message}
</Link>
</>
),
};
});
const remainder = messages.slice(5);
if (remainder.length) {
list.push(`+${remainder.length} more...`);
}
return (
<Tooltip
positionElementClassName="p-tooltip__position-element--inline"
tooltipClassName="p-tooltip--constrain-width"
message={<List className="u-no-margin--bottom" items={list} />}
>
<span className="model-table-list_error-message">{link}</span>
</Tooltip>
);
};

/**
Generates the model name cell.
@param model The model data.
Expand All @@ -108,7 +47,7 @@ const generateModelNameCell = (model: ModelData, groupLabel: string) => {
{model.model.name}
</ModelDetailsLink>
</TruncatedTooltip>
{groupLabel === "blocked" ? generateWarningMessage(model) : null}
{groupLabel === "blocked" ? <WarningMessage model={model} /> : null}
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { screen, act, within } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import type { UserEvent } from "@testing-library/user-event";
import { vi } from "vitest";

import { detailedStatusFactory } from "testing/factories/juju/ClientV6";
import {
modelDataApplicationFactory,
modelDataFactory,
modelDataUnitFactory,
} from "testing/factories/juju/juju";
import { renderComponent } from "testing/utils";

import WarningMessage from "./WarningMessage";

describe("WarningMessage", () => {
let userEventWithTimers: UserEvent;

beforeEach(() => {
vi.useFakeTimers();
userEventWithTimers = userEvent.setup({
advanceTimers: vi.advanceTimersByTime,
});
});

afterEach(() => {
vi.useRealTimers();
});

it("should display nothing if there are no messages", () => {
const model = modelDataFactory.build();
renderComponent(<WarningMessage model={model} />);
expect(screen.queryByRole("link")).not.toBeInTheDocument();
});

it("should display links to blocked apps and units", async () => {
const model = modelDataFactory.build({
applications: {
calico: modelDataApplicationFactory.build({
status: detailedStatusFactory.build({
info: "app blocked",
status: "blocked",
}),
}),
etcd: modelDataApplicationFactory.build({
units: {
"etcd/0": modelDataUnitFactory.build({
"agent-status": detailedStatusFactory.build({
info: "unit blocked",
status: "lost",
}),
}),
},
}),
},
});
renderComponent(<WarningMessage model={model} />);
const error = screen.getByRole("link", { name: "app blocked" });
expect(error).toHaveAttribute("href", "/models/eggman@external/sub-test");
await act(async () => {
await userEventWithTimers.hover(error);
vi.runAllTimers();
});
const tooltip = screen.getAllByRole("tooltip")[0];
expect(error).toHaveAttribute("href", "/models/eggman@external/sub-test");
const appError = within(tooltip).getByRole("link", {
name: "app blocked",
});
expect(appError).toHaveAttribute(
"href",
"/models/eggman@external/sub-test/app/calico",
);
const unitError = within(tooltip).getByRole("link", {
name: "unit blocked",
});
expect(unitError).toHaveAttribute(
"href",
"/models/eggman@external/sub-test/app/etcd/unit/etcd-0",
);
});

it("should display tooltip correctly when more than 5 messages are present", async () => {
const model = modelDataFactory.build({
applications: [1, 2, 3, 4, 5, 6].reduce(
(applications, index) => ({
...applications,
[`app${index}`]: modelDataApplicationFactory.build({
status: detailedStatusFactory.build({
info: `app${index} blocked`,
status: "blocked",
}),
}),
}),
{},
),
});
renderComponent(<WarningMessage model={model} />);
const error = screen.getByRole("link", { name: "app1 blocked" });
expect(error).toHaveAttribute("href", "/models/eggman@external/sub-test");
await act(async () => {
await userEventWithTimers.hover(error);
vi.runAllTimers();
});
const tooltip = screen.getAllByRole("tooltip")[0];
expect(error).toHaveAttribute("href", "/models/eggman@external/sub-test");
[1, 2, 3, 4, 5].forEach((index) => {
const appError = within(tooltip).getByRole("link", {
name: `app${index} blocked`,
});
expect(appError).toHaveAttribute(
"href",
`/models/eggman@external/sub-test/app/app${index}`,
);
});
expect(within(tooltip).getByText("+1 more...")).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { List, Tooltip } from "@canonical/react-components";
import type { ListItem } from "@canonical/react-components/dist/components/List/List";
import { Link } from "react-router-dom";

import ModelDetailsLink from "components/ModelDetailsLink";
import type { ModelData } from "store/juju/types";
import { getModelStatusGroupData } from "store/juju/utils/models";
import urls from "urls";
import { getUserName } from "utils";

type Props = {
/**
* The full model data.
*/
model: ModelData;
};

/**
Warning message for the model name cell.
@return The react component for the warning message.
*/
const WarningMessage = ({ model }: Props) => {
const { messages } = getModelStatusGroupData(model);
if (!messages.length) {
return null;
}
const ownerTag = model?.info?.["owner-tag"] ?? "";
const userName = getUserName(ownerTag);
const modelName = model.model.name;
const link = (
<ModelDetailsLink modelName={modelName} ownerTag={ownerTag}>
{messages[0].message}
</ModelDetailsLink>
);
const list: ListItem[] = messages.slice(0, 5).map((message) => {
const unitId = message.unitId ? message.unitId.replace("/", "-") : null;
const appName = message.appName;
return {
className: "u-truncate",
content: (
<>
{unitId || appName}:{" "}
<Link
to={
unitId
? urls.model.unit({ userName, modelName, appName, unitId })
: urls.model.app.index({ userName, modelName, appName })
}
>
{message.message}
</Link>
</>
),
};
});
const remainder = messages.slice(5);
if (remainder.length) {
list.push(`+${remainder.length} more...`);
}
return (
<Tooltip
positionElementClassName="p-tooltip__position-element--inline"
tooltipClassName="p-tooltip--constrain-width"
message={<List className="u-no-margin--bottom" items={list} />}
>
<span className="model-table-list_error-message">{link}</span>
</Tooltip>
);
};

export default WarningMessage;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from "./WarningMessage";

0 comments on commit 16a7c52

Please sign in to comment.