Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove power fields meant for individual nodes from add chassis form. #1088

Merged
merged 4 commits into from
May 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions ui/src/app/base/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,23 +241,31 @@ export const useVisible = (initialValue) => {
* @param {Function} generateSchemaFunc - Schema generation function.
* @returns {Object} Yup validation schema with power parameters.
*/
export const usePowerParametersSchema = (powerType, generateSchemaFunc) => {
export const usePowerParametersSchema = (
powerType,
generateSchemaFunc,
chassis = false
) => {
const [Schema, setSchema] = useState(generateSchemaFunc({}));

useEffect(() => {
if (powerType) {
const parametersSchema = powerType.fields.reduce((schema, field) => {
if (field.required) {
schema[field.name] = Yup.string().required(`${field.label} required`);
} else {
schema[field.name] = Yup.string();
if (!chassis || (chassis && field.scope !== "node")) {
if (field.required) {
schema[field.name] = Yup.string().required(
`${field.label} required`
);
} else {
schema[field.name] = Yup.string();
}
}
return schema;
}, {});
const newSchema = generateSchemaFunc(parametersSchema);
setSchema(newSchema);
}
}, [generateSchemaFunc, powerType]);
}, [chassis, generateSchemaFunc, powerType]);

return Schema;
};
Expand Down
57 changes: 37 additions & 20 deletions ui/src/app/machines/components/PowerTypeFields/PowerTypeFields.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,43 @@ import React from "react";

import FormikField from "app/base/components/FormikField";

const generateFields = (powerTypeFields, forChassis) => {
// When adding a chassis we need only a subset of fields that aren't specific
// to individual nodes.
const fields = forChassis
? powerTypeFields.filter((field) => field.scope !== "node")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell the scopes are either "node" or "bmc". If a third type was added would this filter still be correct? Or would we only want the fields that are scoped to "bmc"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure to be honest. I know for certain we don't want "node" scopes, and the only two scopes are "node" and "bmc". But whether or not an extra scope might include chassis will depend on the core team keeping us in the loop.

: [...powerTypeFields];

return fields.map((field) => {
const { choices, field_type, label, name, required } = field;
return (
<FormikField
component={field_type === "choice" ? Select : Input}
key={name}
label={label}
name={`power_parameters.${name}`}
options={
field_type === "choice"
? choices.map((choice) => ({
key: `${name}-${choice[0]}`,
label: choice[1],
value: choice[0],
}))
: undefined
}
required={required}
type={
(field_type === "string" && "text") ||
(field_type === "password" && "password") ||
undefined
}
/>
);
});
};

export const PowerTypeFields = ({
forChassis = false,
formikProps,
powerTypes,
selectedPowerType,
Expand Down Expand Up @@ -32,26 +68,7 @@ export const PowerTypeFields = ({
}}
required
/>
{powerType &&
powerType.fields.map((field) => (
<FormikField
component={field.field_type === "choice" ? Select : Input}
key={field.name}
label={field.label}
name={`power_parameters.${field.name}`}
options={
field.field_type === "choice"
? field.choices.map((choice) => ({
key: `${field.name}-${choice[0]}`,
label: choice[1],
value: choice[0],
}))
: undefined
}
required={field.required}
type={field.field_type === "password" ? "password" : "text"}
/>
))}
{powerType && generateFields(powerType.fields, forChassis)}
</>
);
};
Expand Down
112 changes: 112 additions & 0 deletions ui/src/app/machines/components/PowerTypeFields/PowerTypeFields.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,67 @@ import React from "react";
import PowerTypeFields from "../PowerTypeFields";

describe("PowerTypeFields", () => {
it("gives fields the correct types", async () => {
const formikProps = {
setFieldValue: jest.fn(),
setFieldTouched: jest.fn(),
};
const powerTypes = [
{
name: "fake_power_type",
description: "This is not real",
fields: [
{
name: "field1",
label: "String",
field_type: "string",
},
{
name: "field2",
label: "Password",
field_type: "password",
},
{
name: "field3",
label: "Choice",
field_type: "choice",
choices: [
["choice1", "Choice 1"],
["choice2", "Choice 2"],
],
},
],
},
];
const wrapper = mount(
<Formik>
<PowerTypeFields
formikProps={formikProps}
powerTypes={powerTypes}
selectedPowerType="fake_power_type"
/>
</Formik>
);

await act(async () => {
wrapper
.find("select[name='power_type']")
.props()
.onChange({ target: { name: "power_type", value: "fake_power_type" } });
});
wrapper.update();

expect(
wrapper.find("Input[name='power_parameters.field1']").props().type
).toBe("text");
expect(
wrapper.find("Input[name='power_parameters.field2']").props().type
).toBe("password");
expect(
wrapper.find("Select[name='power_parameters.field3']").props().type
).toBe(undefined);
});

it("correctly generates power options from power type", async () => {
const formikProps = {
setFieldValue: jest.fn(),
Expand Down Expand Up @@ -91,4 +152,55 @@ describe("PowerTypeFields", () => {
.text()
).toBe("Choice 2");
});

it("does not show node fields if using chassis power types", async () => {
const formikProps = {
setFieldValue: jest.fn(),
setFieldTouched: jest.fn(),
};
const powerTypes = [
{
name: "power_type",
description: "Power",
fields: [
{
name: "field1",
label: "Label1",
scope: "bmc",
},
{
name: "field2",
label: "Label2",
scope: "node",
},
],
},
];
const wrapper = mount(
<Formik>
<PowerTypeFields
forChassis
formikProps={formikProps}
powerTypes={powerTypes}
selectedPowerType="power_type"
/>
</Formik>
);

// Select the power type from the dropdown
await act(async () => {
wrapper
.find("select[name='power_type']")
.props()
.onChange({ target: { name: "power_type", value: "power_type" } });
});
wrapper.update();

expect(wrapper.find("Input[name='power_parameters.field1']").exists()).toBe(
true
);
expect(wrapper.find("Input[name='power_parameters.field2']").exists()).toBe(
false
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
usePowerParametersSchema,
useWindowTitle,
} from "app/base/hooks";
import { trimPowerParameters } from "app/utils";
import { formatPowerParameters } from "app/utils";
import AddChassisFormFields from "../AddChassisFormFields";
import FormCard from "app/base/components/FormCard";
import FormikForm from "app/base/components/FormikForm";
Expand Down Expand Up @@ -69,7 +69,8 @@ export const AddChassisForm = () => {

const ChassisSchema = usePowerParametersSchema(
powerType,
generateChassisSchema
generateChassisSchema,
true
);

const allPowerParameters = useAllPowerParameters(chassisPowerTypes);
Expand Down Expand Up @@ -107,7 +108,11 @@ export const AddChassisForm = () => {
const params = {
chassis_type: values.power_type,
domain: values.domain,
...trimPowerParameters(powerType, values.power_parameters),
...formatPowerParameters(
powerType,
values.power_parameters,
true
),
};
dispatch(machineActions.addChassis(params));
setSavingChassis(params.hostname || "chassis");
Expand Down
Loading