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

[Rules migration] Add pagination functionality to rules migration table (#11313) #202494

Merged

Conversation

e40pud
Copy link
Contributor

@e40pud e40pud commented Dec 2, 2024

Summary

Internal link to the feature details

With these changes we add pagination functionality for the rules migration table. This way we will improve the performance within the page.

Also, added as part of these PR:

  • moved install and install_translated routes to the rules/api folder; before those were located in rules/api/rules and made confusion
  • a new translation_stats route to return stats for the specific migration about the translated rules, like total number of the rules, and number of prebuilt, custom and installable rules
  • add Updated table column
  • small UI fixes:
    • use correct icon for "SIEM rule migration"
    • do not remove "Install translated rules" button and rather disable it when there are no installable rules
    • do not allow user to update translation status via UI

@e40pud e40pud added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Dec 2, 2024
@e40pud e40pud requested a review from semd December 2, 2024 14:53
@e40pud e40pud self-assigned this Dec 2, 2024
@e40pud e40pud requested a review from a team as a code owner December 2, 2024 14:53
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

import { EuiFieldSearch, EuiFlexItem } from '@elastic/eui';
import * as i18n from './translations';

const SearchBarWrapper = styled(EuiFlexItem)`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this style copied from similar rules management component x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/rules_table_filters/rule_search_field.tsx and we can adjust it if needed

@@ -19,17 +19,17 @@ export const ruleMigrationsFieldMap: FieldMap<SchemaFieldMapKeys<Omit<RuleMigrat
original_rule: { type: 'nested', required: true },
'original_rule.vendor': { type: 'keyword', required: true },
'original_rule.id': { type: 'keyword', required: true },
'original_rule.title': { type: 'keyword', required: true },
'original_rule.description': { type: 'keyword', required: false },
'original_rule.title': { type: 'text', required: true },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated schema to be able to search within title and description fields. For now we use only elastic_rule.title, but I was thinking that potentially we could extend search to other updated fields.

@semd let me know what you think about it

@elastic-vault-github-plugin-prod elastic-vault-github-plugin-prod bot requested a review from a team as a code owner December 2, 2024 15:28
* use correct icon for "SIEM rule migration"
* do not remove "Install translated rules" button and rather disable it when there are no installable rules
* do not allow user to update translation status via UI
Comment on lines +13 to +19
const { addError } = useAppToasts();

return useGetMigrationTranslationStatsQuery(migrationId, {
onError: (error) => {
addError(error, { title: i18n.GET_MIGRATION_TRANSLATION_STATS_FAILURE });
},
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that for every API request we have to implement 2 separate hooks. Why don't we add error handling to the same hook that does the useQuery? so we have everything in one hook. Or is there a reason to have them separate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No real reason, except of following rules management patterns :-) I think we do not need to overcomplicate things and can keep everything in one hook. I will fix that in a separate PR.

Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #10 / it should allow to add a new host
  • [job] [logs] Jest Tests #10 / useActionStatus should refresh statuses on refresh flag

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 6307 6310 +3

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.6MB +3.3KB

History

cc @e40pud

@e40pud e40pud merged commit a662233 into elastic:main Dec 3, 2024
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 202494

Questions ?

Please refer to the Backport tool documentation

@e40pud
Copy link
Contributor Author

e40pud commented Dec 3, 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

e40pud added a commit to e40pud/kibana that referenced this pull request Dec 3, 2024
…le (elastic#11313) (elastic#202494)

## Summary

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

With these changes we add pagination functionality for the rules
migration table. This way we will improve the performance within the
page.

Also, added as part of these PR:
* moved `install` and `install_translated` routes to the `rules/api`
folder; before those were located in `rules/api/rules` and made
confusion
* a new `translation_stats` route to return stats for the specific
migration about the translated rules, like `total` number of the rules,
and number of `prebuilt`, `custom` and `installable` rules
* add `Updated` table column
* small UI fixes:
  * use correct icon for "SIEM rule migration"
* do not remove "Install translated rules" button and rather disable it
when there are no installable rules
  * do not allow user to update translation status via UI

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit a662233)

# Conflicts:
#	x-pack/plugins/security_solution/common/api/quickstart_client.gen.ts
#	x-pack/plugins/security_solution/public/siem_migrations/rules/api/api.ts
#	x-pack/test/api_integration/services/security_solution_api.gen.ts
e40pud added a commit that referenced this pull request Dec 3, 2024
…on table (#11313) (#202494) (#202787)

# Backport

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

<!--- Backport version: 8.9.8 -->

### 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-03T17:50:10Z","message":"[Rules
migration] Add pagination functionality to rules migration table
(#11313) (#202494)\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\nWith these changes we add pagination
functionality for the rules\r\nmigration table. This way we will improve
the performance within the\r\npage.\r\n\r\nAlso, added as part of these
PR:\r\n* moved `install` and `install_translated` routes to the
`rules/api`\r\nfolder; before those were located in `rules/api/rules`
and made\r\nconfusion\r\n* a new `translation_stats` route to return
stats for the specific\r\nmigration about the translated rules, like
`total` number of the rules,\r\nand number of `prebuilt`, `custom` and
`installable` rules\r\n* add `Updated` table column\r\n* small UI
fixes:\r\n * use correct icon for \"SIEM rule migration\"\r\n* do not
remove \"Install translated rules\" button and rather disable it\r\nwhen
there are no installable rules\r\n * do not allow user to update
translation status via UI\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"a662233d8b967d022a32fc9379d338684bf6a413","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"],"number":202494,"url":"https://github.com/elastic/kibana/pull/202494","mergeCommit":{"message":"[Rules
migration] Add pagination functionality to rules migration table
(#11313) (#202494)\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\nWith these changes we add pagination
functionality for the rules\r\nmigration table. This way we will improve
the performance within the\r\npage.\r\n\r\nAlso, added as part of these
PR:\r\n* moved `install` and `install_translated` routes to the
`rules/api`\r\nfolder; before those were located in `rules/api/rules`
and made\r\nconfusion\r\n* a new `translation_stats` route to return
stats for the specific\r\nmigration about the translated rules, like
`total` number of the rules,\r\nand number of `prebuilt`, `custom` and
`installable` rules\r\n* add `Updated` table column\r\n* small UI
fixes:\r\n * use correct icon for \"SIEM rule migration\"\r\n* do not
remove \"Install translated rules\" button and rather disable it\r\nwhen
there are no installable rules\r\n * do not allow user to update
translation status via UI\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"a662233d8b967d022a32fc9379d338684bf6a413"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202494","number":202494,"mergeCommit":{"message":"[Rules
migration] Add pagination functionality to rules migration table
(#11313) (#202494)\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\nWith these changes we add pagination
functionality for the rules\r\nmigration table. This way we will improve
the performance within the\r\npage.\r\n\r\nAlso, added as part of these
PR:\r\n* moved `install` and `install_translated` routes to the
`rules/api`\r\nfolder; before those were located in `rules/api/rules`
and made\r\nconfusion\r\n* a new `translation_stats` route to return
stats for the specific\r\nmigration about the translated rules, like
`total` number of the rules,\r\nand number of `prebuilt`, `custom` and
`installable` rules\r\n* add `Updated` table column\r\n* small UI
fixes:\r\n * use correct icon for \"SIEM rule migration\"\r\n* do not
remove \"Install translated rules\" button and rather disable it\r\nwhen
there are no installable rules\r\n * do not allow user to update
translation status via UI\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"a662233d8b967d022a32fc9379d338684bf6a413"}}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <[email protected]>
e40pud added a commit that referenced this pull request Dec 4, 2024
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.


* Export the memo component directly everywhere
([comment](#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.


* Use one hook to access APIs instead of two
([comment](#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 4, 2024
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](elastic#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.

* Export the memo component directly everywhere
([comment](elastic#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.

* Use one hook to access APIs instead of two
([comment](elastic#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?

(cherry picked from commit 4d8f711)
hop-dev pushed a commit to hop-dev/kibana that referenced this pull request Dec 5, 2024
…le (elastic#11313) (elastic#202494)

## Summary

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

With these changes we add pagination functionality for the rules
migration table. This way we will improve the performance within the
page.

Also, added as part of these PR:
* moved `install` and `install_translated` routes to the `rules/api`
folder; before those were located in `rules/api/rules` and made
confusion
* a new `translation_stats` route to return stats for the specific
migration about the translated rules, like `total` number of the rules,
and number of `prebuilt`, `custom` and `installable` rules
* add `Updated` table column
* small UI fixes:
  * use correct icon for "SIEM rule migration"
* do not remove "Install translated rules" button and rather disable it
when there are no installable rules
  * do not allow user to update translation status via UI

---------

Co-authored-by: kibanamachine <[email protected]>
SoniaSanzV pushed a commit to SoniaSanzV/kibana that referenced this pull request Dec 9, 2024
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](elastic#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.


* Export the memo component directly everywhere
([comment](elastic#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.


* Use one hook to access APIs instead of two
([comment](elastic#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?
SoniaSanzV pushed a commit to SoniaSanzV/kibana that referenced this pull request Dec 9, 2024
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](elastic#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.


* Export the memo component directly everywhere
([comment](elastic#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.


* Use one hook to access APIs instead of two
([comment](elastic#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 9, 2024
…le (elastic#11313) (elastic#202494)

## Summary

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

With these changes we add pagination functionality for the rules
migration table. This way we will improve the performance within the
page.

Also, added as part of these PR:
* moved `install` and `install_translated` routes to the `rules/api`
folder; before those were located in `rules/api/rules` and made
confusion
* a new `translation_stats` route to return stats for the specific
migration about the translated rules, like `total` number of the rules,
and number of `prebuilt`, `custom` and `installable` rules
* add `Updated` table column
* small UI fixes:
  * use correct icon for "SIEM rule migration"
* do not remove "Install translated rules" button and rather disable it
when there are no installable rules
  * do not allow user to update translation status via UI

---------

Co-authored-by: kibanamachine <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 9, 2024
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](elastic#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.


* Export the memo component directly everywhere
([comment](elastic#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.


* Use one hook to access APIs instead of two
([comment](elastic#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?
Samiul-TheSoccerFan pushed a commit to Samiul-TheSoccerFan/kibana that referenced this pull request Dec 10, 2024
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](elastic#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.


* Export the memo component directly everywhere
([comment](elastic#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.


* Use one hook to access APIs instead of two
([comment](elastic#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
…le (elastic#11313) (elastic#202494)

## Summary

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

With these changes we add pagination functionality for the rules
migration table. This way we will improve the performance within the
page.

Also, added as part of these PR:
* moved `install` and `install_translated` routes to the `rules/api`
folder; before those were located in `rules/api/rules` and made
confusion
* a new `translation_stats` route to return stats for the specific
migration about the translated rules, like `total` number of the rules,
and number of `prebuilt`, `custom` and `installable` rules
* add `Updated` table column
* small UI fixes:
  * use correct icon for "SIEM rule migration"
* do not remove "Install translated rules" button and rather disable it
when there are no installable rules
  * do not allow user to update translation status via UI

---------

Co-authored-by: kibanamachine <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](elastic#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.


* Export the memo component directly everywhere
([comment](elastic#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.


* Use one hook to access APIs instead of two
([comment](elastic#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting Security Solution Threat Hunting Team v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants