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

Avoid irrelevant changes in diff #2104

Merged
merged 1 commit into from
Oct 13, 2020
Merged
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
Avoid unrelevant changes in diff
Andres Martinez Gotor committed Oct 8, 2020
commit 4ca39b8569233a8bd2104d4c274013b8a8538bd8
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import * as React from "react";
import { act } from "react-dom/test-utils";
import { defaultStore, mountWrapper } from "shared/specs/mountWrapper";
import { IChartState, IChartVersion } from "shared/types";
import BasicDeploymentForm from "./BasicDeploymentForm";
import DeploymenetFormBody, { IDeploymentFormBodyProps } from "./DeploymentFormBody";
import DifferentialSelector from "./DifferentialSelector";

const defaultProps: IDeploymentFormBodyProps = {
deploymentEvent: "install",
chartID: "foo",
chartVersion: "1.0.0",
chartsIsFetching: false,
selected: {} as IChartState["selected"],
appValues: "foo: bar\n",
setValues: jest.fn(),
setValuesModified: jest.fn(),
};

jest.useFakeTimers();

const versions = [
{ id: "foo", attributes: { version: "1.2.3" }, relationships: { chart: { data: { repo: {} } } } },
] as IChartVersion[];

// Note that most of the tests that cover DeploymentFormBody component are in
// in the DeploymentForm and UpgradeForm parent components

// Context at https://github.com/kubeapps/kubeapps/issues/1293
it("should modify the original values of the differential component if parsed as YAML object", () => {
const oldValues = `a: b


c: d
`;
const schema = { properties: { a: { type: "string", form: true } } };
const selected = {
values: oldValues,
schema,
versions: [versions[0], { ...versions[0], attributes: { version: "1.2.4" } } as IChartVersion],
version: versions[0],
};

const wrapper = mountWrapper(
defaultStore,
<DeploymenetFormBody {...defaultProps} selected={selected} />,
);
expect(wrapper.find(DifferentialSelector).prop("defaultValues")).toBe(oldValues);

// Trigger a change in the basic form and a YAML parse
const input = wrapper.find(BasicDeploymentForm).find("input");
act(() => {
input.simulate("change", { currentTarget: "e" });
jest.advanceTimersByTime(500);
});
wrapper.update();

// The original double empty line gets deleted
const expectedValues = `a: b

c: d
`;
expect(wrapper.find(DifferentialSelector).prop("defaultValues")).toBe(expectedValues);
});
18 changes: 14 additions & 4 deletions dashboard/src/components/DeploymentFormBody/DeploymentFormBody.tsx
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@ import { CdsButton } from "@clr/react/button";
import { CdsIcon } from "@clr/react/icon";
import Alert from "components/js/Alert";
import { isEqual } from "lodash";
import { retrieveBasicFormParams, setValue } from "../../shared/schema";
import { parseValues, retrieveBasicFormParams, setValue } from "../../shared/schema";
import { DeploymentEvent, IBasicFormParam, IChartState } from "../../shared/types";
import { getValueFromEvent } from "../../shared/utils";
import ConfirmDialog from "../ConfirmDialog/ConfirmDialog";
@@ -40,7 +40,9 @@ function DeploymentFormBody({
}: IDeploymentFormBodyProps) {
const [basicFormParameters, setBasicFormParameters] = useState([] as IBasicFormParam[]);
const [restoreModalIsOpen, setRestoreModalOpen] = useState(false);
const { version, versions, schema } = selected;
const [defaultValues, setDefaultValues] = useState("");

const { version, versions, schema, values } = selected;

useEffect(() => {
const params = retrieveBasicFormParams(appValues, schema);
@@ -49,6 +51,10 @@ function DeploymentFormBody({
}
}, [setBasicFormParameters, schema, appValues, basicFormParameters]);

useEffect(() => {
setDefaultValues(values || "");
}, [values]);

const handleValuesChange = (value: string) => {
setValues(value);
setValuesModified();
@@ -58,8 +64,12 @@ function DeploymentFormBody({
};

const handleBasicFormParamChange = (param: IBasicFormParam) => {
const parsedDefaultValues = parseValues(defaultValues);
return (e: React.FormEvent<any>) => {
setValuesModified();
if (parsedDefaultValues !== defaultValues) {
setDefaultValues(parsedDefaultValues);
}
const value = getValueFromEvent(e);
setBasicFormParameters(
basicFormParameters.map(p => (p.path === param.path ? { ...param, value } : p)),
@@ -104,7 +114,7 @@ function DeploymentFormBody({
<DifferentialTab
key="differential-selector"
deploymentEvent={deploymentEvent}
defaultValues={selected.values || ""}
defaultValues={defaultValues}
deployedValues={deployedValues || ""}
appValues={appValues}
/>,
@@ -122,7 +132,7 @@ function DeploymentFormBody({
<DifferentialSelector
key="differential-selector"
deploymentEvent={deploymentEvent}
defaultValues={selected.values || ""}
defaultValues={defaultValues}
deployedValues={deployedValues || ""}
appValues={appValues}
/>,
5 changes: 5 additions & 0 deletions dashboard/src/shared/schema.ts
Original file line number Diff line number Diff line change
@@ -125,6 +125,11 @@ export function setValue(values: string, path: string, newValue: any) {
return doc.toString();
}

// parseValues returns a processed version of the values without modifying anything
export function parseValues(values: string) {
return YAML.parseDocument(values).toString();
}

export function deleteValue(values: string, path: string) {
const doc = YAML.parseDocument(values);
const { splittedPath } = parsePathAndValue(doc, path);