From 7d927d2e0a9fbd52802381c737b57c87e14dc4cd Mon Sep 17 00:00:00 2001 From: Huw Wilkins Date: Fri, 1 May 2020 14:59:23 +1000 Subject: [PATCH 1/2] Track processing state so that the update-your-selection notice doesn't occasionally appear when machines change what actions are available to them. --- ui/src/app/base/hooks.js | 8 +- .../ActionForm/ActionForm.js | 17 +++- .../ActionForm/ActionForm.test.js | 41 +++++++- .../__snapshots__/ActionForm.test.js.snap | 1 + .../ActionFormWrapper/ActionFormWrapper.js | 93 ++++++++++++++----- .../ActionFormWrapper.test.js | 67 +++++++++++++ .../CommissionForm/CommissionForm.js | 16 +++- .../CommissionForm/CommissionForm.test.js | 34 ++++++- .../CommissionFormFields.test.js | 5 +- .../DeployForm/DeployForm.js | 16 +++- .../DeployForm/DeployForm.test.js | 31 ++++++- .../DeployFormFields/DeployFormFields.test.js | 12 +-- .../OverrideTestForm/OverrideTestForm.js | 16 +++- .../OverrideTestForm/OverrideTestForm.test.js | 59 ++++++++++-- .../SetPoolForm/SetPoolForm.js | 14 ++- .../SetPoolForm/SetPoolForm.test.js | 33 ++++++- .../SetPoolFormFields.test.js | 10 +- .../SetZoneForm/SetZoneForm.js | 16 +++- .../SetZoneForm/SetZoneForm.test.js | 33 ++++++- .../ActionFormWrapper/TagForm/TagForm.js | 10 +- .../ActionFormWrapper/TagForm/TagForm.test.js | 31 ++++++- .../TagFormFields/TagFormFields.test.js | 2 +- .../ActionFormWrapper/TestForm/TestForm.js | 12 ++- .../TestForm/TestForm.test.js | 31 ++++++- .../TestFormFields/TestFormFields.test.js | 2 +- 25 files changed, 519 insertions(+), 91 deletions(-) diff --git a/ui/src/app/base/hooks.js b/ui/src/app/base/hooks.js index f824588219..5f9bacaf86 100644 --- a/ui/src/app/base/hooks.js +++ b/ui/src/app/base/hooks.js @@ -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); }); } diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionForm/ActionForm.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionForm/ActionForm.js index dcb4b2c826..a04c39b26a 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionForm/ActionForm.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionForm/ActionForm.js @@ -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"; @@ -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); @@ -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; diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionForm/ActionForm.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionForm/ActionForm.test.js index 58175187ce..5a79b1c2e5 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionForm/ActionForm.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionForm/ActionForm.test.js @@ -58,6 +58,7 @@ describe("ActionForm", () => { > @@ -80,6 +81,7 @@ describe("ActionForm", () => { > @@ -103,6 +105,7 @@ describe("ActionForm", () => { > @@ -123,6 +126,7 @@ describe("ActionForm", () => { > @@ -162,6 +166,7 @@ describe("ActionForm", () => { > @@ -201,6 +206,7 @@ describe("ActionForm", () => { > @@ -242,6 +248,7 @@ describe("ActionForm", () => { > @@ -283,6 +290,7 @@ describe("ActionForm", () => { > @@ -322,6 +330,7 @@ describe("ActionForm", () => { > @@ -361,6 +370,7 @@ describe("ActionForm", () => { > @@ -400,6 +410,7 @@ describe("ActionForm", () => { > @@ -439,6 +450,7 @@ describe("ActionForm", () => { > @@ -478,6 +490,7 @@ describe("ActionForm", () => { > @@ -517,6 +530,7 @@ describe("ActionForm", () => { > @@ -555,14 +569,37 @@ describe("ActionForm", () => { initialEntries={[{ pathname: "/machines", key: "testKey" }]} > ); - 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( + + + + + + ); + act(() => wrapper.find("Formik").props().onSubmit()); + expect(setProcessing).toHaveBeenCalledWith(true); + }); }); diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionForm/__snapshots__/ActionForm.test.js.snap b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionForm/__snapshots__/ActionForm.test.js.snap index 30cff0f756..e887c34b90 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionForm/__snapshots__/ActionForm.test.js.snap +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionForm/__snapshots__/ActionForm.test.js.snap @@ -7,6 +7,7 @@ exports[`ActionForm renders 1`] = ` "name": "release", } } + setProcessing={[MockFunction]} setSelectedAction={[MockFunction]} > { } }; -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]); @@ -65,22 +70,66 @@ export const ActionFormWrapper = ({ selectedAction, setSelectedAction }) => { if (selectedAction && selectedAction.name) { switch (selectedAction.name) { case "commission": - return ; + return ( + + ); case "deploy": - return ; + return ( + + ); case "override-failed-testing": - return ; + return ( + + ); case "set-pool": - return ; + return ( + + ); case "set-zone": - return ; + return ( + + ); case "tag": - return ; + return ( + + ); case "test": - return ; + return ( + + ); default: return ( diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionFormWrapper.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionFormWrapper.test.js index b03f46db33..daf29adc0d 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionFormWrapper.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionFormWrapper.test.js @@ -1,3 +1,4 @@ +import { act } from "react-dom/test-utils"; import { MemoryRouter } from "react-router-dom"; import { mount } from "enzyme"; import { Provider } from "react-redux"; @@ -12,9 +13,46 @@ describe("ActionFormWrapper", () => { let initialState; beforeEach(() => { initialState = { + general: { + machineActions: { + data: [{ name: "commission", sentence: "commission" }], + }, + }, machine: { items: [], selected: [], + statuses: { a: {}, b: {} }, + }, + scripts: { + errors: {}, + loading: false, + loaded: true, + items: [ + { + name: "smartctl-validate", + tags: ["commissioning", "storage"], + parameters: { + storage: { + argument_format: "{path}", + type: "storage", + }, + }, + type: 2, + }, + { + name: "internet-connectivity", + tags: ["internet", "network-validation", "network"], + parameters: { + url: { + default: "https://connectivity-check.ubuntu.com", + description: + "A comma seperated list of URLs, IPs, or domains to test if the specified interface has access to. Any protocol supported by curl is support. If no protocol or icmp is given the URL will be pinged.", + required: true, + }, + }, + type: 2, + }, + ], }, }; }); @@ -45,6 +83,35 @@ describe("ActionFormWrapper", () => { ); }); + it(`does not display a warning when processing and not all selected machines + can perform selected action`, () => { + const state = { ...initialState }; + state.machine.items = [ + { system_id: "a", actions: ["commission"] }, + { system_id: "b", actions: [] }, + ]; + state.machine.selected = ["a", "b"]; + const store = mockStore(state); + let wrapper = mount( + + + + + , + { context: store } + ); + // act(() => wrapper.find("CommissionForm").props().setProcessing(true)); + expect(wrapper.find("[data-test='machine-action-warning']").exists()).toBe( + false + ); + }); + it("can set selected machines to those that can perform action", () => { const state = { ...initialState }; state.machine.items = [ diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/CommissionForm/CommissionForm.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/CommissionForm/CommissionForm.js index fed82d9afa..ddcb30f240 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/CommissionForm/CommissionForm.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/CommissionForm/CommissionForm.js @@ -1,5 +1,6 @@ import pluralize from "pluralize"; -import React, { useEffect, useState } from "react"; +import PropTypes from "prop-types"; +import React, { useEffect } from "react"; import { useDispatch, useSelector } from "react-redux"; import * as Yup from "yup"; @@ -37,9 +38,12 @@ const CommissionFormSchema = Yup.object().shape({ .required(), }); -export const CommissionForm = ({ setSelectedAction }) => { +export const CommissionForm = ({ + processing, + setProcessing, + setSelectedAction, +}) => { const dispatch = useDispatch(); - const [processing, setProcessing] = useState(false); const selectedMachines = useSelector(machineSelectors.selected); const saved = useSelector(machineSelectors.saved); const saving = useSelector(machineSelectors.saving); @@ -164,4 +168,10 @@ export const CommissionForm = ({ setSelectedAction }) => { ); }; +CommissionForm.propTypes = { + processing: PropTypes.bool, + setProcessing: PropTypes.func.isRequired, + setSelectedAction: PropTypes.func.isRequired, +}; + export default CommissionForm; diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/CommissionForm/CommissionForm.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/CommissionForm/CommissionForm.test.js index cb73ac753e..f416905ed5 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/CommissionForm/CommissionForm.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/CommissionForm/CommissionForm.test.js @@ -64,7 +64,10 @@ describe("CommissionForm", () => { - + ); @@ -154,7 +157,31 @@ describe("CommissionForm", () => { - + + + + ); + expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + }); + + it("can set the processing state when successfully submitting", () => { + const state = { ...initialState }; + state.machine.selected = ["abc123", "def456"]; + const store = mockStore(state); + const setProcessing = jest.fn(); + const wrapper = mount( + + + ); @@ -173,7 +200,6 @@ describe("CommissionForm", () => { commissioningScripts: [state.scripts.items[1]], }) ); - wrapper.update(); - expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + expect(setProcessing).toHaveBeenCalledWith(true); }); }); diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/CommissionForm/CommissionFormFields/CommissionFormFields.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/CommissionForm/CommissionFormFields/CommissionFormFields.test.js index 4eebe2ef86..8f3f8691e4 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/CommissionForm/CommissionFormFields/CommissionFormFields.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/CommissionForm/CommissionFormFields/CommissionFormFields.test.js @@ -66,7 +66,10 @@ describe("CommissionForm", () => { - + ); diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/DeployForm/DeployForm.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/DeployForm/DeployForm.js index 330a601714..28cc0d52dd 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/DeployForm/DeployForm.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/DeployForm/DeployForm.js @@ -1,7 +1,8 @@ import { useDispatch, useSelector } from "react-redux"; import * as Yup from "yup"; import pluralize from "pluralize"; -import React, { useEffect, useState } from "react"; +import PropTypes from "prop-types"; +import React, { useEffect } from "react"; import { general as generalActions, @@ -23,9 +24,12 @@ const DeploySchema = Yup.object().shape({ installKVM: Yup.boolean(), }); -export const DeployForm = ({ setSelectedAction }) => { +export const DeployForm = ({ + processing, + setProcessing, + setSelectedAction, +}) => { const dispatch = useDispatch(); - const [processing, setProcessing] = useState(false); const selectedMachines = useSelector(machineSelectors.selected); const saved = useSelector(machineSelectors.saved); const saving = useSelector(machineSelectors.saving); @@ -97,4 +101,10 @@ export const DeployForm = ({ setSelectedAction }) => { ); }; +DeployForm.propTypes = { + processing: PropTypes.bool, + setProcessing: PropTypes.func.isRequired, + setSelectedAction: PropTypes.func.isRequired, +}; + export default DeployForm; diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/DeployForm/DeployForm.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/DeployForm/DeployForm.test.js index 32c52237ac..b7f85c9fb1 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/DeployForm/DeployForm.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/DeployForm/DeployForm.test.js @@ -128,7 +128,7 @@ describe("DeployForm", () => { - + ); @@ -194,7 +194,31 @@ describe("DeployForm", () => { - + + + + ); + expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + }); + + it("can set the processing state when successfully submitting", () => { + const state = { ...initialState }; + state.machine.selected = ["abc123", "def456"]; + const store = mockStore(state); + const setProcessing = jest.fn(); + const wrapper = mount( + + + ); @@ -206,7 +230,6 @@ describe("DeployForm", () => { installKVM: false, }) ); - wrapper.update(); - expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + expect(setProcessing).toHaveBeenCalledWith(true); }); }); diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/DeployForm/DeployFormFields/DeployFormFields.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/DeployForm/DeployFormFields/DeployFormFields.test.js index 57db07c481..70133e5da7 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/DeployForm/DeployFormFields/DeployFormFields.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/DeployForm/DeployFormFields/DeployFormFields.test.js @@ -124,7 +124,7 @@ describe("DeployFormFields", () => { - + ); @@ -140,7 +140,7 @@ describe("DeployFormFields", () => { - + ); @@ -156,7 +156,7 @@ describe("DeployFormFields", () => { - + ); @@ -173,7 +173,7 @@ describe("DeployFormFields", () => { - + ); @@ -202,7 +202,7 @@ describe("DeployFormFields", () => { - + ); @@ -240,7 +240,7 @@ describe("DeployFormFields", () => { - + ); diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/OverrideTestForm/OverrideTestForm.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/OverrideTestForm/OverrideTestForm.js index 11270fd85b..29f4c6dd3c 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/OverrideTestForm/OverrideTestForm.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/OverrideTestForm/OverrideTestForm.js @@ -1,6 +1,7 @@ import { Col, Row, Spinner } from "@canonical/react-components"; import pluralize from "pluralize"; -import React, { useEffect, useState } from "react"; +import PropTypes from "prop-types"; +import React, { useEffect } from "react"; import { useDispatch, useSelector } from "react-redux"; import * as Yup from "yup"; @@ -60,9 +61,12 @@ const OverrideTestFormSchema = Yup.object().shape({ suppressResults: Yup.boolean(), }); -export const OverrideTestForm = ({ setSelectedAction }) => { +export const OverrideTestForm = ({ + processing, + setProcessing, + setSelectedAction, +}) => { const dispatch = useDispatch(); - const [processing, setProcessing] = useState(false); const selectedMachines = useSelector(machineSelectors.selected); const saved = useSelector(machineSelectors.saved); const saving = useSelector(machineSelectors.saving); @@ -185,4 +189,10 @@ export const OverrideTestForm = ({ setSelectedAction }) => { ); }; +OverrideTestForm.propTypes = { + processing: PropTypes.bool, + setProcessing: PropTypes.func.isRequired, + setSelectedAction: PropTypes.func.isRequired, +}; + export default OverrideTestForm; diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/OverrideTestForm/OverrideTestForm.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/OverrideTestForm/OverrideTestForm.test.js index bab43f5fbe..8aa7f9a119 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/OverrideTestForm/OverrideTestForm.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/OverrideTestForm/OverrideTestForm.test.js @@ -65,7 +65,10 @@ describe("OverrideTestForm", () => { - + ); @@ -89,7 +92,10 @@ describe("OverrideTestForm", () => { - + ); @@ -111,7 +117,10 @@ describe("OverrideTestForm", () => { - + ); @@ -133,7 +142,10 @@ describe("OverrideTestForm", () => { - + ); @@ -152,7 +164,10 @@ describe("OverrideTestForm", () => { - + ); @@ -207,7 +222,10 @@ describe("OverrideTestForm", () => { - + ); @@ -247,7 +265,31 @@ describe("OverrideTestForm", () => { - + + + + ); + expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + }); + + it("can set the processing state when successfully submitting", () => { + const state = { ...initialState }; + state.machine.selected = ["abc123", "def456"]; + const store = mockStore(state); + const setProcessing = jest.fn(); + const wrapper = mount( + + + ); @@ -256,7 +298,6 @@ describe("OverrideTestForm", () => { suppressResults: true, }) ); - wrapper.update(); - expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + expect(setProcessing).toHaveBeenCalledWith(true); }); }); diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetPoolForm/SetPoolForm.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetPoolForm/SetPoolForm.js index 180a4d018c..07c3ff6e0e 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetPoolForm/SetPoolForm.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetPoolForm/SetPoolForm.js @@ -1,6 +1,7 @@ import { useDispatch, useSelector } from "react-redux"; import * as Yup from "yup"; import pluralize from "pluralize"; +import PropTypes from "prop-types"; import React, { useEffect, useState } from "react"; import { @@ -22,9 +23,12 @@ const SetPoolSchema = Yup.object().shape({ poolSelection: Yup.string().oneOf(["create", "select"]).required(), }); -export const SetPoolForm = ({ setSelectedAction }) => { +export const SetPoolForm = ({ + processing, + setProcessing, + setSelectedAction, +}) => { const dispatch = useDispatch(); - const [processing, setProcessing] = useState(false); const [initialValues, setInitialValues] = useState({ poolSelection: "select", description: "", @@ -102,4 +106,10 @@ export const SetPoolForm = ({ setSelectedAction }) => { ); }; +SetPoolForm.propTypes = { + processing: PropTypes.bool, + setProcessing: PropTypes.func.isRequired, + setSelectedAction: PropTypes.func.isRequired, +}; + export default SetPoolForm; diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetPoolForm/SetPoolForm.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetPoolForm/SetPoolForm.test.js index ba5e86039e..cb75d275a0 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetPoolForm/SetPoolForm.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetPoolForm/SetPoolForm.test.js @@ -54,7 +54,10 @@ describe("SetPoolForm", () => { - + ); @@ -110,7 +113,30 @@ describe("SetPoolForm", () => { - + + + + ); + expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + }); + + it("can set the processing state when successfully submitting", () => { + const store = mockStore(state); + state.machine.selected = ["abc123", "def456"]; + const setProcessing = jest.fn(); + const wrapper = mount( + + + ); @@ -121,7 +147,6 @@ describe("SetPoolForm", () => { description: "", }) ); - wrapper.update(); - expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + expect(setProcessing).toHaveBeenCalledWith(true); }); }); diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetPoolForm/SetPoolFormFields/SetPoolFormFields.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetPoolForm/SetPoolFormFields/SetPoolFormFields.test.js index 6c0b07f6b7..4b448811ff 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetPoolForm/SetPoolFormFields/SetPoolFormFields.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetPoolForm/SetPoolFormFields/SetPoolFormFields.test.js @@ -47,7 +47,10 @@ describe("SetPoolFormFields", () => { - + ); @@ -67,7 +70,10 @@ describe("SetPoolFormFields", () => { - + ); diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetZoneForm/SetZoneForm.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetZoneForm/SetZoneForm.js index b22f136025..521df56fbd 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetZoneForm/SetZoneForm.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetZoneForm/SetZoneForm.js @@ -2,7 +2,8 @@ import { Col, Row, Select } from "@canonical/react-components"; import { useDispatch, useSelector } from "react-redux"; import * as Yup from "yup"; 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 { @@ -18,9 +19,12 @@ const SetZoneSchema = Yup.object().shape({ zone: Yup.string().required("Zone is required"), }); -export const SetZoneForm = ({ setSelectedAction }) => { +export const SetZoneForm = ({ + processing, + setProcessing, + setSelectedAction, +}) => { const dispatch = useDispatch(); - const [processing, setProcessing] = useState(false); const selectedMachines = useSelector(machineSelectors.selected); const saved = useSelector(machineSelectors.saved); const saving = useSelector(machineSelectors.saving); @@ -91,4 +95,10 @@ export const SetZoneForm = ({ setSelectedAction }) => { ); }; +SetZoneForm.propTypes = { + processing: PropTypes.bool, + setProcessing: PropTypes.func.isRequired, + setSelectedAction: PropTypes.func.isRequired, +}; + export default SetZoneForm; diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetZoneForm/SetZoneForm.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetZoneForm/SetZoneForm.test.js index be531128ab..968be84968 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetZoneForm/SetZoneForm.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/SetZoneForm/SetZoneForm.test.js @@ -53,7 +53,10 @@ describe("SetZoneForm", () => { - + ); @@ -109,7 +112,30 @@ describe("SetZoneForm", () => { - + + + + ); + expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + }); + + it("can set the processing state when successfully submitting", () => { + const store = mockStore(state); + state.machine.selected = ["abc123", "def456"]; + const setProcessing = jest.fn(); + const wrapper = mount( + + + ); @@ -118,7 +144,6 @@ describe("SetZoneForm", () => { zone: "zone-1", }) ); - wrapper.update(); - expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + expect(setProcessing).toHaveBeenCalledWith(true); }); }); diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TagForm/TagForm.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TagForm/TagForm.js index 46a47f48a0..ff87546b01 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TagForm/TagForm.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TagForm/TagForm.js @@ -1,4 +1,5 @@ import pluralize from "pluralize"; +import PropTypes from "prop-types"; import React, { useState } from "react"; import { useDispatch, useSelector } from "react-redux"; import * as Yup from "yup"; @@ -22,9 +23,8 @@ const TagFormSchema = Yup.object().shape({ .required("You must select at least one tag."), }); -export const TagForm = ({ setSelectedAction }) => { +export const TagForm = ({ processing, setProcessing, setSelectedAction }) => { const dispatch = useDispatch(); - const [processing, setProcessing] = useState(false); const [initialValues, setInitialValues] = useState({ tags: [] }); const selectedMachines = useSelector(machineSelectors.selected); const saved = useSelector(machineSelectors.saved); @@ -85,4 +85,10 @@ export const TagForm = ({ setSelectedAction }) => { ); }; +TagForm.propTypes = { + processing: PropTypes.bool, + setProcessing: PropTypes.func.isRequired, + setSelectedAction: PropTypes.func.isRequired, +}; + export default TagForm; diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TagForm/TagForm.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TagForm/TagForm.test.js index 0712723db1..ff9afb9beb 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TagForm/TagForm.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TagForm/TagForm.test.js @@ -47,7 +47,7 @@ describe("TagForm", () => { - + ); @@ -107,7 +107,31 @@ describe("TagForm", () => { - + + + + ); + expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + }); + + it("can set the processing state when successfully submitting", () => { + const state = { ...initialState }; + state.machine.selected = ["abc123", "def456"]; + const store = mockStore(state); + const setProcessing = jest.fn(); + const wrapper = mount( + + + ); @@ -119,7 +143,6 @@ describe("TagForm", () => { tags: [{ name: "tag1" }, { name: "tag2" }], }) ); - wrapper.update(); - expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + expect(setProcessing).toHaveBeenCalledWith(true); }); }); diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TagForm/TagFormFields/TagFormFields.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TagForm/TagFormFields/TagFormFields.test.js index da56b53c55..b1079d4e04 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TagForm/TagFormFields/TagFormFields.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TagForm/TagFormFields/TagFormFields.test.js @@ -40,7 +40,7 @@ describe("TagFormFields", () => { - + ); diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TestForm/TestForm.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TestForm/TestForm.js index 6f8cd5264b..80423302d0 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TestForm/TestForm.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TestForm/TestForm.js @@ -1,5 +1,6 @@ import pluralize from "pluralize"; -import React, { useEffect, useState } from "react"; +import PropTypes from "prop-types"; +import React, { useEffect } from "react"; import { useDispatch, useSelector } from "react-redux"; import * as Yup from "yup"; @@ -31,9 +32,8 @@ const TestFormSchema = Yup.object().shape({ urls: Yup.object(), }); -export const TestForm = ({ setSelectedAction }) => { +export const TestForm = ({ processing, setProcessing, setSelectedAction }) => { const dispatch = useDispatch(); - const [processing, setProcessing] = useState(false); const selectedMachines = useSelector(machineSelectors.selected); const saved = useSelector(machineSelectors.saved); const saving = useSelector(machineSelectors.saving); @@ -120,4 +120,10 @@ export const TestForm = ({ setSelectedAction }) => { ); }; +TestForm.propTypes = { + processing: PropTypes.bool, + setProcessing: PropTypes.func.isRequired, + setSelectedAction: PropTypes.func.isRequired, +}; + export default TestForm; diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TestForm/TestForm.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TestForm/TestForm.test.js index c8a780f4db..6bdc3d3dbe 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TestForm/TestForm.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TestForm/TestForm.test.js @@ -72,7 +72,7 @@ describe("TestForm", () => { - + ); @@ -146,7 +146,31 @@ describe("TestForm", () => { - + + + + ); + expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + }); + + it("can set the processing state when successfully submitting", () => { + const state = { ...initialState }; + state.machine.selected = ["abc123", "def456"]; + const store = mockStore(state); + const setProcessing = jest.fn(); + const wrapper = mount( + + + ); @@ -162,7 +186,6 @@ describe("TestForm", () => { }, }) ); - wrapper.update(); - expect(wrapper.find("MachinesProcessing").exists()).toBe(true); + expect(setProcessing).toHaveBeenCalledWith(true); }); }); diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TestForm/TestFormFields/TestFormFields.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TestForm/TestFormFields/TestFormFields.test.js index 7fef88685a..5e39dcf161 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TestForm/TestFormFields/TestFormFields.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/TestForm/TestFormFields/TestFormFields.test.js @@ -66,7 +66,7 @@ describe("TestForm", () => { - + ); From 76be41b8c3dcc8f19b7021bb168ddacd743fa4e8 Mon Sep 17 00:00:00 2001 From: Huw Wilkins Date: Mon, 4 May 2020 12:02:35 +1000 Subject: [PATCH 2/2] Remove unused act. --- .../HeaderStrip/ActionFormWrapper/ActionFormWrapper.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionFormWrapper.test.js b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionFormWrapper.test.js index daf29adc0d..8baa20d66b 100644 --- a/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionFormWrapper.test.js +++ b/ui/src/app/machines/components/HeaderStrip/ActionFormWrapper/ActionFormWrapper.test.js @@ -1,4 +1,3 @@ -import { act } from "react-dom/test-utils"; import { MemoryRouter } from "react-router-dom"; import { mount } from "enzyme"; import { Provider } from "react-redux"; @@ -106,7 +105,6 @@ describe("ActionFormWrapper", () => { , { context: store } ); - // act(() => wrapper.find("CommissionForm").props().setProcessing(true)); expect(wrapper.find("[data-test='machine-action-warning']").exists()).toBe( false );