Skip to content

Commit

Permalink
fix(core/pipeline): KLUDGE: use react 'key' to reinitialize formik wh…
Browse files Browse the repository at this point in the history
…en pipeline reverted

This is a workaround for angular/react interactions.

Previously, we had turned on `enableReinitialize` for trigger formik forms.
This allowed angular code to revert the pipeline and the changes would flow through through to formik via 'initialValues'.
However, turning on that prop had the unwanted side effect of reinitializing the form whenever _anything_ changed.
This caused problems with the validation logic.

This workaround causes the triggers component to be remounted whenever the pipeline is rolled back.
  • Loading branch information
christopherthielen committed Oct 8, 2019
1 parent 60290ed commit 829c295
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
field-updated="stageFieldUpdated"
pipeline="isV2TemplatedPipeline ? pipeline : pipelineConfig"
update-pipeline-config="updatePipelineConfig"
view-state="viewState"
>
</triggers>
<pipeline-config-stage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ module.exports = angular
section: 'triggers',
stageIndex: 0,
loading: false,
revertCount: 0,
};

$scope.viewState.loadingHistory = true;
Expand Down Expand Up @@ -454,6 +455,7 @@ module.exports = angular
this.navigateTo({ section: 'triggers' });
}
}
$scope.viewState.revertCount++;
$scope.$broadcast('pipeline-reverted');
});
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export const Trigger = (props: ITriggerProps) => (
<SpinFormik
onSubmit={() => null}
initialValues={props.trigger}
enableReinitialize={true}
render={formik => <TriggerForm {...props} formik={formik} />}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,19 @@ export interface ITriggersProps {
pipeline: IPipeline;
fieldUpdated: () => void;
updatePipelineConfig: (changes: Partial<IPipeline>) => void;
viewState: { revertCount: number };
}

export function Triggers(props: ITriggersProps) {
const { pipeline } = props;
const { pipeline, viewState } = props;

function checkFeatureFlag(flag: string): boolean {
return !!SETTINGS.feature[flag];
}

// KLUDGE: This value is used as a React key when rendering the Triggers.
// Whenever the pipeline is reverted, this causes the Triggers to remount and reset formik state.
const revertCountKLUDGE = viewState.revertCount;
return (
<PageNavigator scrollableContainer="[ui-view]">
<PageSection pageKey="concurrent" label="Execution Options" visible={!pipeline.strategy}>
Expand All @@ -45,7 +49,7 @@ export function Triggers(props: ITriggersProps) {
badge={pipeline.triggers ? pipeline.triggers.length.toString() : '0'}
noWrapper={true}
>
<TriggersPageContent {...props} />
<TriggersPageContent {...props} key={revertCountKLUDGE} />
</PageSection>
<PageSection
pageKey="parameters"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ import { ARTIFACT_MODULE } from './artifacts/artifact.module';
export const TRIGGERS = 'spinnaker.core.pipeline.config.trigger.triggersDirective';
module(TRIGGERS, [ARTIFACT_MODULE]).component(
'triggers',
react2angular(Triggers, ['application', 'pipeline', 'fieldUpdated', 'updatePipelineConfig']),
react2angular(Triggers, ['application', 'pipeline', 'fieldUpdated', 'updatePipelineConfig', 'viewState']),
);

0 comments on commit 829c295

Please sign in to comment.