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

Jump to bottom

[INGEST_MANAGER] Make package config name blank for endpoint on Package Config create #73082

Merged
merged 4 commits into from
Jul 24, 2020
Merged

[INGEST_MANAGER] Make package config name blank for endpoint on Package Config create #73082

merged 4 commits into from
Jul 24, 2020

Conversation

paul-tavares
Copy link
Contributor

@paul-tavares paul-tavares commented Jul 23, 2020 •
edited
Loading

Summary

On Package Configuration Create flow, if the selected package is endpoint, do not generate a default package configuration name - which will force the user to define one manually.

emt-617-insgest-endpoint-package-config-create

@paul-tavares paul-tavares added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Management Feature:Endpoint Elastic Endpoint feature v7.9.0 Team:Fleet Team label for Observability Data Collection Fleet team labels Jul 23, 2020
@paul-tavares paul-tavares self-assigned this Jul 23, 2020
@paul-tavares paul-tavares marked this pull request as ready for review July 23, 2020 17:37
@paul-tavares paul-tavares requested a review from a team as a code owner July 23, 2020 17:37
@paul-tavares paul-tavares requested a review from a team July 23, 2020 17:37
@paul-tavares paul-tavares requested a review from a team as a code owner July 23, 2020 17:37
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-app-team (Feature:Endpoint)

@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-management (Team:Endpoint Management)

@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@kevinlog
Copy link
Contributor

LGTM, did we consider a placeholder for the integration name?

FYI @hbharding

jen-huang
jen-huang approved these changes Jul 23, 2020
View reviewed changes
Copy link
Contributor

@jen-huang jen-huang left a comment

Choose a reason for hiding this comment

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

To Kevin's comment, I wouldn't mind adding a placeholder attribute to the name input. This would help UX in general if user clears out suggested name for any other integration.

Small comment but otherwise LGTM

...gest_manager/sections/agent_config/create_package_config_page/step_define_package_config.tsx
dsWithMatchingNames.length ? dsWithMatchingNames[dsWithMatchingNames.length - 1] + 1 : 1
}`,
name:
// For Endpoint packages, the user must fill in the name, thus we don't attempt to generate
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a FIXME comment to improve this in the next release? :) I'm not sure if there is an issue filed but adding the URL of the issue would be great too.

@jen-huang
Copy link
Contributor

P.S. added 7.10 label too since 7.x targets 7.10 now.

@paul-tavares
Copy link
Contributor Author

@kevinlog I spoke with @hbharding on this. Placeholders sometimes cause confusion to the user in that they appear to actually be the value that is populated already. The idea is to put some more text below (or the left) of each field that provides the user with some more guidance/info. on what to use for that field.

I'm also waiting to hear from Eui Team - because if we can set focus to the name field (during initialization only) if it is blank would further improve the UX.

Thanks @jen-huang
I will add the FIXME. We have an issue that is tracking long term solution (I remember seeing something from Caitlin), but I think its in a private repo.

@paul-tavares
Copy link
Contributor Author

Found it - will reference this in the code: #72948

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
ingestManager 1.1MB +33.0B 1.1MB

History

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

@paul-tavares paul-tavares merged commit e341eaf into elastic:master Jul 24, 2020
@paul-tavares paul-tavares deleted the task/EMT-617-blank-ingest-package-config-name branch July 24, 2020 13:02
This was referenced Jul 24, 2020
Merged
Merged
paul-tavares added a commit to paul-tavares/kibana that referenced this pull request Jul 24, 2020
@paul-tavares
[INGEST_MANAGER] Make package config name blank for endpoint on Packa…
539a0d6 
…ge Config create (elastic#73082)

* Make package config name blank for endpoint
* Added functional tests on endpoint side
paul-tavares added a commit to paul-tavares/kibana that referenced this pull request Jul 24, 2020
@paul-tavares
[INGEST_MANAGER] Make package config name blank for endpoint on Packa…
e8db07f 
…ge Config create (elastic#73082)

* Make package config name blank for endpoint
* Added functional tests on endpoint side
paul-tavares added a commit that referenced this pull request Jul 24, 2020
@paul-tavares
[INGEST_MANAGER] Make package config name blank for endpoint on Packa…
26fdc46 
…ge Config create (#73082) (#73196)

* Make package config name blank for endpoint
* Added functional tests on endpoint side
paul-tavares added a commit that referenced this pull request Jul 24, 2020
@paul-tavares
[INGEST_MANAGER] Make package config name blank for endpoint on Packa…
f3b2077 
…ge Config create (#73082) (#73197)

* Make package config name blank for endpoint
* Added functional tests on endpoint side
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 27, 2020
@gmmorris
Merge branch 'master' into event-log/use-alerts-actions-clients
2d63a67 
* master: (111 commits)
  Remove flaky note from gauge tests (elastic#73240)
  Convert functional vega tests to ts and unskip tests (elastic#72238)
  [Graph] Unskip graph tests (elastic#72291)
  Add default Elasticsearch credentials to docs (elastic#72617)
  [APM] Read body from indicesStats in upload-telemetry-data (elastic#72732)
  The directory in the command was missing the /generated directory and would cause all definitions to be regenerated in the wrong place. (elastic#72766)
  [KP] use new ES client in SO service (elastic#72289)
  [Security Solution][Exceptions] Prevents value list entries from co-existing with non value list entries (elastic#72995)
  Return EUI CSS to Shareable Runtime (elastic#72990)
  Removed useless karma test (elastic#73190)
  [INGEST_MANAGER] Make package config name blank for endpoint on Package Config create (elastic#73082)
  [Ingest Manager] Support DEGRADED state in fleet agent event (elastic#73104)
  [Security Solution][Detections] Change detections breadcrumb title (elastic#73059)
  [ML] Fixing unnecessary deleting job polling (elastic#73087)
  [ML] Fixing recognizer wizard create job button (elastic#73025)
  [Composable template] Preview composite template (elastic#72598)
  [Uptime] Use manual intervals for ping histogram (elastic#72928)
  [Security Solution][Endpoint] Task/policy save modal text change, remove duplicate policy details text (elastic#73130)
  [Maps] fix tile layer attibution text and attribution link validation errors (elastic#73160)
  skip ingest pipeline api tests
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jen-huang jen-huang jen-huang approved these changes

@kevinlog kevinlog kevinlog approved these changes

Assignees

@paul-tavares paul-tavares

Labels
Feature:Endpoint Elastic Endpoint feature release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.9.0 v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@paul-tavares @elasticmachine @kevinlog @jen-huang @kibanamachine