[Ingest Pipelines] Pipeline Editor MVP#63617
[Ingest Pipelines] Pipeline Editor MVP#63617jloleysens wants to merge 4 commits intoelastic:feature/ingest-node-pipelinesfrom
Conversation
|
@elasticmachine merge upstream |
|
merge conflict between base and head |
…bana into pipeline-editor-part-mvp-1 * 'feature/ingest-node-pipelines' of github.com:elastic/kibana: [Ingest pipelines] Edit pipeline page (elastic#63522) # Conflicts: # x-pack/plugins/ingest_pipelines/public/application/sections/pipelines_create/pipelines_create.tsx # x-pack/plugins/ingest_pipelines/public/shared_imports.ts
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
💔 Build Failed
Failed CI StepsHistory
To update your PR or re-run it, just comment with: |
| <UseField config={typeConfig} path={'type'} defaultValue={initialType}> | ||
| {typeField => { | ||
| return ( | ||
| <EuiComboBox |
There was a problem hiding this comment.
There is an <EuiComboBoxField /> that already takes care of serialization, deserialization, validation at the array level, validation at the item level. I would recommend using it.
This is the config (from the schema) where you can see the 2 types of validations: at the Array level (the first one), and at the item level (the second one, declaring a type: VALIDATION_TYPES.ARRAY_ITEM).
And this is how it is simply declared
No need to re-write the logic of onChange, options, selectedOptions.
The important part is to not forget to declare the defaultValue as [] (empty array).
| } from '../../../../../shared_imports'; | ||
|
|
||
| const { emptyField } = fieldValidators; | ||
|
|
There was a problem hiding this comment.
Do you prefer to declare all the configs above the component instead of inside a FormSchema? I would find it easier to scan to have all the fields declaration in a single place, but that's my preference 😊
| label: i18n.translate('xpack.ingestPipelines.pipelineEditor.gsubForm.patternFieldLabel', { | ||
| defaultMessage: 'Pattern', | ||
| }), | ||
| validations: [ |
There was a problem hiding this comment.
Not sure if this field is for index pattern or not, but in case it is, know that there is an indexPatternField validator for them.
| }); | ||
|
|
||
| useEffect(() => { | ||
| const subscription = form.subscribe(({ data }) => { |
There was a problem hiding this comment.
Instead of this + having to declare a state and update it, it is better to use the FormDataProvider
return (
<Form form={form}>
<ProcessorTypeField initialType={processor?.type} />
<FormDataProvider pathsToWatch="type">
{({ type }) => {
const FormFields = getProcessorFormDescriptor(type as any);
// TODO: Handle this error in a different way
if (!FormFields) {
throw new Error(`Could not find form for type ${type}`);
}
return (
<>
<FormFields />
<CommonProcessorFields />
<EuiButton onClick={form.submit}>Submit</EuiButton>
</>
);
}}
</FormDataProvider>
</Form>
);
Summary
First iteration of pipeline editor.
Notes to reviewer
Main focus has been establishing patterns for us to flesh out all the other processor forms.
All styling optimisations have been deferred until after this review and once final designs have been agreed upon.
Translations have been done or marked as TODO throughout (please flag any ones that I may have missed).
The data has not been flattened to a
byIdmap which keeps the reordering and rendering logic a bit simpler for now. Optimisation other than flattening to a byId map is still possible, which I would like to explore a bit later on but for now, worst case O(n) on an array that should never grow past 50-60 (guessing?) items is acceptable IMO.Have not built rendering of on failure pipelines yet. There are some things that are still in flux here, however I can definitely work on just rending the nested list next.
Screenshots
Main View
React DnD 🎉
Processor form