-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Fleet] Add view agents button to activity flyout #152555
Conversation
@elasticmachine merge upstream |
I'm opening the PR even though there are still some issues that I need to fix before merging, but I would like to have some early reviews . In local testing I'm having cases where the Also the "clear filters" button added on top of the table is almost always visible, as it's tied to the existing |
Pinging @elastic/fleet (Team:Fleet) |
Yes, update tags action doesn't have real agent ids, see the end of this comment: #141206 (comment) We might want to hide the |
...blic/applications/fleet/sections/agents/agent_list_page/components/agent_activity_flyout.tsx
Outdated
Show resolved
Hide resolved
|
||
import { getKuery } from './get_kuery'; | ||
|
||
describe('getKuery', () => { |
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.
great refactor with tests!
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.
Thank you! :D
So should we even display the "view agents" button in this case? I could hide it in the case of tag updating. -edit I re-read and realised that you already answered :) Also, do you know if we have other cases similar to this where the action doesn't hold the correct agent ids? Since the agent activity covers all type of actions we need to be sure that it work properly in all those cases. |
Yes I noticed it as well. I'll remove all the filters when clicking the button and this will solve also the weird behaviour of the "clear filters" link. |
@juliaElastic do you think that passing the kuery instead of the agent ids will be enough to handle the bulk actions case? you were suggesting to do this ticket in two passes but maybe this will be enough? |
Passing the query would work in most cases, though there are some edge cases where it wouldn't e.g. if agent version was included in the filter, and the agents were upgraded, or if |
setSearch(kuery); | ||
} | ||
if (action?.type === 'UNENROLL' || action?.type === 'FORCE_UNENROLL') { | ||
setSelectedStatus(['unenrolled', 'inactive']); |
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 it mean that only unenrolled
and inactive
statuses are visible or all statuses? I'll try it locally. Ideally it should be all statuses, so agents are visible if unenroll is in progress. For simple unenroll, agents will move to Offline
at first.
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 you're right, I probably need to add all of them. I'm not sure if I need to add them in all cases or only for unenrolled agents.
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.
It could be for all cases. Does the Clear filters
button reset to default filters?
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.
That applies the clearFIlters
function here:
kibana/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/index.tsx
Lines 116 to 123 in 662fd85
const clearFilters = useCallback(() => { | |
setDraftKuery(''); | |
setSearch(''); | |
setSelectedAgentPolicies([]); | |
setSelectedStatus([]); | |
setSelectedTags([]); | |
setShowUpgradeable(false); | |
}, [setSearch, setDraftKuery, setSelectedAgentPolicies, setSelectedStatus, setShowUpgradeable]); |
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 changed so that it makes all possible statuses visible for all cases. I think I had misunderstood how this filtering works.
...blic/applications/fleet/sections/agents/agent_list_page/components/agent_activity_flyout.tsx
Outdated
Show resolved
Hide resolved
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, tested locally
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @criamico |
Part of #141206 ## Summary ### Server side: - Create a new API that returns Agents by actions Ids: ``` POST kbn:/api/fleet/agents { actionIds: [ 'action1', 'action2' ] } ``` ### UI: Add "view agents" button to activity flyout; when clicking on it, the button will take the user to the agent list and display only the subset of agents affected by the action. <img width="1100" alt="Screenshot 2023-03-09 at 16 27 41" src="https://user-images.githubusercontent.com/16084106/224072551-bf7b6cf3-9f32-4a79-8e61-d7dc35f4db54.png"> Also addiing a "clear filters" on top of the header to be able to remove the applied filter after the button is selected I also did some refactoring of the `AgentListPage` component, mostly extracted some small components and the `kueryBuilder` function, that became its own function and it's also been tested. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Part of #141206
Summary
Server side:
UI:
Add "view agents" button to activity flyout; when clicking on it, the button will take the user to the agent list and display only the subset of agents affected by the action.
Also addiing a "clear filters" on top of the header to be able to remove the applied filter after the button is selected
I also did some refactoring of the
AgentListPage
component, mostly extracted some small components and thekueryBuilder
function, that became its own function and it's also been tested.Checklist