Skip to content

Commit

Permalink
WD-8528 - feat: Display errors in Actions Panel (#1691)
Browse files Browse the repository at this point in the history
  • Loading branch information
vladimir-cucu authored Feb 2, 2024
1 parent aa059d2 commit 0559f36
Show file tree
Hide file tree
Showing 6 changed files with 195 additions and 32 deletions.
21 changes: 19 additions & 2 deletions src/panels/ActionsPanel/ActionsPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ describe("ActionsPanel", () => {
).not.toBeInTheDocument();
});

it("should display console error when trying to get actions for application", async () => {
it("should display error when trying to get actions for application", async () => {
jest
.spyOn(juju, "getActionsForApplication")
.mockImplementation(
Expand All @@ -323,16 +323,28 @@ describe("ActionsPanel", () => {
),
);
renderComponent(<ActionsPanel />, { path, url, state });
expect(juju.getActionsForApplication).toHaveBeenCalledTimes(1);
await waitFor(() =>
expect(juju.getActionsForApplication).toHaveBeenCalledTimes(1),
);
expect(console.error).toHaveBeenCalledWith(
Label.GET_ACTIONS_ERROR,
new Error("Error while trying to get actions for application!"),
);
await waitFor(() =>
expect(
screen.getByText(new RegExp(Label.GET_ACTIONS_ERROR)),
).toBeInTheDocument(),
);
await userEvent.click(
screen.getByRole("button", {
name: "refetching",
}),
);
expect(juju.getActionsForApplication).toHaveBeenCalledTimes(2);
});

it("should display console error when trying to submit the action request", async () => {
it("should display error when trying to submit the action request", async () => {
jest
.spyOn(juju, "executeActionOnUnits")
.mockImplementation(
Expand Down Expand Up @@ -363,5 +375,10 @@ describe("ActionsPanel", () => {
Label.EXECUTE_ACTION_ERROR,
new Error("Error while trying to execute action on units!"),
);
await waitFor(() =>
expect(
screen.getByText(new RegExp(Label.EXECUTE_ACTION_ERROR)),
).toBeInTheDocument(),
);
});
});
111 changes: 93 additions & 18 deletions src/panels/ActionsPanel/ActionsPanel.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import type { ActionSpec } from "@canonical/jujulib/dist/api/facades/action/ActionV7";
import { Button, ConfirmationModal } from "@canonical/react-components";
import {
ActionButton,
Button,
ConfirmationModal,
} from "@canonical/react-components";
import type { MutableRefObject } from "react";
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { useSelector } from "react-redux";
Expand All @@ -12,6 +16,7 @@ import Panel from "components/Panel";
import RadioInputBox from "components/RadioInputBox/RadioInputBox";
import type { EntityDetailsRoute } from "components/Routes/Routes";
import { executeActionOnUnits, getActionsForApplication } from "juju/api";
import PanelInlineErrors from "panels/PanelInlineErrors";
import { usePanelQueryParams } from "panels/hooks";
import type { ConfirmTypes } from "panels/types";
import { getModelUUID } from "store/juju/selectors";
Expand All @@ -26,8 +31,8 @@ export enum Label {
CONFIRM_BUTTON = "Confirm",
NO_UNITS_SELECTED = "0 units selected",
NO_ACTIONS_PROVIDED = "This charm has not provided any actions.",
GET_ACTIONS_ERROR = "Error while trying to get actions for application.",
EXECUTE_ACTION_ERROR = "Error while trying to execute action on units.",
GET_ACTIONS_ERROR = "Unable to get actions for application.",
EXECUTE_ACTION_ERROR = "Couldn't start the action.",
}

export enum TestId {
Expand Down Expand Up @@ -101,6 +106,14 @@ export default function ActionsPanel(): JSX.Element {
string | undefined,
SetSelectedAction,
] = useState<string>();
// First element in inLineErrors array corresponds to the get actions for
// application error. Second element corresponds to execute action error.
const [inlineErrors, setInlineErrors] = useState<(string | null)[]>([
null,
null,
]);
const [isExecutingAction, setIsExecutingAction] = useState<boolean>(false);
const scrollArea = useRef<HTMLDivElement>(null);
const { Portal } = usePortal();

const actionOptionsValues = useRef<ActionOptionValues>({});
Expand All @@ -110,34 +123,53 @@ export default function ActionsPanel(): JSX.Element {
usePanelQueryParams<ActionsQueryParams>(defaultQueryParams);
const selectedUnits = queryParams.units;

useEffect(() => {
const getActionsForApplicationCallback = useCallback(() => {
setFetchingActionData(true);
if (appName && modelUUID) {
// eslint-disable-next-line promise/catch-or-return
getActionsForApplication(appName, modelUUID, appStore.getState())
.then((actions) => {
if (actions?.results?.[0]?.actions) {
setActionData(actions.results[0].actions);
}
setFetchingActionData(false);
setInlineErrors((prevInlineErrors) => {
const newInlineErrors = [...prevInlineErrors];
newInlineErrors[0] = null;
return newInlineErrors;
});
return;
})
.catch((error) => console.error(Label.GET_ACTIONS_ERROR, error));
.catch((error) => {
setInlineErrors((prevInlineErrors) => {
const newInlineErrors = [...prevInlineErrors];
newInlineErrors[0] = Label.GET_ACTIONS_ERROR;
return newInlineErrors;
});
console.error(Label.GET_ACTIONS_ERROR, error);
})
.finally(() => {
setFetchingActionData(false);
});
}
}, [appName, appStore, modelUUID]);

useEffect(() => {
getActionsForApplicationCallback();
}, [getActionsForApplicationCallback]);

const namespace =
appName && modelUUID
? appState.juju?.modelData?.[modelUUID]?.applications?.[appName]?.charm
: null;

const generateSelectedUnitList = () => {
const generateSelectedUnitList = useCallback(() => {
if (!selectedUnits.length) {
return Label.NO_UNITS_SELECTED;
}
return selectedUnits.reduce((acc, unitName) => {
return `${acc}, ${unitName.split("/")[1]}`;
});
};
}, [selectedUnits]);

const generateTitle = () => {
const unitLength = selectedUnits.length;
Expand Down Expand Up @@ -211,12 +243,28 @@ export default function ActionsPanel(): JSX.Element {
cancelButtonLabel={Label.CANCEL_BUTTON}
confirmButtonLabel={Label.CONFIRM_BUTTON}
confirmButtonAppearance="positive"
onConfirm={() => {
onConfirm={(event: React.MouseEvent<HTMLElement, MouseEvent>) => {
// Stop propagation of the click event in order for the Panel
// to remain open after an error occurs in executeAction().
// Remove this manual fix once this issue gets resolved:
// https://github.com/canonical/react-components/issues/1032
event.nativeEvent.stopImmediatePropagation();
setConfirmType(null);
executeAction().catch((error) =>
console.error(Label.EXECUTE_ACTION_ERROR, error),
);
handleRemovePanelQueryParams();
setIsExecutingAction(true);
executeAction()
.then(() => {
handleRemovePanelQueryParams();
return;
})
.catch((error) => {
setInlineErrors((prevInlineError) => {
const newInlineErrors = [...prevInlineError];
newInlineErrors[1] = Label.EXECUTE_ACTION_ERROR;
return newInlineErrors;
});
setIsExecutingAction(false);
console.error(Label.EXECUTE_ACTION_ERROR, error);
});
}}
close={() => setConfirmType(null)}
>
Expand All @@ -242,26 +290,53 @@ export default function ActionsPanel(): JSX.Element {
return (
<Panel
drawer={
<Button
<ActionButton
appearance="positive"
disabled={disableSubmit}
loading={isExecutingAction}
disabled={disableSubmit || isExecutingAction}
onClick={handleSubmit}
>
Run action
</Button>
</ActionButton>
}
width="narrow"
data-testid={TestId.PANEL}
title={generateTitle()}
onRemovePanelQueryParams={handleRemovePanelQueryParams}
ref={scrollArea}
>
<PanelInlineErrors
inlineErrors={
inlineErrors[0]
? inlineErrors.map((error, index) =>
index === 0 ? (
// If get actions for application fails, we add a button for
// refetching the actions data to the first inline error.
<>
{error} Try{" "}
<Button
appearance="link"
onClick={() => getActionsForApplicationCallback()}
>
refetching
</Button>{" "}
the actions data.
</>
) : (
error
),
)
: inlineErrors
}
scrollArea={scrollArea.current}
/>
<p data-testid="actions-panel-unit-list">
Run action on: {generateSelectedUnitList()}
</p>
<LoadingHandler
hasData={data ? true : false}
hasData={!!data}
loading={fetchingActionData}
noDataMessage={Label.NO_ACTIONS_PROVIDED}
noDataMessage={inlineErrors[0] ? "" : Label.NO_ACTIONS_PROVIDED}
>
{Object.keys(actionData).map((actionName) => (
<RadioInputBox
Expand Down
18 changes: 6 additions & 12 deletions src/panels/ConfigPanel/ConfigPanel.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {
Button,
ConfirmationModal,
Notification,
Spinner,
} from "@canonical/react-components";
import classnames from "classnames";
Expand All @@ -14,11 +13,11 @@ import type { Store } from "redux";
import FadeIn from "animations/FadeIn";
import CharmIcon from "components/CharmIcon";
import Panel from "components/Panel";
import ScrollOnRender from "components/ScrollOnRender";
import { isSet } from "components/utils";
import useAnalytics from "hooks/useAnalytics";
import type { Config, ConfigData, ConfigValue } from "juju/api";
import { getApplicationConfig, setApplicationConfig } from "juju/api";
import PanelInlineErrors from "panels/PanelInlineErrors";
import { usePanelQueryParams } from "panels/hooks";
import type { ConfirmTypes } from "panels/types";
import bulbImage from "static/images/bulb.svg";
Expand All @@ -29,6 +28,7 @@ import BooleanConfig from "./BooleanConfig";
import type { SetNewValue, SetSelectedConfig } from "./ConfigField";
import NumberConfig from "./NumberConfig";
import TextAreaConfig from "./TextAreaConfig";

import "./_config-panel.scss";

export enum Label {
Expand Down Expand Up @@ -57,13 +57,6 @@ type ConfigQueryParams = {
modelUUID: string | null;
};

const generateErrors = (errors: string[], scrollArea?: HTMLElement | null) =>
errors.map((error) => (
<ScrollOnRender key={error} scrollArea={scrollArea}>
<Notification severity="negative">{error}</Notification>
</ScrollOnRender>
));

export default function ConfigPanel(): JSX.Element {
const reduxStore = useAppStore();
const [config, setConfig] = useState<Config>({});
Expand Down Expand Up @@ -391,9 +384,10 @@ export default function ConfigPanel(): JSX.Element {
</div>

<div className="config-panel__list">
{formErrors
? generateErrors(formErrors, scrollArea?.current)
: null}
<PanelInlineErrors
inlineErrors={formErrors}
scrollArea={scrollArea.current}
/>
{generateConfigElementList(
config,
selectedConfig,
Expand Down
49 changes: 49 additions & 0 deletions src/panels/PanelInlineErrors/PanelInlineErrors.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { render } from "@testing-library/react";

import PanelInlineErrors from "./PanelInlineErrors";

describe("PanelInlineErrors", () => {
it("shouldn't render if there is no inline error", () => {
render(<PanelInlineErrors inlineErrors={[null]} />);
const notifications = document.getElementsByClassName(
"p-notification__message",
);
expect(notifications).toHaveLength(0);
});

it("should render inline errors", () => {
render(
<PanelInlineErrors
inlineErrors={["Error 1", <button>Error 2</button>]}
/>,
);
const notifications = document.getElementsByClassName(
"p-notification__message",
);
expect(notifications).toHaveLength(2);
expect(notifications[0]).toHaveTextContent(/Error 1/);
expect(notifications[1]).toHaveTextContent(/Error 2/);
});

it("shouldn't scroll on render in there is no inline error", () => {
render(
<PanelInlineErrors
inlineErrors={[null]}
scrollArea={document.createElement("div")}
/>,
);
expect(window.HTMLElement.prototype.scrollIntoView).not.toHaveBeenCalled();
});

it("should scroll on render if there are inline errors", () => {
render(
<PanelInlineErrors
inlineErrors={[<button>Error</button>]}
scrollArea={document.createElement("div")}
/>,
);
expect(window.HTMLElement.prototype.scrollIntoView).toHaveBeenCalledWith({
behavior: "smooth",
});
});
});
27 changes: 27 additions & 0 deletions src/panels/PanelInlineErrors/PanelInlineErrors.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { Notification } from "@canonical/react-components";
import type { ReactNode } from "react";

import ScrollOnRender from "components/ScrollOnRender";

type Props = {
inlineErrors: ReactNode[] | null;
scrollArea?: HTMLElement | null;
};

const PanelInlineErrors = ({
inlineErrors,
scrollArea,
}: Props): JSX.Element | null =>
inlineErrors && inlineErrors.some((error) => error) ? (
<ScrollOnRender scrollArea={scrollArea}>
{inlineErrors.map((error, index) =>
error ? (
<Notification key={index} severity="negative">
{error}
</Notification>
) : null,
)}
</ScrollOnRender>
) : null;

export default PanelInlineErrors;
1 change: 1 addition & 0 deletions src/panels/PanelInlineErrors/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from "./PanelInlineErrors";

0 comments on commit 0559f36

Please sign in to comment.