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][Exception]: Add to shared lists fixes #146750

Merged
merged 13 commits into from
Dec 2, 2022

Conversation

WafaaNasr
Copy link
Contributor

@WafaaNasr WafaaNasr commented Nov 30, 2022

Summary

  • Continuing from PR to apply the same changes to the Add to Shared Lists.
  • Fix showing the number of Linked rules correctly => in route.ts use the list.namespaceType instead of namespaceTypes array
  • Apply docs comment on the text
  • Use the HeaderMenu item from the kbn package for the Number of Linked rules menu
  • Allow displaying the HeaderMenu without iconType
  • Update snapshots and add tests in HeaderMenu

@WafaaNasr WafaaNasr self-assigned this Nov 30, 2022
@WafaaNasr WafaaNasr added release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed ci:cloud-deploy Create or update a Cloud deployment v8.6.0 Team:Security Solution Platform Security Solution Platform Team labels Nov 30, 2022
@@ -425,7 +425,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
return (
<EuiFlyout
ownFocus
maskProps={{ style: 'z-index: 5000' }} // For an edge case to display above the timeline flyout
maskProps={{ style: 'z-index: 1000' }} // For an edge case to display above the timeline flyout
Copy link
Contributor Author

@WafaaNasr WafaaNasr Dec 1, 2022

Choose a reason for hiding this comment

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

Adding this line for testing the PR on then Cloud env, then will revert it back until we fix it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted back 208a29e after testing

@WafaaNasr WafaaNasr marked this pull request as ready for review December 1, 2022 13:54
@WafaaNasr WafaaNasr requested a review from a team as a code owner December 1, 2022 13:54
@WafaaNasr WafaaNasr requested a review from a team as a code owner December 1, 2022 17:37
...rest
}) => ({
...rest,
namespace_type: namespaceType ?? 'single',
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this transformation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I understood is that in order to send namespacetype: single by default, if the list.namespace had a falsy value and in order to do so in the tsx if we used the snack case we will receive the below error
Parameter name namespace_type must match one of the following formats: camelCase, PascalCase

@kibana-ci
Copy link
Collaborator

kibana-ci commented Dec 2, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 3318 3320 +2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/securitysolution-exception-list-components 91 93 +2

Async chunks

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

id before after diff
securitySolution 9.6MB 9.8MB +166.8KB
Unknown metric groups

API count

id before after diff
@kbn/securitysolution-exception-list-components 102 104 +2

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 59 65 +6
osquery 109 115 +6
securitySolution 442 448 +6
total +20

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 68 74 +6
osquery 110 117 +7
securitySolution 519 525 +6
total +21

History

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

cc @WafaaNasr

@WafaaNasr WafaaNasr merged commit 78b4851 into elastic:main Dec 2, 2022
@WafaaNasr WafaaNasr deleted the fix-add-to-sharedlist branch December 2, 2022 14:02
kibanamachine pushed a commit that referenced this pull request Dec 2, 2022
## Summary

- Continuing from [PR](#146121) to
apply the same changes to the `Add to Shared Lists`.
- Fix showing the number of Linked rules correctly => in `route.ts` use
the `list.namespaceType` instead of namespaceTypes array
- Apply docs comment on the text
- Use the HeaderMenu item from the `kbn` package for the `Number of
Linked rules` menu
- Allow displaying the HeaderMenu without iconType
- Update snapshots and add tests in HeaderMenu

(cherry picked from commit 78b4851)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.6

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

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

WafaaNasr added a commit that referenced this pull request Dec 2, 2022
) (#146887)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[Security Solution][Exception]: Add to shared lists fixes
(#146750)](#146750)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Wafaa
Nasr","email":"[email protected]"},"sourceCommit":{"committedDate":"2022-12-02T14:02:33Z","message":"[Security
Solution][Exception]: Add to shared lists fixes (#146750)\n\n##
Summary\r\n\r\n- Continuing from
[PR](#146121) to\r\napply the same
changes to the `Add to Shared Lists`.\r\n- Fix showing the number of
Linked rules correctly => in `route.ts` use\r\nthe `list.namespaceType`
instead of namespaceTypes array\r\n- Apply docs comment on the text\r\n-
Use the HeaderMenu item from the `kbn` package for the `Number
of\r\nLinked rules` menu\r\n- Allow displaying the HeaderMenu without
iconType\r\n- Update snapshots and add tests in
HeaderMenu","sha":"78b4851a214e5018c8ea6535477ca9374ffb377f","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","auto-backport","Team:Security
Solution
Platform","ci:cloud-deploy","v8.6.0"],"number":146750,"url":"https://github.com/elastic/kibana/pull/146750","mergeCommit":{"message":"[Security
Solution][Exception]: Add to shared lists fixes (#146750)\n\n##
Summary\r\n\r\n- Continuing from
[PR](#146121) to\r\napply the same
changes to the `Add to Shared Lists`.\r\n- Fix showing the number of
Linked rules correctly => in `route.ts` use\r\nthe `list.namespaceType`
instead of namespaceTypes array\r\n- Apply docs comment on the text\r\n-
Use the HeaderMenu item from the `kbn` package for the `Number
of\r\nLinked rules` menu\r\n- Allow displaying the HeaderMenu without
iconType\r\n- Update snapshots and add tests in
HeaderMenu","sha":"78b4851a214e5018c8ea6535477ca9374ffb377f"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Wafaa Nasr <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed ci:cloud-deploy Create or update a Cloud deployment release_note:skip Skip the PR/issue when compiling release notes Team:Security Solution Platform Security Solution Platform Team v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants