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.

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] Added modal to manage agent policies of an integration policy #186987

Merged
merged 19 commits into from
Jul 2, 2024

Conversation

juliaElastic
Copy link
Contributor

@juliaElastic juliaElastic commented Jun 26, 2024

Summary

Closes #182112

Added modal to manage agent policies.

To verify:

  • go to an integration where the integration policies are listed
  • click on the + button in the agent policies column
  • click on Manage agent policies in the popover
  • add/remove agent policies in the modal
  • click submit, the integration policy should be updated to be linked to the updated agent policies
image

Made a change to the table column display to show a + button even if there is only one policy. Previously the popover could only be accessed if there are at least 2 agent policies.
Also restored the agent policy link, lock icon and revision display (instead of a badge) if there are multiple agent policies.
@simosilvestri Let me know if you have any UX recommendation as it differs slightly from the prototype.

image image

EDIT: after discussing with Simona, removed the + button in case of a single agent policy assigned to the integration policy.
image

Disabling Manage agent policies button if the current user doesn't have at least write integration policies and write agent policies privilege.
This is how it looks with read privileges:
image

Checklist

@juliaElastic juliaElastic added the release_note:skip Skip the PR/issue when compiling release notes label Jun 26, 2024
@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@juliaElastic
Copy link
Contributor Author

/ci

@juliaElastic
Copy link
Contributor Author

/ci

@juliaElastic
Copy link
Contributor Author

/ci

@juliaElastic juliaElastic self-assigned this Jun 27, 2024
@simosilvestri
Copy link

Hi @juliaElastic, as we discussed, we can remove the badge with the plus icon when displaying a single Agent policy, since we expect that the majority of customers will go back to the Integration wizard to edit their settings.

@juliaElastic
Copy link
Contributor Author

/ci

@juliaElastic juliaElastic marked this pull request as ready for review June 27, 2024 12:20
@juliaElastic juliaElastic requested a review from a team as a code owner June 27, 2024 12:20
@juliaElastic
Copy link
Contributor Author

juliaElastic commented Jun 27, 2024

There are 2 remaining issues that I noticed:

  • after submitting the Manage agent policies modal, the Integration list UI doesn't refresh with the changes, should refresh automatically - this is fixed
  • the managed agent policies are not showing up in the combo box (this is an existing behaviour). I think they should be visible and shouldn't be removed
    • looked into this to show the managed policies in the combo, but didn't find a way to prevent removing them. Instead I went with a solution to keep them hidden, but always keep them in the policy_ids list, so the managed policies are not removed when updating an integration policy.
image image

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jun 27, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@jen-huang jen-huang self-requested a review June 28, 2024 18:04
@criamico criamico self-requested a review July 2, 2024 07:04
@criamico
Copy link
Contributor

criamico commented Jul 2, 2024

I'm testing locally and I have two small observations about this modal:

  • When loading, the modal size briefly changes, it's not a big issue but it looks weird. It seems to happen when the modal loads the policies, so having a fixed min-width or something like this could help. I captured it in a quick video:
Screen.Recording.2024-07-02.at.10.04.26.mov
  • The font size in the callout description is a bit too big (looks the same as the title)

Screenshot 2024-07-02 at 10 05 06

import { isPackageLimited } from '../../../../../../../../../common/services';
import { useGetAgentPolicies, useGetOutputs, useGetPackagePolicies } from '../../../../../../hooks';

export function useAgentPoliciesOptions(packageInfo?: PackageInfo) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for extracting this function!

<EuiText>
<FormattedMessage
id="xpack.fleet.manageAgentPolicies.confirmModalDescription"
defaultMessage="Agent policies which share this integration"
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it would sound better Agent policies sharing this integration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea, updated

@juliaElastic
Copy link
Contributor Author

Added min-width to the modal, so it shouldn't jump (it happened with at least 3 policies).
Also decreased the callout description text size:
image

Copy link
Contributor

@criamico criamico left a comment

Choose a reason for hiding this comment

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

Tested with the new changes, LGTM 🚢

@juliaElastic juliaElastic enabled auto-merge (squash) July 2, 2024 09:30
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Integration Tests #3 / migration actions checkClusterRoutingAllocation resolves left when cluster.routing.allocation.enabled is incompatible
  • [job] [logs] Jest Integration Tests #3 / migration actions checkClusterRoutingAllocation resolves right when cluster.routing.allocation.enabled=all
  • [job] [logs] Jest Integration Tests #3 / migration actions cloneIndex resolves left cluster_shard_limit_exceeded when the action would exceed the maximum normal open shards
  • [job] [logs] Jest Integration Tests #3 / migration actions cloneIndex resolves left index_not_found_exception if the source index does not exist
  • [job] [logs] Jest Integration Tests #3 / migration actions cloneIndex resolves left with a index_not_green_timeout if clone target already exists but takes longer than the specified timeout before turning green
  • [job] [logs] Jest Integration Tests #3 / migration actions cloneIndex resolves right if clone target already existed after waiting for index status to be green
  • [job] [logs] Jest Integration Tests #3 / migration actions cloneIndex resolves right if cloning into a new target index
  • [job] [logs] Jest Integration Tests #3 / migration actions fetchIndices includes the _meta data of the indices in the response
  • [job] [logs] Jest Integration Tests #3 / migration actions fetchIndices resolves right empty record if no indices were found
  • [job] [logs] Jest Integration Tests #3 / migration actions fetchIndices resolves right record with found indices
  • [job] [logs] Jest Integration Tests #3 / migration actions removeWriteBlock rejects if there is a non-retryable error
  • [job] [logs] Jest Integration Tests #3 / migration actions removeWriteBlock resolves right if successful when an index already has a write block
  • [job] [logs] Jest Integration Tests #3 / migration actions removeWriteBlock resolves right if successful when an index does not have a write block
  • [job] [logs] Jest Integration Tests #3 / migration actions setWriteBlock once resolved, prevents further writes to the index
  • [job] [logs] Jest Integration Tests #3 / migration actions setWriteBlock resolves left index_not_found_exception when the index does not exist
  • [job] [logs] Jest Integration Tests #3 / migration actions setWriteBlock resolves right when setting a write block on an index that already has one
  • [job] [logs] Jest Integration Tests #3 / migration actions setWriteBlock resolves right when setting the write block succeeds
  • [job] [logs] Jest Integration Tests #3 / migration actions waitForIndexStatus resolves left with "index_not_green_timeout" after waiting for an index status to be green timeout
  • [job] [logs] Jest Integration Tests #3 / migration actions waitForIndexStatus resolves left with "index_not_yellow_timeout" after waiting for an index status to be yellow timeout
  • [job] [logs] Jest Integration Tests #3 / migration actions waitForIndexStatus resolves right after waiting for an index status to be yellow if the index already existed

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
fleet 1206 1208 +2

Async chunks

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

id before after diff
fleet 1.8MB 1.8MB +5.3KB

Page load bundle

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

id before after diff
fleet 167.2KB 167.2KB -9.0B
Unknown metric groups

async chunk count

id before after diff
fleet 11 10 -1

History

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

cc @juliaElastic

@juliaElastic juliaElastic merged commit 813fc21 into elastic:main Jul 2, 2024
21 checks passed
@kibanamachine kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Manage agent policies action
7 participants