Skip to content

Commit

Permalink
fix(core): do not validate pipeline configs before initialization (#7003
Browse files Browse the repository at this point in the history
)

* fix(core): do not validate pipeline configs before initialization

* refactor(core): clean up old code in configure projects modal
  • Loading branch information
anotherchrisberry committed May 15, 2019
1 parent 044410b commit cfb2a51
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { FormikApplicationsPicker } from 'core/projects/configure/FormikApplicat
export interface IApplicationsProps {
formik: FormikProps<IProject>;
allApplications: string[];
onApplicationsChanged: (applications: string[]) => void;
}

export class Applications extends React.Component<IApplicationsProps> implements IWizardPageComponent<IProject> {
Expand All @@ -30,17 +29,11 @@ export class Applications extends React.Component<IApplicationsProps> implements
} as any;
}

public componentDidMount() {
const apps = getIn(this.props.formik.values, 'config.applications', []);
this.props.onApplicationsChanged && this.props.onApplicationsChanged(apps);
}

public componentDidUpdate(prevProps: IApplicationsProps) {
const prevApps = getIn(prevProps.formik.values, 'config.applications', []);
const nextApps = getIn(this.props.formik.values, 'config.applications', []);

if (!isEqual(prevApps, nextApps)) {
this.props.onApplicationsChanged && this.props.onApplicationsChanged(nextApps);
// Remove any pipelines associated with the applications removed.
const existingPipelineConfigs: IProjectPipeline[] = getIn(this.props.formik.values, 'config.pipelineConfigs', []);
const newPipelineConfigs = existingPipelineConfigs.filter(({ application }) => nextApps.includes(application));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export interface IConfigureProjectModalState {
allAccounts: IAccount[];
allProjects: IProject[];
allApplications: IApplicationSummary[];
configuredApps: string[];
loading: boolean;
taskMonitor?: TaskMonitor;
}
Expand All @@ -43,7 +42,6 @@ export class ConfigureProjectModal extends React.Component<IConfigureProjectModa
allAccounts: [],
allProjects: [],
allApplications: [],
configuredApps: [],
};

public static show(props?: IConfigureProjectModalProps): Promise<any> {
Expand All @@ -67,14 +65,8 @@ export class ConfigureProjectModal extends React.Component<IConfigureProjectModa
return ReactModal.show(ConfigureProjectModal, projectProps, modalProps);
}

private handleApplicationsChanged = (configuredApps: string[]) => {
this.setState({ configuredApps });
};

public componentDidMount() {
const { projectConfiguration } = this.props;
const configuredApps = (projectConfiguration && projectConfiguration.config.applications) || [];
Promise.all([this.initialFetch()]).then(() => this.setState({ loading: false, configuredApps }));
this.initialFetch().then(() => this.setState({ loading: false }));
}

private submit = (project: IProject) => {
Expand Down Expand Up @@ -146,14 +138,7 @@ export class ConfigureProjectModal extends React.Component<IConfigureProjectModa
label="Applications"
wizard={wizard}
order={nextIdx()}
render={({ innerRef }) => (
<Applications
ref={innerRef}
formik={formik}
allApplications={appNames}
onApplicationsChanged={this.handleApplicationsChanged}
/>
)}
render={({ innerRef }) => <Applications ref={innerRef} formik={formik} allApplications={appNames} />}
/>

<WizardPage
Expand Down
40 changes: 29 additions & 11 deletions app/scripts/modules/core/src/projects/configure/Pipelines.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import { FieldArray, FormikErrors, FormikProps, getIn } from 'formik';
import { chain, each, get, isEqual } from 'lodash';
import { chain, get, isEqual } from 'lodash';

import { PipelineConfigService } from 'core/pipeline';
import { FormikFormField, ReactSelectInput, StringsAsOptions } from 'core/presentation';
Expand All @@ -16,6 +16,7 @@ export interface IPipelinesState {
appsPipelines: {
[appName: string]: IPipeline[];
};
initialized: boolean;
}

export class Pipelines extends React.Component<IPipelinesProps, IPipelinesState>
Expand All @@ -24,15 +25,16 @@ export class Pipelines extends React.Component<IPipelinesProps, IPipelinesState>

public state: IPipelinesState = {
appsPipelines: {},
initialized: false,
};

public validate = (value: IProject): FormikErrors<IProject> => {
const projectApplications = (value.config && value.config.applications) || [];
const { appsPipelines } = this.state;
const { appsPipelines, initialized } = this.state;

if (value.config && value.config.pipelineConfigs && value.config.pipelineConfigs.length) {
if (initialized && value.config && value.config.pipelineConfigs && value.config.pipelineConfigs.length) {
const pipelineConfigErrors = value.config.pipelineConfigs.map(config => {
const pipelineIdsForApp = (appsPipelines[config.application] || []).map(p => p.id);
const pipelineIdsForApp = appsPipelines[config.application].map(p => p.id);

if (!config.application) {
return { application: 'Application must be specified' };
Expand Down Expand Up @@ -71,12 +73,20 @@ export class Pipelines extends React.Component<IPipelinesProps, IPipelinesState>
.filter(appName => appName && !this.state.appsPipelines[appName])
.value();

each(appsToFetch, appName => {
PipelineConfigService.getPipelinesForApplication(appName).then(pipelines =>
this.setState({
appsPipelines: { ...this.state.appsPipelines, [appName]: pipelines },
}),
);
const appsPipelines: { [appName: string]: IPipeline[] } = { ...this.state.appsPipelines };

Promise.all(
appsToFetch.map(appName => {
return PipelineConfigService.getPipelinesForApplication(appName)
.then(pipelines => {
appsPipelines[appName] = pipelines;
})
.catch(() => {
appsPipelines[appName] = [];
});
}),
).then(() => {
this.setState({ appsPipelines, initialized: true });
});
};

Expand All @@ -91,7 +101,15 @@ export class Pipelines extends React.Component<IPipelinesProps, IPipelinesState>
}

public render() {
const { appsPipelines } = this.state;
const { appsPipelines, initialized } = this.state;

if (!initialized) {
return (
<div style={{ height: '200px' }}>
<Spinner size="medium" />
</div>
);
}

const tableHeader = (
<tr>
Expand Down

0 comments on commit cfb2a51

Please sign in to comment.