[Ingest Pipelines] Add descriptions for ingest processors A-D by lockewritesdocs · Pull Request #75975 · elastic/kibana

lockewritesdocs · 2020-08-26T13:24:06Z

Summary

This PR updates UI field strings and adds a description for the following processors, extending the work completed in #72849:

Append Processor
Bytes Processor
Circle Processor
Convert Processor
CSV Processor
Date Processor
Date Index Name Processor
Dissect Processor
Dot Expander Processor
Drop Processor

Relates to Ingest Node Processors Editors Forms
Relates to #76113 and #72849

How to test

Start Kibana with basic license
Go to ingest pipelines plugin in management section
Add a processor for each of the forms listed above (attempt to submit to see which fields are required)
Create the pipeline

Notes

There's a strong chance that the coding in x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/shared/map_processor_type_to_form.tsx is incorrect. @alisonelizabeth provided the proper syntax, but I might have broken something 😉

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-08-26T13:24:08Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

jloleysens

Great work @lockewritesdocs ! This PR is looking in good shape, thanks for making those copy changes!

I left a few comments in the code that would be great if you could take a look at. It also looks like we are missing type annotations here for the new field (the thing I would like to block on):

kibana/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/shared/map_processor_type_to_form.tsx

Line 36 in c233ecf

interface FieldDescriptor {

I see you have named the new field helpText which aligns well with the name in the EuiFormRow which we are going to be using, but I think description would be a better name for this text.

jloleysens · 2020-08-27T13:51:55Z

...rs_editor/components/manage_processor_form/processors/common_fields/processor_type_field.tsx

