-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[ResponceOps][MaintenanceWindow] MX Pagination #202539
[ResponceOps][MaintenanceWindow] MX Pagination #202539
Conversation
…e/kibana into MX-193076-MX-pagination-backend
…e/kibana into MX-193076-MX-pagination-backend
…ue/kibana into MX-193076-MX-pagination-frontend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SO changes LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing the comments. I think there are some more left.
// loadTestFile(require.resolve('./home_page')); | ||
// loadTestFile(require.resolve('./rules_list')); | ||
// loadTestFile(require.resolve('./alert_create_flyout')); | ||
// loadTestFile(require.resolve('./details')); | ||
// loadTestFile(require.resolve('./connectors')); | ||
// loadTestFile(require.resolve('./logs_list')); | ||
// loadTestFile(require.resolve('./rules_settings')); | ||
// loadTestFile(require.resolve('./stack_alerts_page')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's umcomment all of them.
// loadTestFile(require.resolve('./maintenance_window_create_form')); | ||
// loadTestFile(require.resolve('./maintenance_window_update_form')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's umcomment all of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already done. I forgot to do it first
@@ -53,7 +66,7 @@ export const useFindMaintenanceWindows = (props?: UseFindMaintenanceWindowsProps | |||
}); | |||
|
|||
return { | |||
maintenanceWindows: data, | |||
data: data || { maintenanceWindows: [], total: 0 }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is happening because on an error data
will be undefined
. What if you keep the placeholderData
and also do
const { data, ..// other stuff } = useFindMaintenanceWindows({ ...// args })
const { maintenanceWindows, total } = data ?? { maintenanceWindows: [], total: 0 };
const fullQuery = statuses | ||
?.slice(1) | ||
.reduce( | ||
(acc, currentStatusFilter) => acc + ` or ${statusToQueryMapping[currentStatusFilter] || ''}`, | ||
statusToQueryMapping[statuses[0]] || '' | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this?
[MaintenanceWindowStatus.Upcoming]: | ||
'(not (maintenance-window.attributes.events: "now") and maintenance-window.attributes.events >= "now")', | ||
[MaintenanceWindowStatus.Finished]: | ||
'(not (maintenance-window.attributes.events >= "now" or maintenance-window.attributes.expirationDate < "now"))', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible for an archived MW to have events where maintenance-window.attributes.events < "now")
?
}); | ||
await browser.refresh(); | ||
|
||
await pageObjects.maintenanceWindows.searchMaintenanceWindows('pagination'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, then I would suggest moving the state for the input in x-pack/plugins/alerting/public/pages/maintenance_windows/components/maintenance_windows_list.tsx
and keeping only the state for the search in this component. Example
// x-pack/plugins/alerting/public/pages/maintenance_windows/index.tsx
const [search, setSearch] = useState<string>('');
<MaintenanceWindowsList
onSearchChange={setSearch}
/>
// x-pack/plugins/alerting/public/pages/maintenance_windows/components/maintenance_windows_list.tsx
const [search, setSearch] = useState<string>('');
<EuiFieldSearch
incremental={false}
placeholder={i18n.SEARCH_PLACEHOLDER}
value={inputText}
onSearch={onSearchChange} // from props
onChange={(e) => setSearch(e.target.value)}
/>
queryKey: ['findMaintenanceWindows'], | ||
const queryKey = ['findMaintenanceWindows', page, perPage, search, selectedStatus]; | ||
|
||
const { isLoading, isFetching, isInitialLoading, data, refetch } = useQuery({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also add keepPreviousData: true
to avoid having the table clear every time we change status.
perPage: number; | ||
total: number; | ||
onPageChange: ({ page: { index, size } }: { page: { index: number; size: number } }) => void; | ||
inputText: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kindly reminder.
[MaintenanceWindowStatus.Upcoming]: | ||
'(not (maintenance-window.attributes.events: "now") and maintenance-window.attributes.events >= "now")', | ||
[MaintenanceWindowStatus.Finished]: | ||
'(not (maintenance-window.attributes.events >= "now" or maintenance-window.attributes.expirationDate < "now"))', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes sorry what I meant was not maintenance-window.attributes.events: > "now" and maintenance-window.attributes.expirationDate > "now"
…ue/kibana into MX-193076-MX-pagination-frontend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work 🚀
…ue/kibana into MX-193076-MX-pagination-frontend
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
History
cc @guskovaue |
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/12448048216 |
Fixes: elastic#198252 In this PR I introduced pagination in MW frontend part and also pass filters(status and search) to the backend. Pagination arguments were passed to backend in another PR: https://github.com/elastic/kibana/pull/197172/files#diff-f375a192a08a6db3fbb6b6e927cecaab89ff401efc4034f00761e8fc4478734c How to test: Go to Maintenance Window, create more than 10 MW with different statuses. Try pagination, search on text and filter by status. Check the PR satisfies following conditions: - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit 43abe23)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
# Backport This will backport the following commits from `main` to `8.x`: - [[ResponceOps][MaintenanceWindow] MX Pagination (#202539)](#202539) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Julia","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-21T20:01:11Z","message":"[ResponceOps][MaintenanceWindow] MX Pagination (#202539)\n\nFixes: https://github.com/elastic/kibana/issues/198252\r\n\r\nIn this PR I introduced pagination in MW frontend part and also pass\r\nfilters(status and search) to the backend. Pagination arguments were\r\npassed to backend in another PR:\r\nhttps://github.com//pull/197172/files#diff-f375a192a08a6db3fbb6b6e927cecaab89ff401efc4034f00761e8fc4478734c\r\n\r\nHow to test:\r\nGo to Maintenance Window, create more than 10 MW with different\r\nstatuses. Try pagination, search on text and filter by status.\r\n\r\nCheck the PR satisfies following conditions:\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_note:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"43abe233d814cb9a5519a63a2f5942f8802879b2","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:ResponseOps","v9.0.0","Feature:Alerting/RulesManagement","backport:prev-minor","v8.18.0"],"title":"[ResponceOps][MaintenanceWindow] MX Pagination","number":202539,"url":"https://github.com/elastic/kibana/pull/202539","mergeCommit":{"message":"[ResponceOps][MaintenanceWindow] MX Pagination (#202539)\n\nFixes: https://github.com/elastic/kibana/issues/198252\r\n\r\nIn this PR I introduced pagination in MW frontend part and also pass\r\nfilters(status and search) to the backend. Pagination arguments were\r\npassed to backend in another PR:\r\nhttps://github.com//pull/197172/files#diff-f375a192a08a6db3fbb6b6e927cecaab89ff401efc4034f00761e8fc4478734c\r\n\r\nHow to test:\r\nGo to Maintenance Window, create more than 10 MW with different\r\nstatuses. Try pagination, search on text and filter by status.\r\n\r\nCheck the PR satisfies following conditions:\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_note:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"43abe233d814cb9a5519a63a2f5942f8802879b2"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202539","number":202539,"mergeCommit":{"message":"[ResponceOps][MaintenanceWindow] MX Pagination (#202539)\n\nFixes: https://github.com/elastic/kibana/issues/198252\r\n\r\nIn this PR I introduced pagination in MW frontend part and also pass\r\nfilters(status and search) to the backend. Pagination arguments were\r\npassed to backend in another PR:\r\nhttps://github.com//pull/197172/files#diff-f375a192a08a6db3fbb6b6e927cecaab89ff401efc4034f00761e8fc4478734c\r\n\r\nHow to test:\r\nGo to Maintenance Window, create more than 10 MW with different\r\nstatuses. Try pagination, search on text and filter by status.\r\n\r\nCheck the PR satisfies following conditions:\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_note:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"43abe233d814cb9a5519a63a2f5942f8802879b2"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Julia <[email protected]>
Fixes: elastic#198252 In this PR I introduced pagination in MW frontend part and also pass filters(status and search) to the backend. Pagination arguments were passed to backend in another PR: https://github.com/elastic/kibana/pull/197172/files#diff-f375a192a08a6db3fbb6b6e927cecaab89ff401efc4034f00761e8fc4478734c How to test: Go to Maintenance Window, create more than 10 MW with different statuses. Try pagination, search on text and filter by status. Check the PR satisfies following conditions: - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]>
Fixes: elastic#198252 In this PR I introduced pagination in MW frontend part and also pass filters(status and search) to the backend. Pagination arguments were passed to backend in another PR: https://github.com/elastic/kibana/pull/197172/files#diff-f375a192a08a6db3fbb6b6e927cecaab89ff401efc4034f00761e8fc4478734c How to test: Go to Maintenance Window, create more than 10 MW with different statuses. Try pagination, search on text and filter by status. Check the PR satisfies following conditions: - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]>
Fixes: elastic#198252 In this PR I introduced pagination in MW frontend part and also pass filters(status and search) to the backend. Pagination arguments were passed to backend in another PR: https://github.com/elastic/kibana/pull/197172/files#diff-f375a192a08a6db3fbb6b6e927cecaab89ff401efc4034f00761e8fc4478734c How to test: Go to Maintenance Window, create more than 10 MW with different statuses. Try pagination, search on text and filter by status. Check the PR satisfies following conditions: - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]>
Fixes: #198252
In this PR I introduced pagination in MW frontend part and also pass filters(status and search) to the backend. Pagination arguments were passed to backend in another PR: https://github.com/elastic/kibana/pull/197172/files#diff-f375a192a08a6db3fbb6b6e927cecaab89ff401efc4034f00761e8fc4478734c
How to test:
Go to Maintenance Window, create more than 10 MW with different statuses. Try pagination, search on text and filter by status.
Check the PR satisfies following conditions:
release_note:*
label is applied per the guidelines