-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[Ingest] Agent config settings UI #64854
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.
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
Conversation
…clean up import paths
…ssociated data sources
|
Pinging @elastic/ingest-management (Team:Ingest Management) |
jfsiii
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have only reviewed on my phone, but it looks very good.
| <EuiButtonEmpty | ||
| color="ghost" | ||
| onClick={() => { | ||
| setAgentConfig({ ...originalAgentConfig }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, why do we make a copy here?
| const refreshConfig = useConfigRefresh(); | ||
| const [isNavDrawerLocked, setIsNavDrawerLocked] = useState(false); | ||
| const [agentConfig, setAgentConfig] = useState<AgentConfig>({ | ||
| ...originalAgentConfig, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be a copy of the object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not really, just a personal habit when working with objects that are passed by reference (though in this context with useState it doesn't matter much)
| public async delete( | ||
| soClient: SavedObjectsClientContract, | ||
| ids: string[], | ||
| unassignFromAgentConfigs: boolean = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should go into the options parameter.
We can still default it to true/whatever and it's backwards compatible.
This reminded me of https://ariya.io/2011/08/hall-of-api-shame-boolean-trap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adjusted
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
hbharding
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
* Remove duplicate donut chart, move used donut chart closer to usage, clean up import paths * Change delete agent config to only single delete and delete all its associated data sources * Initial pass at settings tab * Reuse existing create agent config form instead * Fix delete api * Prevent nav drawer from hiding bottom bar (save action area) content * Remove delete config functionality from list page * Prevent API from deleting config with agents enrolled * Fix namespace populating in form * Display confirmation modal to deploy to agents if agents are detected * Adjust confirm delete copy * Fix i18n checks * Fix type check * Fix it again * De-dupe confirm modal * Fix i18n * Update agent config info after saving * Adjust skip unassign from agent config option schema in delete datasource method
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
* Remove duplicate donut chart, move used donut chart closer to usage, clean up import paths * Change delete agent config to only single delete and delete all its associated data sources * Initial pass at settings tab * Reuse existing create agent config form instead * Fix delete api * Prevent nav drawer from hiding bottom bar (save action area) content * Remove delete config functionality from list page * Prevent API from deleting config with agents enrolled * Fix namespace populating in form * Display confirmation modal to deploy to agents if agents are detected * Adjust confirm delete copy * Fix i18n checks * Fix type check * Fix it again * De-dupe confirm modal * Fix i18n * Update agent config info after saving * Adjust skip unassign from agent config option schema in delete datasource method Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Resolves #57959
Summary
This PR implements functionality to edit an agent config by using the existing form component for creating an agent config in the Settings tab of config details page:
Screenshots
Expand me for more screenshots
.
Save changes bottom bar appears after form is dirty:
Save changes modal when agents are enrolled (no modal is shown if no agents are enrolled):
Success notification:
Delete config modal when no agents are enrolled:
Delete config modal when agents are enrolled:
Delete disabled for default config: