Edit text strings and links in add logstash output UI #129551

dedemorton · 2022-04-06T02:34:15Z

Summary

Related to #129131

@nchaulet Gail and I came up with most of these edits together. i want her to confirm my changes before you review this. I wasn't sure if I was supposed to update the test files, too. I did, but might have missed some.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support

Open issues:

How will users provide feedback and report bugs? Discuss? GitHub? We already have a Send feedback link on the page, so we should direct users to that unless we prefer the discuss forum.
The links here go to the same page. We should remove one. I think we should remove the button.
I'm not sure if my "Invalid logstash host..." edit is correct because I couldn't understand the original message. IMO it's better to tell users what they should do, not what they shouldn't do.

Possible bugs:

When you edit the output and change the type to Logstash, the URL specified is invalid. I think this might be a bug because the field has the Elasticsearch URL--maybe the field is not being reset.
If you try to select the URL from here ^^ by dragging and highlighting, the whole flyout closes.

Deferred for later:

~~Users need more guidance about what is valid in the certs fields. Do we expect them to paste in a block? Provide a path? Can we show an example?~~ There's a different issue to track this work for 8.2, so I am marking this off.
There are quite a few places in the Fleet UI where we use really generic links. I want to go through the UI at some point, evaluate the links, and identify any content gaps that are causing us to create awkward links.
Edit validation errors (add periods).

Updated screen

...ns/fleet/public/applications/fleet/sections/settings/components/edit_output_flyout/index.tsx

...ations/fleet/sections/settings/components/edit_output_flyout/output_form_validators.test.tsx

nchaulet · 2022-04-06T18:14:02Z

The default URL shown for the Logstash host field is invalid. We need to change the default or fix the validation:

Yes we need to change the placeholder, there is no protocol for logstash host

Users need more guidance about what is valid in the certs fields. Do we expect them to paste in a block? Provide a path? Can we show an example?

We expect them to paste the certificate block something like, I guess we can update the placeholder with more info

---BEGIN RSA
certificate...
---END RSA

The links here go to the same page. We should remove one. I think we should remove the button

Yes I think removing the button make sense too @dborodyansky does it make sense to you?

I'm not sure if my "Invalid logstash host..." edit is correct because I couldn't understand the original message. IMO it's better to tell users what they should do, not what they shouldn't do.

The logstash host should be a domain or ip + port, there is no protocol, I think your message make sense (maybe not use the URL term as there is no protocol)

Validation errors need to end in a period, but I didn't make the updates because the errors are consistent with other parts of the Fleet UI. We should fix it all at once.

What do you mean here? if you correct the validation error it should disappear.

dborodyansky · 2022-04-06T18:38:46Z

The links here go to the same page. We should remove one. I think we should remove the button

Yes I think removing the button make sense too @dborodyansky does it make sense to you?

Agree

dedemorton · 2022-04-06T18:46:50Z

@nchaulet Re:

Validation errors need to end in a period, but I didn't make the updates because the errors are consistent with other parts of the Fleet UI. We should fix it all at once.

What do you mean here? if you correct the validation error it should disappear.

I was commenting on a style issue. Validation messages should end with a period (.) for punctuation. :-) But I didn't fix this because it's better to just do all the validation messages in one edit so they are consistent across the UI.

...ns/fleet/public/applications/fleet/sections/settings/components/edit_output_flyout/index.tsx

...pplications/fleet/sections/settings/components/edit_output_flyout/output_form_validators.tsx

...fleet/public/applications/fleet/sections/settings/components/logstash_instructions/index.tsx

x-pack/plugins/fleet/server/types/models/output.test.ts

x-pack/plugins/fleet/server/types/models/output.ts

...ations/fleet/sections/settings/components/edit_output_flyout/output_form_validators.test.tsx

...ns/fleet/public/applications/fleet/sections/settings/components/edit_output_flyout/index.tsx

dedemorton · 2022-04-07T01:55:54Z

This is ready for review, though the CI might fail...if it does, I'll fix it. Just wanted to get this up for review.

...ns/fleet/public/applications/fleet/sections/settings/components/edit_output_flyout/index.tsx

elasticmachine · 2022-04-07T02:05:06Z

Pinging @elastic/fleet (Team:Fleet)

nchaulet

LGTM 🚀 (I took the liberty to push a commit to fix tests)

dedemorton · 2022-04-07T18:27:51Z

Ah, I see the problem here. I will fix it.

gchaps

LGTM

kibana-ci · 2022-04-07T20:10:50Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`fleet`	684.6KB	684.3KB	-270.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`core`	318.7KB	318.8KB	+54.0B

History

