Skip to content

Commit

Permalink
Track processing state so that the update-your-selection notice doesn…
Browse files Browse the repository at this point in the history
…'t occasionally appear when machines change what actions are available to them.
  • Loading branch information
huwshimi committed May 1, 2020
1 parent ff5717c commit 4a9b3da
Show file tree
Hide file tree
Showing 25 changed files with 519 additions and 91 deletions.
8 changes: 7 additions & 1 deletion ui/src/app/base/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,13 @@ export const useFormikErrors = (errors) => {
!simpleObjectEquality(errors, previousErrors)
) {
Object.keys(errors).forEach((field) => {
setFieldError(field, errors[field].join(" "));
let errorString;
if (Array.isArray(errors[field])) {
errorString = errors[field].join(" ");
} else {
errorString = errors[field];
}
setFieldError(field, errorString);
setFieldTouched(field, true, false);
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useDispatch, useSelector } from "react-redux";
import pluralize from "pluralize";
import React, { useState } from "react";
import PropTypes from "prop-types";
import React from "react";

import { machine as machineActions } from "app/base/actions";
import { machine as machineSelectors } from "app/base/selectors";
Expand Down Expand Up @@ -103,9 +104,13 @@ const useSelectedProcessing = (actionName) => {
return useSelector(selector);
};

export const ActionForm = ({ selectedAction, setSelectedAction }) => {
export const ActionForm = ({
processing,
setProcessing,
selectedAction,
setSelectedAction,
}) => {
const dispatch = useDispatch();
const [processing, setProcessing] = useState(false);
const selectedMachines = useSelector(machineSelectors.selected);
const saved = useSelector(machineSelectors.saved);
const saving = useSelector(machineSelectors.saving);
Expand Down Expand Up @@ -157,4 +162,10 @@ export const ActionForm = ({ selectedAction, setSelectedAction }) => {
);
};

ActionForm.propTypes = {
processing: PropTypes.bool,
setProcessing: PropTypes.func.isRequired,
setSelectedAction: PropTypes.func.isRequired,
};

export default ActionForm;
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "release" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
Expand All @@ -80,6 +81,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "release" }}
setProcessing={jest.fn()}
setSelectedAction={setSelectedAction}
/>
</MemoryRouter>
Expand All @@ -103,6 +105,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "delete" }}
setProcessing={jest.fn()}
setSelectedAction={setSelectedAction}
/>
</MemoryRouter>
Expand All @@ -123,6 +126,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "abort" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
Expand Down Expand Up @@ -162,6 +166,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "acquire" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
Expand Down Expand Up @@ -201,6 +206,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "delete" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
Expand Down Expand Up @@ -242,6 +248,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "exit-rescue-mode" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
Expand Down Expand Up @@ -283,6 +290,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "lock" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
Expand Down Expand Up @@ -322,6 +330,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "mark-broken" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
Expand Down Expand Up @@ -361,6 +370,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "mark-fixed" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
Expand Down Expand Up @@ -400,6 +410,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "off" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
Expand Down Expand Up @@ -439,6 +450,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "on" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
Expand Down Expand Up @@ -478,6 +490,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "release" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
Expand Down Expand Up @@ -517,6 +530,7 @@ describe("ActionForm", () => {
>
<ActionForm
selectedAction={{ name: "unlock" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
Expand Down Expand Up @@ -555,14 +569,37 @@ describe("ActionForm", () => {
initialEntries={[{ pathname: "/machines", key: "testKey" }]}
>
<ActionForm
processing={true}
selectedAction={{ name: "unlock" }}
setProcessing={jest.fn()}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
</Provider>
);
act(() => wrapper.find("Formik").props().onSubmit());
wrapper.update();
expect(wrapper.find("MachinesProcessing").exists()).toBe(true);
});

it("can set the processing state when successfully submitting", () => {
const state = { ...initialState };
state.machine.items = [{ system_id: "abc123", actions: ["unlock"] }];
state.machine.selected = ["abc123"];
const store = mockStore(state);
const setProcessing = jest.fn();
const wrapper = mount(
<Provider store={store}>
<MemoryRouter
initialEntries={[{ pathname: "/machines", key: "testKey" }]}
>
<ActionForm
selectedAction={{ name: "unlock" }}
setProcessing={setProcessing}
setSelectedAction={jest.fn()}
/>
</MemoryRouter>
</Provider>
);
act(() => wrapper.find("Formik").props().onSubmit());
expect(setProcessing).toHaveBeenCalledWith(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ exports[`ActionForm renders 1`] = `
"name": "release",
}
}
setProcessing={[MockFunction]}
setSelectedAction={[MockFunction]}
>
<FormikForm
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Button, Col, Row } from "@canonical/react-components";
import pluralize from "pluralize";
import PropTypes from "prop-types";
import React, { useLayoutEffect, useEffect, useState } from "react";
import React, { useEffect, useState } from "react";
import { useDispatch, useSelector } from "react-redux";

import { machine as machineActions } from "app/base/actions";
Expand Down Expand Up @@ -38,25 +38,30 @@ const getErrorSentence = (action, count) => {
}
};

export const ActionFormWrapper = ({ selectedAction, setSelectedAction }) => {
export const ActionFormWrapper = ({
selectedAction,
setSelectedAction,
_processing = false,
}) => {
const dispatch = useDispatch();

// Initialise the processing state from a prop to allow testing the state change.
const [processing, setProcessing] = useState(_processing);
const selectedMachines = useSelector(machineSelectors.selected);

const [actionableMachines, setActionableMachines] = useState([]);
const actionDisabled = actionableMachines.length !== selectedMachines.length;

useLayoutEffect(() => {
if (selectedAction) {
const actionable = selectedMachines.filter((machine) =>
machine.actions.includes(selectedAction.name)
);
setActionableMachines(actionable);
}
}, [selectedAction, selectedMachines]);
let actionableMachines = [];
if (selectedAction) {
actionableMachines = selectedMachines.filter((machine) =>
machine.actions.includes(selectedAction.name)
);
}
// The action should be disabled if not all the selected machines can perform
// The selected action. When machines are processing the available actions
// can change, so the action should not be disabled while processing.
const actionDisabled =
!processing && actionableMachines.length !== selectedMachines.length;

useEffect(() => {
if (selectedMachines.length === 0) {
// All the machines were deselected so close the form.
setSelectedAction(null);
}
}, [selectedMachines, setSelectedAction]);
Expand All @@ -65,22 +70,66 @@ export const ActionFormWrapper = ({ selectedAction, setSelectedAction }) => {
if (selectedAction && selectedAction.name) {
switch (selectedAction.name) {
case "commission":
return <CommissionForm setSelectedAction={setSelectedAction} />;
return (
<CommissionForm
processing={processing}
setProcessing={setProcessing}
setSelectedAction={setSelectedAction}
/>
);
case "deploy":
return <DeployForm setSelectedAction={setSelectedAction} />;
return (
<DeployForm
processing={processing}
setProcessing={setProcessing}
setSelectedAction={setSelectedAction}
/>
);
case "override-failed-testing":
return <OverrideTestForm setSelectedAction={setSelectedAction} />;
return (
<OverrideTestForm
processing={processing}
setProcessing={setProcessing}
setSelectedAction={setSelectedAction}
/>
);
case "set-pool":
return <SetPoolForm setSelectedAction={setSelectedAction} />;
return (
<SetPoolForm
processing={processing}
setProcessing={setProcessing}
setSelectedAction={setSelectedAction}
/>
);
case "set-zone":
return <SetZoneForm setSelectedAction={setSelectedAction} />;
return (
<SetZoneForm
processing={processing}
setProcessing={setProcessing}
setSelectedAction={setSelectedAction}
/>
);
case "tag":
return <TagForm setSelectedAction={setSelectedAction} />;
return (
<TagForm
processing={processing}
setProcessing={setProcessing}
setSelectedAction={setSelectedAction}
/>
);
case "test":
return <TestForm setSelectedAction={setSelectedAction} />;
return (
<TestForm
processing={processing}
setProcessing={setProcessing}
setSelectedAction={setSelectedAction}
/>
);
default:
return (
<ActionForm
processing={processing}
setProcessing={setProcessing}
selectedAction={selectedAction}
setSelectedAction={setSelectedAction}
/>
Expand Down
Loading

0 comments on commit 4a9b3da

Please sign in to comment.