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

[Security Solution][Endpoint] Artifacts event filter card on integration policy edit view #121879

Conversation

ashokaditya
Copy link
Member

@ashokaditya ashokaditya commented Dec 22, 2021

Summary

For each integration policy under Endpoint Security integration, this PR adds an artifact event filters summary card.

Screenshot 2021-12-22 at 14 53 36

Checklist

Delete any items that are not applicable to this PR.

@ashokaditya ashokaditya self-assigned this Dec 22, 2021
@ashokaditya ashokaditya added Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.1.0 release_note:feature Makes this part of the condensed release notes auto-backport Deprecated: Automatically backport this PR after it's merged labels Dec 22, 2021
@ashokaditya ashokaditya force-pushed the task/olm-artifact_event_filter_card_integration_policy_edit-2031 branch 4 times, most recently from 1ba29a4 to 505ac94 Compare December 22, 2021 16:14
@academo
Copy link
Contributor

academo commented Dec 31, 2021

General advice: don't use the "wrapper" approach for this functionality if you can save it. It makes things more complicated than they should be. I'd rather have two separate components, one for the fleet "advance settings" and other for this specific view. I saw how the code got complex in trusted apps and in general you are only reusing one variable.

@ashokaditya ashokaditya force-pushed the task/olm-artifact_event_filter_card_integration_policy_edit-2031 branch 2 times, most recently from 5c6b490 to 15de6c0 Compare January 4, 2022 14:33
@ashokaditya ashokaditya force-pushed the task/olm-artifact_event_filter_card_integration_policy_edit-2031 branch from 8621eb9 to 1c25b10 Compare January 5, 2022 09:38
@ashokaditya ashokaditya marked this pull request as ready for review January 5, 2022 09:39
@ashokaditya ashokaditya requested a review from a team as a code owner January 5, 2022 09:39
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

Comment on lines 48 to +50
}>`
font-size: ${({ isSmall, theme }) => (isSmall ? theme.eui.euiFontSizeXS : 'innherit')};
font-weight: ${({ isSmall }) => (isSmall ? '1px' : 'innherit')};
font-size: ${({ isSmall, theme }) => (isSmall ? theme.eui.euiFontSizeXS : 'inherit')};
font-weight: ${({ isSmall }) => (isSmall ? '1px' : 'inherit')};
Copy link
Member Author

Choose a reason for hiding this comment

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

I missed this in an earlier review! 😅

Copy link
Contributor

@dasansol92 dasansol92 left a comment

Choose a reason for hiding this comment

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

Thanks for handling this! Left two small comments

@@ -351,7 +351,7 @@ export const getPolicyDetailsArtifactsListPath = (
)}`;
};

export const extractEventFiltetrsPageLocation = (
export const extractEventFiltersPageLocation = (
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

`/app/security/administration/policy/${policyId}/eventFilters`
);
});
});
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 test case for the error handling? A test that checks a toast is shown when there is an API error.

Copy link
Member Author

Choose a reason for hiding this comment

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

done 3bc3a04

@ashokaditya ashokaditya force-pushed the task/olm-artifact_event_filter_card_integration_policy_edit-2031 branch from 659b71b to 3bc3a04 Compare January 5, 2022 19:05
Copy link
Contributor

@paul-tavares paul-tavares left a comment

Choose a reason for hiding this comment

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

👍

@ashokaditya
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Contributor

@parkiino parkiino left a comment

Choose a reason for hiding this comment

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

Checked it out and it looks goood to meee 🐑 🚢

@ashokaditya ashokaditya enabled auto-merge (squash) January 6, 2022 18:38
@ashokaditya
Copy link
Member Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Test Failures

  • [job] [logs] Default CI Group #1 / APM API tests trial apm_mappings_only_8.0.0 fetching service anomalies with a trial license with ml jobs "before all" hook for "returns a 200 for a user with access to ML"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2838 2839 +1

Async chunks

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

id before after diff
securitySolution 4.6MB 4.6MB +2.1KB

History

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

cc @ashokaditya

@ashokaditya ashokaditya merged commit 34d7ca6 into elastic:main Jan 6, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jan 6, 2022
@kibanamachine
Copy link
Contributor

💔 Backport failed

The backport operation could not be completed due to the following error:
You must specify a valid Github repository

You can specify it via either:

The backport PRs will be merged automatically after passing CI.

To backport manually run:
node scripts/backport --pr 121879

@ashokaditya ashokaditya deleted the task/olm-artifact_event_filter_card_integration_policy_edit-2031 branch January 7, 2022 08:13
@ashokaditya ashokaditya removed the auto-backport Deprecated: Automatically backport this PR after it's merged label Jan 7, 2022
gbamparop pushed a commit to gbamparop/kibana that referenced this pull request Jan 12, 2022
…ion policy edit view (elastic#121879)

* fix typo

refs elastic/security-team/issues/2031

* Add artifact event filters card to policy edit view on endpoint integration

fixes elastic/security-team/issues/2031

* add tests

fixes elastic/security-team/issues/2031

* fix typo

refs elastic/pull/111708

* use `eventFiltersListQueryHttpMock` instead

review suggestion

* add a test for verifying error toast

review suggestion

* fix tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
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:feature Makes this part of the condensed release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants