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

Show correct release name in case of error #791

Merged
merged 3 commits into from
Nov 8, 2018
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
25 changes: 24 additions & 1 deletion dashboard/src/components/DeploymentForm/DeploymentForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,36 @@ describe("renders an error", () => {
error={new UnprocessableEntity("wrong format!")}
/>,
);
wrapper.setState({ releaseName: "my-app" });
wrapper.setState({ latestSubmittedReleaseName: "my-app" });
expect(wrapper.find(ErrorSelector).exists()).toBe(true);
expect(wrapper.find(ErrorSelector).html()).toContain(
"Sorry! Something went wrong processing my-app",
);
expect(wrapper.find(ErrorSelector).html()).toContain("wrong format!");
});

it("the error does not change if the release name changes", () => {
const expectedErrorMsg = "Sorry! Something went wrong processing my-app";

const wrapper = shallow(
<DeploymentForm
{...defaultProps}
selected={
{
version: { attributes: {} },
versions: [{ id: "foo", attributes: {} }],
} as IChartState["selected"]
}
error={new UnprocessableEntity("wrong format!")}
/>,
);

wrapper.setState({ latestSubmittedReleaseName: "my-app" });
expect(wrapper.find(ErrorSelector).exists()).toBe(true);
expect(wrapper.find(ErrorSelector).html()).toContain(expectedErrorMsg);
wrapper.setState({ releaseName: "another-app" });
expect(wrapper.find(ErrorSelector).html()).toContain(expectedErrorMsg);
Copy link
Contributor

Choose a reason for hiding this comment

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

this assertion won't fail even if the bug is still present (since you are appending just a 2), you'll need to use a completely different name for the releaseName or use .not.toContain("my-app2").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahaaa, good catch!

});
});

it("renders the full DeploymentForm", () => {
Expand Down
14 changes: 11 additions & 3 deletions dashboard/src/components/DeploymentForm/DeploymentForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ interface IDeploymentFormState {
isDeploying: boolean;
// deployment options
releaseName: string;
// Name of the release that was submitted for creation
// This is different than releaseName since it is also used in the error banner
// and we do not want to use releaseName since it is controller by the form field.
latestSubmittedReleaseName: string;
namespace: string;
appValues?: string;
valuesModified: boolean;
Expand All @@ -47,6 +51,7 @@ class DeploymentForm extends React.Component<IDeploymentFormProps, IDeploymentFo
isDeploying: false,
namespace: this.props.namespace,
releaseName: "",
latestSubmittedReleaseName: "",
valuesModified: false,
};

Expand Down Expand Up @@ -102,7 +107,7 @@ class DeploymentForm extends React.Component<IDeploymentFormProps, IDeploymentFo
public render() {
const { selected, bindingsWithSecrets, chartID, chartVersion, namespace } = this.props;
const { version, versions } = selected;
const { appValues, releaseName } = this.state;
const { appValues, latestSubmittedReleaseName } = this.state;
if (selected.error) {
return (
<ErrorSelector error={selected.error} resource={`Chart "${chartID}" (${chartVersion})`} />
Expand All @@ -120,7 +125,7 @@ class DeploymentForm extends React.Component<IDeploymentFormProps, IDeploymentFo
namespace={namespace}
defaultRequiredRBACRoles={{ create: this.requiredRBACRoles() }}
action="create"
resource={releaseName}
resource={latestSubmittedReleaseName}
/>
)}
<div className="row">
Expand Down Expand Up @@ -187,8 +192,9 @@ class DeploymentForm extends React.Component<IDeploymentFormProps, IDeploymentFo
public handleDeploy = async (e: React.FormEvent<HTMLFormElement>) => {
e.preventDefault();
const { selected, deployChart, push } = this.props;
this.setState({ isDeploying: true });
const { releaseName, namespace, appValues } = this.state;

this.setState({ isDeploying: true, latestSubmittedReleaseName: releaseName });
if (selected.version) {
const deployed = await deployChart(selected.version, releaseName, namespace, appValues);
if (deployed) {
Expand All @@ -202,13 +208,15 @@ class DeploymentForm extends React.Component<IDeploymentFormProps, IDeploymentFo
public handleReleaseNameChange = (e: React.FormEvent<HTMLInputElement>) => {
this.setState({ releaseName: e.currentTarget.value });
};

public handleChartVersionChange = (e: React.FormEvent<HTMLSelectElement>) => {
this.props.push(
`/apps/ns/${this.props.namespace}/new/${this.props.chartID}/versions/${
e.currentTarget.value
}`,
);
};

public handleValuesChange = (value: string) => {
this.setState({ appValues: value, valuesModified: true });
};
Expand Down