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

[Fleet] refresh policies on create and flyout open #125499

Merged
merged 3 commits into from
Feb 14, 2022
Merged

[Fleet] refresh policies on create and flyout open #125499

merged 3 commits into from
Feb 14, 2022

Conversation

juliaElastic
Copy link
Contributor

@juliaElastic juliaElastic commented Feb 14, 2022 •
edited
Loading

Summary

Fixes #125481

Steps to reproduce:

  1. Navigate to Fleet>Agents tab (without any agent policies created, only one Fleet Server policy should be there with Fleet Server enrolled)
  2. Click Add Agent flyout and click Create policy with default name- Agent Policy 1 (both Enroll in Agent and Standalone tab)
  3. Navigate to Agent policies and observe Agent Policy 1 is created.
  4. Navigate back to Agents tab and click Add Agent flyout.
  5. Dropdown should be visible with Agent Policy 1

@juliaElastic juliaElastic added release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v8.1.0 v8.2.0 labels Feb 14, 2022
@juliaElastic juliaElastic requested a review from a team as a code owner February 14, 2022 13:04
@juliaElastic juliaElastic self-assigned this Feb 14, 2022
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Feb 14, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

juliaElastic
juliaElastic commented Feb 14, 2022
View reviewed changes
x-pack/plugins/fleet/public/components/agent_enrollment_flyout/index.tsx
data: agentPoliciesData,
isLoading: isLoadingAgentPolicies,
resendRequest: refreshAgentPolicies,
} = useGetAgentPolicies({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to load/refresh agent policies here in AgentEnrollmentFlyout.
The reason is that AgentEnrollmentFlyout is being called from 7 places, from which 4 are used without agentPolicies, 3 with agentPolicies. It would be too much duplication to reload agentPolicies from all those caller sites.
AgentEnrollmentFlyout Props should be cleaned up so that agentPolicies is not passed as a prop.

Copy link
Member

Choose a reason for hiding this comment

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

What do you think of creating a hook like useAgentEnrollmentFlyoutData that contains this and the useMemo below, to start separating more business and presentation in this component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to create a hook in a next pr, also clean up some unused props

juliaElastic
juliaElastic commented Feb 14, 2022
View reviewed changes
x-pack/plugins/fleet/public/components/agent_enrollment_flyout/standalone_instructions.tsx

type Props = BaseProps;
export const StandaloneInstructions = React.memo<InstructionProps>(
({ agentPolicy, agentPolicies, refreshAgentPolicies }) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the only change in this file is refreshAgentPolicies, the others are whitespace

@nchaulet nchaulet self-requested a review February 14, 2022 14:24
nchaulet
nchaulet approved these changes Feb 14, 2022
View reviewed changes
Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

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

It works as expected 🚀!

@juliaElastic
Copy link
Contributor Author

juliaElastic commented Feb 14, 2022 •
edited
Loading

there is one thing I broke with this change:

With existing agent policies in Add Agent flyout, click on Create new agent policy link.
After the new policy is created, the dropdown doesn't preselect it, this was working before.
I'm trying to fix this.

image

@juliaElastic
Copy link
Contributor Author

juliaElastic commented Feb 14, 2022 •
edited
Loading

there is one thing I broke with this change:

With existing agent policies in Add Agent flyout, click on Create new agent policy link. After the new policy is created, the dropdown doesn't preselect it, this was working before. I'm trying to fix this.

This is fixed
image

@juliaElastic juliaElastic enabled auto-merge (squash) February 14, 2022 16:26
@juliaElastic juliaElastic merged commit b4855ba into elastic:main Feb 14, 2022
@kibana-ci
Copy link
Collaborator

💚 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 660.7KB 660.9KB +205.0B

History

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

cc @juliaElastic

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 14, 2022
@juliaElastic @kibanamachine
[Fleet] refresh policies on create and flyout open (elastic#125499)
0d18ba9 
* refresh policies on create and flyout open

* fix checks

* fixed scenario when newly created policy should be preselected

(cherry picked from commit b4855ba)
@kibanamachine kibanamachine mentioned this pull request Feb 14, 2022
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.1

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Feb 14, 2022
@kibanamachine @juliaElastic
[Fleet] refresh policies on create and flyout open (#125499) (#125544)
1086f45 
* refresh policies on create and flyout open

* fix checks

* fixed scenario when newly created policy should be preselected

(cherry picked from commit b4855ba)

Co-authored-by: Julia Bardi <[email protected]>
@juliaElastic juliaElastic mentioned this pull request Feb 15, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nchaulet nchaulet nchaulet approved these changes

Assignees

@juliaElastic juliaElastic

Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.1.0 v8.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Fleet]: Agent policy name not incremented itself when one policy is created with system settings off.
5 participants
@juliaElastic @elasticmachine @kibana-ci @kibanamachine @nchaulet