    <UseField<string> config={typeConfig} defaultValue={initialType} path="type">
      {(typeField) => {
        let selectedOptions: ProcessorTypeAndLabel[];
+        let helpText: string = '';


It looks like we still need to assign this value to the EuiFormRow below

You can also drop the : string annotation. If you init this value to a string TS will figure out that it is a string automatically.

jloleysens · 2020-08-27T13:55:17Z

...rs_editor/components/manage_processor_form/processors/common_fields/processor_type_field.tsx

            ? [{ label: descriptor.label, value: type }]
            : // If there is no label for this processor type, just use the type as the label
              [{ label: type, value: type }];
+          helpText = descriptor?.helpText || '';


We could probably restructure this code now to something like:

const procesorDescriptor = getProcessorDescriptor(type); if (procesorDescriptor) { selectedOptions = [{ label: procesorDescriptor.label, value: type }]; helpText = procesorDescriptor.description; // assuming we are using the key `description` } else { // If there is no label for this processor type, just use the type as the label selectedOptions = [{ label: type, value: type }]; }

I think this would read a bit easier :)

lockewritesdocs · 2020-08-27T19:24:07Z

@elasticmachine merge upstream

jloleysens · 2020-08-31T08:00:39Z

@elasticmachine merge upstream

jloleysens

@alisonelizabeth @lockewritesdocs Thanks for addressing my feedback!

I did not re-test locally, but the proposed code changes look good to me.

jrodewig

Looks good overall. I left some suggestions, but most are non-blocking nits.

One bigger discussion point: Most of the processors have an output field. Some can also update the input field in place.

We should develop a common pattern. Here are the ones we have so far:

Field to assign the converted value to. Defaults to ...
Field used to contain the converted value. Defaults to ...

Those are okay, but I think we may be able to get away with something simpler like:

Output field. If empty, the field is updated in place.

Regardless, we should pick one and use it consistently throughout. Let me know your thoughts. Once we make a decision, I can update the field help text in processors E-J.

...ation/components/pipeline_processors_editor/components/shared/map_processor_type_to_form.tsx

...components/pipeline_processors_editor/components/manage_processor_form/processors/append.tsx

...ation/components/pipeline_processors_editor/components/shared/map_processor_type_to_form.tsx

...ents/pipeline_processors_editor/components/manage_processor_form/processors/dot_expander.tsx

...ation/components/pipeline_processors_editor/components/shared/map_processor_type_to_form.tsx

jrodewig · 2020-09-01T14:06:33Z

...omponents/pipeline_processors_editor/components/manage_processor_form/processors/dissect.tsx

      <FormattedMessage
        id="xpack.ingestPipelines.pipelineEditor.dissectForm.appendSeparatorHelpText"
-        defaultMessage="The character(s) that separate the appended fields. Default value is {value} (an empty string)."
+        defaultMessage="Characters used to separate fields when appending two or more results together. Defaults to {value}."


This description didn't make sense to me. Is it used to separate or append results?

I dug a little deeper on this field, and the append separator is only applicable when an append modifier is specified. I've updated the Pattern field to reference the key modifiers (including a link to that section of the doc page), and noted in the append separator description that it applies only if using a modifier in the pattern.

Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

lockewritesdocs · 2020-09-02T13:05:44Z

@elasticmachine merge upstream

jloleysens · 2020-09-02T16:12:21Z

...ation/components/pipeline_processors_editor/components/shared/map_processor_type_to_form.tsx

+    description: {
+      defaultMessage: (
+        <FormattedMessage
+          id="xpack.ingestPipelines.processors.description.dateIndexName"
+          defaultMessage="Uses a date or timestamp to add documents to the correct time-based index. Index names must use a date math pattern, such as {value}."
+          values={{ value: <EuiCode inline>{'my-index-yyyy-MM-dd'}</EuiCode> }}
+        />
+      ),
+    },


You will need to remove the { defaultMessage: ... } wrapping the <FormattedMessage ... /> component - you you will probably also get a TS error about this :)

lockewritesdocs · 2020-09-02T18:00:26Z

@jloleysens, I incorporated feedback from @jrodewig, so I think that these changes can now be merged. The only other change I recommend is removing the two horizontal bars for the Drop processor, since there are no additional fields to display.

jloleysens · 2020-09-04T13:41:14Z

@lockewritesdocs Thanks for the updated! I will create a fix for that.

lockewritesdocs · 2020-09-04T15:33:37Z

@elasticmachine merge upstream

elasticmachine · 2020-09-04T15:33:39Z

merge conflict between base and head

kibanamachine · 2020-09-04T18:41:07Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 6872e49

Build metrics

async chunks size

id	value	diff	baseline
ingestPipelines	734.8KB	+3.3KB	731.6KB

History

💔 Build #72661 failed 500403b
💚 Build #72014 succeeded 9982b01
💔 Build #71991 failed 0a1b715
💔 Build #71939 failed b588bd9
💔 Build #71869 failed e129444

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* master: [APM] Use observer.hostname instead of observer.name (elastic#76074) Legacy logging: fix remoteAddress being duplicated in userAgent field (elastic#76751) Remove legacy deprecation adapter (elastic#76753) [Security Solution][Detections] Rule forms cleanup (elastic#76138) Adds back in custom images for reporting + tests (elastic#76810) [APM] Fix overlapping transaction names (elastic#76083) [Ingest Pipelines] Add descriptions for ingest processors A-D (elastic#75975)

…76113) (#76948) * [Ingest Pipelines] Add descriptions for ingest processors E-J (#76113) Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com> # Conflicts: # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/shared/map_processor_type_to_form.tsx * Add code from #76948 * [Ingest Pipelines] Add descriptions for ingest processors A-D (#75975) * Fixing formatting issues identified by Prettier, part 2. * Fixing helpText labels. * Adding {value} object for dissect processor. * Incorporating reviewer feedback. * fix dropdown not rendering * Fixing typo. * add support for FormattedMessage in help text * fix TS * Updating some strings and trying to add code formatting. * fix formatted message * Editing some field descriptions. * Apply suggestions from code review Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com> * Trying to add EuiLink, plus edits. * fix help text for dissect processor * Incorporating reviewer feedback. * Trying to add another EUI element, plus edits. * fix date_index_name description text * Minor edit. * Fixing linter error. * Removing FunctionComponent, which was not read and caused build errors. Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com> Co-authored-by: Adam Locke <adam.locke@elastic.co> Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

kibanamachine · 2020-09-08T19:28:44Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 75975 or prevent reminders by adding the backport:skip label.

lockewritesdocs · 2020-09-09T18:51:05Z

I added the backport:skip label because #76113 by @jrodewig included the changes from this PR.

Fixing formatting issues identified by Prettier, part 2.

5a74358

lockewritesdocs added v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.10.0 Feature:Ingest Node Pipelines Ingest node pipelines management labels Aug 26, 2020

lockewritesdocs requested a review from jloleysens August 26, 2020 13:24

lockewritesdocs requested a review from a team as a code owner August 26, 2020 13:24

lockewritesdocs self-assigned this Aug 26, 2020

lockewritesdocs added the release_note:skip Skip the PR/issue when compiling release notes label Aug 26, 2020

Adam Locke added 2 commits August 26, 2020 10:31

Fixing helpText labels.

377a778

Adding {value} object for dissect processor.

c233ecf

jloleysens requested changes Aug 27, 2020

View reviewed changes

jrodewig mentioned this pull request Aug 27, 2020

[Ingest Pipelines] Add descriptions for ingest processors E-J #76113

Merged

1 task

Adam Locke and others added 3 commits August 27, 2020 12:59

Incorporating reviewer feedback.

446c108

fix dropdown not rendering

a848afe

Fixing typo.

404ebb4

elasticmachine and others added 3 commits August 27, 2020 13:24

Merge branch 'master' into docs__update-ingest-pipeline-strings

adf6e80

add support for FormattedMessage in help text

8e34f7a

fix TS

439dfdb

Merge branch 'master' into docs__update-ingest-pipeline-strings

6de3d1e

jloleysens approved these changes Aug 31, 2020

View reviewed changes

Adam Locke and others added 3 commits August 31, 2020 16:09

Updating some strings and trying to add code formatting.

2835d3d

fix formatted message

f4fdf28

Editing some field descriptions.

a0be7d0

lockewritesdocs requested a review from jrodewig August 31, 2020 21:39

jrodewig approved these changes Sep 1, 2020

View reviewed changes

Apply suggestions from code review

929e016

Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

Adam Locke and others added 3 commits September 1, 2020 14:45

Trying to add EuiLink, plus edits.

67e3f5f

fix help text for dissect processor

9145874

Incorporating reviewer feedback.

dfc4b4f

elasticmachine and others added 2 commits September 2, 2020 07:05

Merge branch 'master' into docs__update-ingest-pipeline-strings

e129444

Trying to add another EUI element, plus edits.

b588bd9

jloleysens reviewed Sep 2, 2020

View reviewed changes

alisonelizabeth and others added 2 commits September 2, 2020 13:25

fix date_index_name description text

6b43188

Minor edit.

0a1b715

Fixing linter error.

9982b01

lockewritesdocs changed the title ~~[DOCS] Updating UI strings for Ingest Pipeline processors A-D~~ [Ingest Pipelines] Add descriptions for ingest processors A-D Sep 4, 2020

Adam Locke added 2 commits September 4, 2020 11:44

Merge branch 'master' into docs__update-ingest-pipeline-strings

500403b

Removing FunctionComponent, which was not read and caused build errors.

6872e49

lockewritesdocs merged commit 6d4e772 into elastic:master Sep 4, 2020

lockewritesdocs deleted the docs__update-ingest-pipeline-strings branch September 4, 2020 19:26

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 8, 2020

lockewritesdocs added backport:skip This PR does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Sep 9, 2020

Conversation

lockewritesdocs commented Aug 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How to test

Notes

Checklist

For maintainers

Uh oh!

elasticmachine commented Aug 26, 2020

Uh oh!

jloleysens left a comment

Choose a reason for hiding this comment

Uh oh!

jloleysens Aug 27, 2020

Choose a reason for hiding this comment

Uh oh!

jloleysens Aug 27, 2020

Choose a reason for hiding this comment

Uh oh!

jloleysens Aug 27, 2020

Choose a reason for hiding this comment

Uh oh!

lockewritesdocs commented Aug 27, 2020

Uh oh!

jloleysens commented Aug 31, 2020

Uh oh!

jloleysens left a comment

Choose a reason for hiding this comment

Uh oh!

jrodewig left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jrodewig Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

lockewritesdocs Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

lockewritesdocs commented Sep 2, 2020

Uh oh!

jloleysens Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

lockewritesdocs commented Sep 2, 2020

Uh oh!

jloleysens commented Sep 4, 2020

Uh oh!

lockewritesdocs commented Sep 4, 2020

Uh oh!

elasticmachine commented Sep 4, 2020

Uh oh!

kibanamachine commented Sep 4, 2020

💚 Build Succeeded

Build metrics

async chunks size

History

Uh oh!

kibanamachine commented Sep 8, 2020

Uh oh!

lockewritesdocs commented Sep 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lockewritesdocs commented Aug 26, 2020 •

edited

Loading

jrodewig left a comment •

edited

Loading