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.

Sign up for GitHub

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

[Ingest Node Pipelines] Refactor pipeline simulator code #72328

Merged
merged 8 commits into from
Aug 5, 2020
Merged

[Ingest Node Pipelines] Refactor pipeline simulator code #72328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5a4dfa8
migrate test pipeline flyout to editor
alisonelizabeth Jul 17, 2020
de7bd10
Merge branch 'master' of github.com:elastic/kibana into feature/pipel…
alisonelizabeth Jul 17, 2020
d012f0e
Merge branch 'master' of github.com:elastic/kibana into feature/pipel…
alisonelizabeth Jul 29, 2020
05ebb2a
cleanup
alisonelizabeth Jul 29, 2020
1749aee
fix i18n + tests
alisonelizabeth Jul 31, 2020
be2e246
Merge branch 'master' into feature/pipeline_debugger
elasticmachine Jul 31, 2020
18e1a3d
fix i18n
alisonelizabeth Jul 31, 2020
8035de2
Merge branch 'master' into feature/pipeline_debugger
elasticmachine Aug 5, 2020
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
2 changes: 1 addition & 1 deletion x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
* you may not use this file except in compliance with the Elastic License.
*/

export { PipelineFormProvider as PipelineForm } from './pipeline_form_provider';
export { PipelineForm } from './pipeline_form';
22 changes: 0 additions & 22 deletions ...ck/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import './pipeline_form.scss';
import { OnUpdateHandlerArg, OnUpdateHandler } from '../pipeline_processors_editor';

import { PipelineRequestFlyout } from './pipeline_request_flyout';
import { PipelineTestFlyout } from './pipeline_test_flyout';
import { PipelineFormFields } from './pipeline_form_fields';
import { PipelineFormError } from './pipeline_form_error';
import { pipelineFormSchema } from './schema';
Expand Down Expand Up @@ -48,8 +47,6 @@ export const PipelineForm: React.FunctionComponent<PipelineFormProps> = ({
}) => {
const [isRequestVisible, setIsRequestVisible] = useState<boolean>(false);

const [isTestingPipeline, setIsTestingPipeline] = useState<boolean>(false);

const {
processors: initialProcessors,
on_failure: initialOnFailureProcessors,
Expand Down Expand Up @@ -79,18 +76,13 @@ export const PipelineForm: React.FunctionComponent<PipelineFormProps> = ({
}
};

const handleTestPipelineClick = () => {
setIsTestingPipeline(true);
};

const { form } = useForm<IPipelineForm>({
schema: pipelineFormSchema,
defaultValue: defaultFormValues,
onSubmit: handleSave,
});

const onEditorFlyoutOpen = useCallback(() => {
setIsTestingPipeline(false);
setIsRequestVisible(false);
}, [setIsRequestVisible]);

Expand Down Expand Up @@ -137,8 +129,6 @@ export const PipelineForm: React.FunctionComponent<PipelineFormProps> = ({
onFailure={processorsState.onFailure}
onProcessorsUpdate={onProcessorsChangeHandler}
hasVersion={Boolean(defaultValue.version)}
isTestButtonDisabled={isTestingPipeline || form.isValid === false}
onTestPipelineClick={handleTestPipelineClick}
isEditing={isEditing}
/>

Expand Down Expand Up @@ -198,18 +188,6 @@ export const PipelineForm: React.FunctionComponent<PipelineFormProps> = ({
closeFlyout={() => setIsRequestVisible((prevIsRequestVisible) => !prevIsRequestVisible)}
/>
) : null}

{/* Test pipeline flyout */}
{isTestingPipeline ? (
<PipelineTestFlyout
readProcessors={() =>
processorStateRef.current?.getData() || { processors: [], on_failure: [] }
}
closeFlyout={() => {
setIsTestingPipeline((prevIsTestingPipeline) => !prevIsTestingPipeline);
}}
/>
) : null}
</Form>

<EuiSpacer size="m" />
Expand Down
18 changes: 6 additions & 12 deletions ...ins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form_fields.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { Processor } from '../../../../common/types';
import { getUseField, getFormRow, Field, useKibana } from '../../../shared_imports';

import {
PipelineProcessorsContextProvider,
ProcessorsEditorContextProvider,
GlobalOnFailureProcessorsEditor,
ProcessorsEditor,
OnUpdateHandler,
Expand All @@ -29,8 +29,6 @@ interface Props {
onLoadJson: OnDoneLoadJsonHandler;
onProcessorsUpdate: OnUpdateHandler;
hasVersion: boolean;
isTestButtonDisabled: boolean;
onTestPipelineClick: () => void;
onEditorFlyoutOpen: () => void;
isEditing?: boolean;
}
Expand All @@ -45,8 +43,6 @@ export const PipelineFormFields: React.FunctionComponent<Props> = ({
onProcessorsUpdate,
isEditing,
hasVersion,
isTestButtonDisabled,
onTestPipelineClick,
onEditorFlyoutOpen,
}) => {
const { services } = useKibana();
Expand Down Expand Up @@ -125,20 +121,18 @@ export const PipelineFormFields: React.FunctionComponent<Props> = ({

{/* Pipeline Processors Editor */}

<PipelineProcessorsContextProvider
<ProcessorsEditorContextProvider
onFlyoutOpen={onEditorFlyoutOpen}
Copy link
Contributor

Choose a reason for hiding this comment

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

Using the global flyout will also enable us to remove this prop which would be great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! I was thinking the same.

links={{ esDocsBasePath: services.documentation.getEsDocsBasePath() }}
api={services.api}
toasts={services.notifications.toasts}
Comment on lines +127 to +128
Copy link
Contributor

Choose a reason for hiding this comment

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

I see we are using the Kibana context provider here. I think it would be cool to add that as a dependency for the processors editor component. So in tests it would become:

<KibanaContextProvider>
    <ProcessorsEditorContextProvider>
        {/* Editors in here */}
    </ProcessorsEditorContextProvider>
</KibanaContextProvider>

That way we can get rid of the api and toasts prop. What do you think? Happy to do this in a separate pass if you agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point. I agree. I'd like to get this PR in since it's needed for any subsequent PRs related to the pipeline simulation, but will definitely address in the next pass.

onUpdate={onProcessorsUpdate}
value={{ processors, onFailure }}
>
<div className="pipelineProcessorsEditor">
<EuiFlexGroup gutterSize="m" responsive={false} direction="column">
<EuiFlexItem grow={false}>
<ProcessorsHeader
onLoadJson={onLoadJson}
onTestPipelineClick={onTestPipelineClick}
isTestButtonDisabled={isTestButtonDisabled}
/>
<ProcessorsHeader onLoadJson={onLoadJson} />
</EuiFlexItem>
<EuiFlexItem grow={false}>
<ProcessorsEditor />
Expand All @@ -154,7 +148,7 @@ export const PipelineFormFields: React.FunctionComponent<Props> = ({
</EuiFlexItem>
</EuiFlexGroup>
</div>
</PipelineProcessorsContextProvider>
</ProcessorsEditorContextProvider>
</>
);
};
20 changes: 0 additions & 20 deletions ...s/ingest_pipelines/public/application/components/pipeline_form/pipeline_form_provider.tsx
View file
Open in desktop

This file was deleted.

203 changes: 0 additions & 203 deletions ...public/application/components/pipeline_form/pipeline_test_flyout/pipeline_test_flyout.tsx
View file
Open in desktop

This file was deleted.

Loading