From 626aa88d6242ed68ecdcc27a90bbf23839c2bbaa Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Mon, 25 Mar 2024 11:06:22 -0600 Subject: [PATCH] Fix alert group table default filter being ignored, add 1 week window as default time range (#4080) # What this PR does - Default filters were being ignored on the alerts group page (Only show acknowledged and firing). This was because the object contained an empty array for the team field so it was never considered empty, now all fields are checked. - Added a 1 week window for `started_at` as a default. This should give more useful behavior in larger instances so we won't have long queries counting all alert groups since the beginning. ## Which issue(s) this PR closes Closes https://github.com/grafana/oncall-private/issues/2520 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- grafana-plugin/e2e-tests/utils/alertGroup.ts | 2 +- .../containers/RemoteFilters/RemoteFilters.helpers.ts | 2 +- .../src/containers/RemoteFilters/RemoteFilters.tsx | 5 +++-- grafana-plugin/src/pages/incidents/Incidents.tsx | 3 +++ grafana-plugin/src/utils/utils.ts | 11 ++++++++++- 5 files changed, 18 insertions(+), 5 deletions(-) diff --git a/grafana-plugin/e2e-tests/utils/alertGroup.ts b/grafana-plugin/e2e-tests/utils/alertGroup.ts index 2711c896..77696e55 100644 --- a/grafana-plugin/e2e-tests/utils/alertGroup.ts +++ b/grafana-plugin/e2e-tests/utils/alertGroup.ts @@ -101,6 +101,6 @@ export const verifyThatAlertGroupIsTriggered = async ( export const resolveFiringAlert = async (page: Page) => { await goToOnCallPage(page, 'alert-groups'); - await page.getByText('Firing').nth(1).click(); + await page.getByText('Firing').nth(2).click({force: true}); await page.getByLabel('Context menu').getByText('Resolve').click(); }; diff --git a/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.helpers.ts b/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.helpers.ts index 619ac98d..8969bd20 100644 --- a/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.helpers.ts +++ b/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.helpers.ts @@ -15,7 +15,7 @@ export function parseFilters( filterOptions: FilterOption[], query: { [key: string]: any } ) { - const dataWithPredefinedTeams = { ...data, team: data.team || [] }; + const dataWithPredefinedTeams = { ...data, team: data?.team || [] }; const filters = filterOptions.filter((filterOption: FilterOption) => filterOption.name in dataWithPredefinedTeams); const values = filters.reduce((memo: any, filterOption: FilterOption) => { diff --git a/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.tsx b/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.tsx index 86a28f54..dfccafda 100644 --- a/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.tsx +++ b/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.tsx @@ -14,7 +14,7 @@ import { } from '@grafana/ui'; import { capitalCase } from 'change-case'; import cn from 'classnames/bind'; -import { debounce, isEmpty, isUndefined, omitBy, pickBy } from 'lodash-es'; +import { debounce, isUndefined, omitBy, pickBy } from 'lodash-es'; import { observer } from 'mobx-react'; import moment from 'moment-timezone'; import Emoji from 'react-emoji-render'; @@ -29,6 +29,7 @@ import { SelectOption, WithStoreProps } from 'state/types'; import { withMobXProviderContext } from 'state/withStore'; import { LocationHelper } from 'utils/LocationHelper'; import { PAGE } from 'utils/consts'; +import { allFieldsEmpty } from 'utils/utils'; import { parseFilters } from './RemoteFilters.helpers'; import { FilterOption } from './RemoteFilters.types'; @@ -102,7 +103,7 @@ class _RemoteFilters extends Component { let { filters, values } = parseFilters({ ...query, ...filtersStore.globalValues }, filterOptions, query); - if (isEmpty(values)) { + if (allFieldsEmpty(values)) { ({ filters, values } = parseFilters(defaultFilters, filterOptions, query)); } diff --git a/grafana-plugin/src/pages/incidents/Incidents.tsx b/grafana-plugin/src/pages/incidents/Incidents.tsx index db9138e1..43daa986 100644 --- a/grafana-plugin/src/pages/incidents/Incidents.tsx +++ b/grafana-plugin/src/pages/incidents/Incidents.tsx @@ -304,6 +304,8 @@ class _IncidentsPage extends React.Component diff --git a/grafana-plugin/src/utils/utils.ts b/grafana-plugin/src/utils/utils.ts index b44bed52..2cad629d 100644 --- a/grafana-plugin/src/utils/utils.ts +++ b/grafana-plugin/src/utils/utils.ts @@ -3,7 +3,7 @@ import { AxiosError } from 'axios'; import { sentenceCase } from 'change-case'; // @ts-ignore import appEvents from 'grafana/app/core/app_events'; -import { isArray, concat, isPlainObject, flatMap, map, keys } from 'lodash-es'; +import { isArray, concat, every, isEmpty, isObject, isPlainObject, flatMap, map, keys } from 'lodash-es'; import { isNetworkError } from 'network/network'; import { getGrafanaVersion } from 'plugin/GrafanaPluginRootPage.helpers'; @@ -107,3 +107,12 @@ export function isUseProfileExtensionPointEnabled(): boolean { return isRequiredGrafanaVersion; } + +function isFieldEmpty(value: any): boolean { + if (isObject(value)) { + return isEmpty(value); + } + return value === '' || value === null || value === undefined; +} + +export const allFieldsEmpty = (obj: any) => every(obj, isFieldEmpty);