💔 Build #36934 failed dc5e7f4
💔 Build #36849 failed d43fb07
💔 Build #36703 failed 5e31076
💔 Build #36550 failed 31849f8
💔 Build #36349 failed f08275a

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

cc @dedemorton

* Edit text strings and links in add logstash output UI * fix test to match * Add changes from review * Fix tests * Fix type * Fix I18N error Co-authored-by: Nicolas Chaulet <[email protected]> Co-authored-by: gchaps <[email protected]> (cherry picked from commit a583edc)

kibanamachine · 2022-04-07T20:30:41Z

💚 All backports created successfully

Status	Branch	Result
✅	8.2

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

) * Edit text strings and links in add logstash output UI * fix test to match * Add changes from review * Fix tests * Fix type * Fix I18N error Co-authored-by: Nicolas Chaulet <[email protected]> Co-authored-by: gchaps <[email protected]> (cherry picked from commit a583edc) Co-authored-by: DeDe Morton <[email protected]>

Edit text strings and links in add logstash output UI

6fe9a39

dedemorton requested a review from gchaps April 6, 2022 02:34

dedemorton self-assigned this Apr 6, 2022

dedemorton commented Apr 6, 2022

View reviewed changes

...ns/fleet/public/applications/fleet/sections/settings/components/edit_output_flyout/index.tsx Outdated Show resolved Hide resolved

dedemorton requested a review from nchaulet April 6, 2022 02:37

dedemorton commented Apr 6, 2022

View reviewed changes

...ations/fleet/sections/settings/components/edit_output_flyout/output_form_validators.test.tsx Outdated Show resolved Hide resolved

dedemorton added 2 commits April 5, 2022 19:42

fix test to match

f08275a

Merge branch 'main' into edit_add_logstash_output_strings

31849f8

gchaps reviewed Apr 6, 2022

View reviewed changes

Add changes from review

2e4dc70

dedemorton commented Apr 7, 2022

View reviewed changes

...ns/fleet/public/applications/fleet/sections/settings/components/edit_output_flyout/index.tsx Show resolved Hide resolved

Merge branch 'main' into edit_add_logstash_output_strings

5e31076

dedemorton marked this pull request as ready for review April 7, 2022 01:56

dedemorton requested review from a team as code owners April 7, 2022 01:56

dedemorton requested a review from gchaps April 7, 2022 01:56

dedemorton commented Apr 7, 2022

View reviewed changes

...ns/fleet/public/applications/fleet/sections/settings/components/edit_output_flyout/index.tsx Show resolved Hide resolved

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Apr 7, 2022

Fix tests

d43fb07

nchaulet approved these changes Apr 7, 2022

View reviewed changes

nchaulet added release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v8.2.0 v8.3.0 labels Apr 7, 2022

Fix type

dc5e7f4

Fix I18N error

da98897

gchaps approved these changes Apr 7, 2022

View reviewed changes

dedemorton merged commit a583edc into elastic:main Apr 7, 2022

dedemorton deleted the edit_add_logstash_output_strings branch April 7, 2022 20:27

kibanamachine mentioned this pull request Apr 7, 2022

[8.2] Edit text strings and links in add logstash output UI (#129551) #129795

Merged

dedemorton mentioned this pull request Apr 13, 2023

[Request] Review error messages and doc links in Fleet UI elastic/ingest-docs#110

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Edit text strings and links in add logstash output UI #129551

Edit text strings and links in add logstash output UI #129551

dedemorton commented Apr 6, 2022 •

edited

Loading

nchaulet commented Apr 6, 2022

dborodyansky commented Apr 6, 2022

dedemorton commented Apr 6, 2022

dedemorton commented Apr 7, 2022

elasticmachine commented Apr 7, 2022

nchaulet left a comment

dedemorton commented Apr 7, 2022

gchaps left a comment

kibana-ci commented Apr 7, 2022

kibanamachine commented Apr 7, 2022

Edit text strings and links in add logstash output UI #129551

Edit text strings and links in add logstash output UI #129551

Conversation

dedemorton commented Apr 6, 2022 • edited Loading

Summary

Checklist

Open issues:

Possible bugs:

Deferred for later:

Updated screen

nchaulet commented Apr 6, 2022

dborodyansky commented Apr 6, 2022

dedemorton commented Apr 6, 2022

dedemorton commented Apr 7, 2022

elasticmachine commented Apr 7, 2022

nchaulet left a comment

Choose a reason for hiding this comment

dedemorton commented Apr 7, 2022

gchaps left a comment

Choose a reason for hiding this comment

kibana-ci commented Apr 7, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

kibanamachine commented Apr 7, 2022

💚 All backports created successfully

Questions ?

dedemorton commented Apr 6, 2022 •

edited

Loading