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

[Rules migration] Add sorting functionality to rules migration table (#11379) #203396

Merged

e40pud merged 6 commits into elastic:main from e40pud:security/siem-migrations/11379-sorting

Dec 9, 2024

+541 −122

Contributor

e40pud commented Dec 9, 2024 •

edited by kibanamachine

Loading

Summary

Internal link to the feature details

These changes add sorting functionality to the migration rules table. It is possible to sort migration rules by next columns: Updated, Name, Status, Risk Score, Severity and Author.

Other changes

Next fixes and adjustments were also implemented as part of this PR:

Installed status in migration rules table to indicate whether the rule was installed
Rules selection and installation of selected rules
Disable selection for not fully translated rules
Author column to show whether the translated rule matched one of the existing Elastic prebuilt rules
Install and enable and Install without enabling buttons within the migration rule details flyout

e40pud added 4 commits

December 9, 2024 12:01


          [Rules migration] Add sorting functionality to rules migration table (e…

20326a9

…lastic#11379)


          Disable install button for already installed or not fully translated …

eec8204

…migration rules


          Remove obsolete code


          Remove obsolete code

01cc9a1

e40pud added release_note:skip Team:Threat Hunting Team: SecuritySolution backport:prev-minor labels

e40pud requested a review from semd

December 9, 2024 11:23

e40pud self-assigned this

e40pud requested a review from a team as a code owner

December 9, 2024 11:23

Contributor

elasticmachine commented Dec 9, 2024

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

Contributor

elasticmachine commented Dec 9, 2024

Pinging @elastic/security-solution (Team: SecuritySolution)

e40pud commented

View reviewed changes

packages/kbn-index-adapter/src/field_maps/types.ts

@@ @@ -46,6 +46,7 @@ export type FieldMap<T extends string = string> = Record< @@
                   array?: boolean;
                   doc_values?: boolean;
                   enabled?: boolean;
+                  fields?: Record<string, { type: string }>;

Contributor Author

e40pud Dec 9, 2024

@semd I extended FieldMap to have fields as well. We need it to be able to do sort by title field which has text type. And in order to sort by this field we need to add sub-field of a keyword type.

https://www.elastic.co/guide/en/elasticsearch/reference/current/text.html#before-enabling-fielddata

e40pud commented

View reviewed changes

...lugins/security_solution/public/siem_migrations/rules/components/status_badge/index.test.tsx Outdated

Contributor Author

e40pud Dec 9, 2024

removing it for now since it does not cover anything and I have a separate task to add all tests


          Merge branch 'main' into security/siem-migrations/11379-sorting

12d57a8

semd reviewed

View reviewed changes

...security_solution/server/lib/siem_migrations/rules/data/rule_migrations_data_rules_client.ts Outdated

+                direction === 'desc' ? '_last' : '_first';
+              const sortingOptions = {
+                author(direction: estypes.SortOrder = 'asc'): estypes.SortCombinations {

Contributor

semd Dec 9, 2024

Calling this "author" is something specific to the UI, In the API we should call it matchedPrebuiltRule or customTranslation (opposite).

Contributor Author

e40pud Dec 9, 2024

done

semd reviewed

View reviewed changes

...security_solution/server/lib/siem_migrations/rules/data/rule_migrations_data_rules_client.ts Outdated

@@ @@ -395,3 +407,89 @@ const conditions = { @@
                   return [this.isFullyTranslated(), this.isNotInstalled()];
                 },
               };
+              const missing = (direction: estypes.SortOrder = 'asc') =>

Contributor

semd Dec 9, 2024

This function name is vague, can we call it sortMissingValue or something more descriptive?

Contributor Author

e40pud Dec 9, 2024

done

semd reviewed

View reviewed changes

...security_solution/server/lib/siem_migrations/rules/data/rule_migrations_data_rules_client.ts Outdated

Comment on lines 483 to 495

+              const getSortingOptions = (sort?: RuleMigrationSort): estypes.Sort => {
+                if (!sort?.sortField) {
+                  return DEFAULT_SORTING;
+                }
+                const sortingOptionsMap: { [key: string]: estypes.Sort } = {
+                  'elastic_rule.title': sortingOptions.name(sort.sortDirection),
+                  'elastic_rule.severity': sortingOptions.severity(sort.sortDirection),
+                  'elastic_rule.prebuilt_rule_id': sortingOptions.author(sort.sortDirection),
+                  translation_result: sortingOptions.status(sort.sortDirection),
+                  updated_at: sortingOptions.updated(sort.sortDirection),
+                };
+                return sortingOptionsMap[sort.sortField] ?? DEFAULT_SORTING;
+              };

Contributor

semd Dec 9, 2024

Wouln't it be better to create the map only once?

Suggested change

      
            const getSortingOptions = (sort?: RuleMigrationSort): estypes.Sort => {
          
              if (!sort?.sortField) {
          
                return DEFAULT_SORTING;
          
              }
          
              const sortingOptionsMap: { [key: string]: estypes.Sort } = {
          
                'elastic_rule.title': sortingOptions.name(sort.sortDirection),
          
                'elastic_rule.severity': sortingOptions.severity(sort.sortDirection),
          
                'elastic_rule.prebuilt_rule_id': sortingOptions.author(sort.sortDirection),
          
                translation_result: sortingOptions.status(sort.sortDirection),
          
                updated_at: sortingOptions.updated(sort.sortDirection),
          
              };
          
              return sortingOptionsMap[sort.sortField] ?? DEFAULT_SORTING;
          
            };
          
            const sortingOptionsMap: { [key: string]: estypes.Sort } = {
          
              'elastic_rule.title': sortingOptions.name,
          
              'elastic_rule.severity': sortingOptions.severity,
          
              'elastic_rule.prebuilt_rule_id': sortingOptions.author,
          
              translation_result: sortingOptions.status,
          
              updated_at: sortingOptions.updated,
          
            };
          
            const getSortingOptions = (sort?: RuleMigrationSort): estypes.Sort => {
          
              if (!sort?.sortField) {
          
                return DEFAULT_SORTING;
          
              }
          
              return sortingOptionsMap[sort.sortField]?.(sort.sortDirection) ?? DEFAULT_SORTING;
          
            };

Contributor Author

e40pud Dec 9, 2024

done

semd reviewed

View reviewed changes

...security_solution/server/lib/siem_migrations/rules/data/rule_migrations_data_rules_client.ts Outdated

+                if (!sort?.sortField) {
+                  return DEFAULT_SORTING;
+                }
+                const sortingOptionsMap: { [key: string]: estypes.Sort } = {

Contributor

semd Dec 9, 2024

It would be good to have a union type with the fields we support sorting for, instead of using string

Contributor Author

e40pud Dec 9, 2024

done

Contributor Author

e40pud Dec 9, 2024 •

edited

Loading

Now, instead of returning only one condition estypes.SortCombinations we can return an array of conditions estypes.SortCombinations[]. I actually used that in case of sorting by status 😄

semd reviewed

View reviewed changes

...security_solution/server/lib/siem_migrations/rules/data/rule_migrations_data_rules_client.ts Outdated

@@ @@ -395,3 +407,89 @@ const conditions = { @@
                   return [this.isFullyTranslated(), this.isNotInstalled()];
                 },
               };

Contributor

semd Dec 9, 2024 •

edited

Loading

This file is starting to grow too much.
Would it make sense to move these sorting helpers to a separate place?
Maybe:

server/lib/siem_migrations/rules/data/rule_migrations_data_rules_client/sort.ts

And the conditions object to:

server/lib/siem_migrations/rules/data/rule_migrations_data_rules_client/search.ts

Contributor Author

e40pud Dec 9, 2024

done


          Review feedback

786a025

Contributor

elasticmachine commented Dec 9, 2024

💚 Build Succeeded

Buildkite Build
Commit: 786a025

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	6347	6349	+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`	14.6MB	14.7MB	+3.5KB

History

💛 Build #258175 was flaky 12d57a8

semd approved these changes

View reviewed changes

e40pud merged commit 70a5bb3 into elastic:main

8 checks passed

kibanamachine added the v9.0.0 label

Contributor

kibanamachine commented Dec 9, 2024

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12242755200

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request


          [Rules migration] Add sorting functionality to rules migration table (e…

3a9f048

…lastic#11379) (elastic#203396)

## Summary

[Internal link](elastic/security-team#10820)
to the feature details

These changes add sorting functionality to the migration rules table. It
is possible to sort migration rules by next columns: `Updated`, `Name`,
`Status`, `Risk Score`, `Severity` and `Author`.

### Other changes

Next fixes and adjustments were also implemented as part of this PR:
* `Installed` status in migration rules table to indicate whether the
rule was installed
* Rules selection and installation of selected rules
* Disable selection for not fully translated rules
* `Author` column to show whether the translated rule matched one of the
existing Elastic prebuilt rules
* `Install and enable` and `Install without enabling` buttons within the
migration rule details flyout

(cherry picked from commit 70a5bb3)

kibanamachine mentioned this pull request

[8.x] [Rules migration] Add sorting functionality to rules migration table (#11379) (#203396) #203486

Merged

Contributor

kibanamachine commented Dec 9, 2024

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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


          [8.x] [Rules migration] Add sorting functionality to rules migration …

71bfa26

…table (#11379) (#203396) (#203486)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Rules migration] Add sorting functionality to rules migration table
(#11379) (#203396)](#203396)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Ievgen
Sorokopud","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-09T19:21:16Z","message":"[Rules
migration] Add sorting functionality to rules migration table (#11379)
(#203396)\n\n## Summary\r\n\r\n[Internal
link](https://github.com/elastic/security-team/issues/10820)\r\nto the
feature details\r\n\r\nThese changes add sorting functionality to the
migration rules table. It\r\nis possible to sort migration rules by next
columns: `Updated`, `Name`,\r\n`Status`, `Risk Score`, `Severity` and
`Author`.\r\n\r\n### Other changes\r\n\r\nNext fixes and adjustments
were also implemented as part of this PR:\r\n* `Installed` status in
migration rules table to indicate whether the\r\nrule was installed\r\n*
Rules selection and installation of selected rules\r\n* Disable
selection for not fully translated rules\r\n* `Author` column to show
whether the translated rule matched one of the\r\nexisting Elastic
prebuilt rules\r\n* `Install and enable` and `Install without enabling`
buttons within the\r\nmigration rule details
flyout","sha":"70a5bb33c438912b64259ea4c7a3c77c41f93f45","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Threat
Hunting","Team: SecuritySolution","backport:prev-minor"],"title":"[Rules
migration] Add sorting functionality to rules migration table
(#11379)","number":203396,"url":"https://github.com/elastic/kibana/pull/203396","mergeCommit":{"message":"[Rules
migration] Add sorting functionality to rules migration table (#11379)
(#203396)\n\n## Summary\r\n\r\n[Internal
link](https://github.com/elastic/security-team/issues/10820)\r\nto the
feature details\r\n\r\nThese changes add sorting functionality to the
migration rules table. It\r\nis possible to sort migration rules by next
columns: `Updated`, `Name`,\r\n`Status`, `Risk Score`, `Severity` and
`Author`.\r\n\r\n### Other changes\r\n\r\nNext fixes and adjustments
were also implemented as part of this PR:\r\n* `Installed` status in
migration rules table to indicate whether the\r\nrule was installed\r\n*
Rules selection and installation of selected rules\r\n* Disable
selection for not fully translated rules\r\n* `Author` column to show
whether the translated rule matched one of the\r\nexisting Elastic
prebuilt rules\r\n* `Install and enable` and `Install without enabling`
buttons within the\r\nmigration rule details
flyout","sha":"70a5bb33c438912b64259ea4c7a3c77c41f93f45"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203396","number":203396,"mergeCommit":{"message":"[Rules
migration] Add sorting functionality to rules migration table (#11379)
(#203396)\n\n## Summary\r\n\r\n[Internal
link](https://github.com/elastic/security-team/issues/10820)\r\nto the
feature details\r\n\r\nThese changes add sorting functionality to the
migration rules table. It\r\nis possible to sort migration rules by next
columns: `Updated`, `Name`,\r\n`Status`, `Risk Score`, `Severity` and
`Author`.\r\n\r\n### Other changes\r\n\r\nNext fixes and adjustments
were also implemented as part of this PR:\r\n* `Installed` status in
migration rules table to indicate whether the\r\nrule was installed\r\n*
Rules selection and installation of selected rules\r\n* Disable
selection for not fully translated rules\r\n* `Author` column to show
whether the translated rule matched one of the\r\nexisting Elastic
prebuilt rules\r\n* `Install and enable` and `Install without enabling`
buttons within the\r\nmigration rule details
flyout","sha":"70a5bb33c438912b64259ea4c7a3c77c41f93f45"}}]}]
BACKPORT-->

---------

Co-authored-by: Ievgen Sorokopud <[email protected]>

kibanamachine added the v8.18.0 label

Samiul-TheSoccerFan pushed a commit to Samiul-TheSoccerFan/kibana that referenced this pull request


          [Rules migration] Add sorting functionality to rules migration table (e…

2e395cb

…lastic#11379) (elastic#203396)

## Summary

[Internal link](elastic/security-team#10820)
to the feature details

These changes add sorting functionality to the migration rules table. It
is possible to sort migration rules by next columns: `Updated`, `Name`,
`Status`, `Risk Score`, `Severity` and `Author`.

### Other changes

Next fixes and adjustments were also implemented as part of this PR:
* `Installed` status in migration rules table to indicate whether the
rule was installed
* Rules selection and installation of selected rules
* Disable selection for not fully translated rules
* `Author` column to show whether the translated rule matched one of the
existing Elastic prebuilt rules
* `Install and enable` and `Install without enabling` buttons within the
migration rule details flyout

CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request


          [Rules migration] Add sorting functionality to rules migration table (e…

51e8ad0

…lastic#11379) (elastic#203396)

## Summary

[Internal link](elastic/security-team#10820)
to the feature details

These changes add sorting functionality to the migration rules table. It
is possible to sort migration rules by next columns: `Updated`, `Name`,
`Status`, `Risk Score`, `Severity` and `Author`.

### Other changes

Next fixes and adjustments were also implemented as part of this PR:
* `Installed` status in migration rules table to indicate whether the
rule was installed
* Rules selection and installation of selected rules
* Disable selection for not fully translated rules
* `Author` column to show whether the translated rule matched one of the
existing Elastic prebuilt rules
* `Install and enable` and `Install without enabling` buttons within the
migration rule details flyout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:prev-minor release_note:skip Team: SecuritySolution Team:Threat Hunting v8.18.0 v9.0.0