fix issues related to alert groups table (#3905)

# What this PR does
- get rid of duplicated GET requests during polling
- ignore results of older requests
- fix Refresh btn that doesn't disappear
- show correct data in alert groups table
- show placeholder instead of table when filters are updated

## Which issue(s) this PR fixes
https://github.com/grafana/oncall/issues/3894

## 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] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
This commit is contained in:
Dominik Broj 2024-02-19 10:23:04 +01:00 committed by GitHub
parent 4ace9780c5
commit 23bd517213
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 129 additions and 74 deletions

View file

@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix edit default team by admin @mderynck ([#3885](https://github.com/grafana/oncall/pull/3885))
- Unblock slack install by skipping check chatops gateway link in OSS deployment @mderynck ([#3893](https://github.com/grafana/oncall/pull/3893))
- Fix multiple issues of alert groups table ([#3894](https://github.com/grafana/oncall/issues/3894))
- Improvements for dragging the add rotation form in Schedules ([#3904](https://github.com/grafana/oncall/pull/3904))
### Changed

View file

@ -11,7 +11,6 @@ import styles from './GTable.module.css';
const cx = cn.bind(styles);
export interface Props<RecordType = unknown> extends TableProps<RecordType> {
loading?: boolean;
pagination?: {
page: number;
total: number;
@ -38,7 +37,6 @@ export const GTable = <RT extends DefaultRecordType = DefaultRecordType>(props:
data,
className,
pagination,
loading,
rowSelection,
rowKey,
expandable,

View file

@ -29,6 +29,7 @@ import { AlertGroupColumn, AlertGroupColumnType } from 'models/alertgroup/alertg
import { ActionKey } from 'models/loader/action-keys';
import { useStore } from 'state/useStore';
import { UserActions } from 'utils/authorization/authorization';
import { useIsLoading } from 'utils/hooks';
import { openErrorNotification } from 'utils/utils';
import { getColumnsSelectorStyles } from './ColumnsSelector.styles';
@ -115,7 +116,8 @@ interface ColumnsSelectorProps {
export const ColumnsSelector: React.FC<ColumnsSelectorProps> = observer(
({ onColumnAddModalOpen, onConfirmRemovalModalOpen }) => {
const { alertGroupStore, loaderStore } = useStore();
const { alertGroupStore } = useStore();
const isResetLoading = useIsLoading(ActionKey.RESET_COLUMNS_FROM_ALERT_GROUP);
const styles = useStyles2(getColumnsSelectorStyles);
@ -133,8 +135,6 @@ export const ColumnsSelector: React.FC<ColumnsSelectorProps> = observer(
})
);
const isResetLoading = loaderStore.isLoading(ActionKey.RESET_COLUMNS_FROM_ALERT_GROUP);
return (
<div className={styles.columnsSelectorView}>
<Text type="primary" className={styles.columnsHeader}>

View file

@ -26,7 +26,7 @@ import { components } from 'network/oncall-api/autogenerated-api.types';
import { useStore } from 'state/useStore';
import { UserActions } from 'utils/authorization/authorization';
import { WrapWithGlobalNotification } from 'utils/decorators';
import { useDebouncedCallback } from 'utils/hooks';
import { useDebouncedCallback, useIsLoading } from 'utils/hooks';
import { pluralize } from 'utils/utils';
import { getColumnsSelectorWrapperStyles } from './ColumnsSelectorWrapper.styles';
@ -56,7 +56,7 @@ export const ColumnsModal: React.FC<ColumnsModalProps> = observer(
const [searchResults, setSearchResults] = useState<SearchResult[]>([]);
const debouncedOnInputChange = useDebouncedCallback(onInputChange, DEBOUNCE_MS);
const isLoading = store.loaderStore.isLoading(ActionKey.ADD_NEW_COLUMN_TO_ALERT_GROUP);
const isLoading = useIsLoading(ActionKey.ADD_NEW_COLUMN_TO_ALERT_GROUP);
const availableKeysForSearching = useMemo(() => {
const cols = store.alertGroupStore.columns;
return labelKeys.filter(

View file

@ -13,6 +13,7 @@ import { ApiSchemas } from 'network/oncall-api/api.types';
import { useStore } from 'state/useStore';
import { UserActions } from 'utils/authorization/authorization';
import { WrapAutoLoadingState, WrapWithGlobalNotification } from 'utils/decorators';
import { useIsLoading } from 'utils/hooks';
import { ColumnsModal } from './ColumnsModal';
@ -23,8 +24,8 @@ export const ColumnsSelectorWrapper: React.FC<ColumnsSelectorWrapperProps> = obs
const [columnToBeRemoved, setColumnToBeRemoved] = useState<AlertGroupColumn>(undefined);
const [isColumnAddModalOpen, setIsColumnAddModalOpen] = useState(false);
const [isFloatingDisplayOpen, setIsFloatingDisplayOpen] = useState(false);
const [labelKeys, setLabelKeys] = useState<Array<ApiSchemas['LabelKey']>>([]);
const isRemoveLoading = useIsLoading(ActionKey.REMOVE_COLUMN_FROM_ALERT_GROUP);
const inputRef = useRef<HTMLInputElement>(null);
const wrappingFloatingContainerRef = useRef<HTMLDivElement>(null);
@ -49,8 +50,6 @@ export const ColumnsSelectorWrapper: React.FC<ColumnsSelectorWrapperProps> = obs
};
}, []);
const isRemoveLoading = store.loaderStore.isLoading(ActionKey.REMOVE_COLUMN_FROM_ALERT_GROUP);
return (
<>
<ColumnsModal

View file

@ -17,6 +17,7 @@ import { User } from 'models/user/user.types';
import { getStartOfWeekBasedOnCurrentDate } from 'pages/schedule/Schedule.helpers';
import { useStore } from 'state/useStore';
import { PLUGIN_ROOT } from 'utils/consts';
import { useIsLoading } from 'utils/hooks';
import { DEFAULT_TRANSITION_TIMEOUT } from './Rotations.config';
@ -31,7 +32,8 @@ interface SchedulePersonalProps extends RouteComponentProps {
const _SchedulePersonal: FC<SchedulePersonalProps> = observer(({ userPk, onSlotClick, history }) => {
const store = useStore();
const { timezoneStore, scheduleStore, userStore, loaderStore } = store;
const { timezoneStore, scheduleStore, userStore } = store;
const updatePersonalEventsLoading = useIsLoading(ActionKey.UPDATE_PERSONAL_EVENTS);
useEffect(() => {
updatePersonalEvents();
@ -74,9 +76,7 @@ const _SchedulePersonal: FC<SchedulePersonalProps> = observer(({ userPk, onSlotC
const storeUser = userStore.items[userPk];
const emptyRotationsText = loaderStore.isLoading(ActionKey.UPDATE_PERSONAL_EVENTS)
? 'Loading ...'
: 'There are no schedules relevant to user';
const emptyRotationsText = updatePersonalEventsLoading ? 'Loading ...' : 'There are no schedules relevant to user';
return (
<div className={cx('root')}>

View file

@ -28,9 +28,6 @@ export class AlertGroupStore extends BaseStore {
@observable.shallow
searchResult: { [key: string]: Array<Alert['pk']> } = {};
@observable
alertGroupsLoading = false;
@observable
incidentFilters: any;
@ -70,6 +67,9 @@ export class AlertGroupStore extends BaseStore {
@observable
liveUpdatesPaused = false;
@observable
latestFetchAlertGroupsTimestamp: number;
@observable
columns: AlertGroupColumn[] = [];
@ -231,17 +231,14 @@ export class AlertGroupStore extends BaseStore {
});
}
// methods were moved from rootBaseStore.
// TODO check if methods are dublicating existing ones
async updateIncidents() {
async fetchIncidentsAndStats(isPollingJob = false) {
await Promise.all([
this.getNewIncidentsStats(),
this.getAcknowledgedIncidentsStats(),
this.getResolvedIncidentsStats(),
this.getSilencedIncidentsStats(),
this.updateAlertGroups(),
this.fetchAlertGroups(isPollingJob),
]);
this.setLiveUpdatesPaused(false);
}
@ -251,21 +248,20 @@ export class AlertGroupStore extends BaseStore {
}
@action
async updateIncidentFilters(params: any, keepCursor = false) {
@AutoLoadingState(ActionKey.UPDATE_FILTERS_AND_FETCH_INCIDENTS)
async updateIncidentFiltersAndRefetchIncidentsAndStats(params: any, keepCursor = false) {
if (!keepCursor) {
this.setIncidentsCursor(undefined);
}
this.incidentFilters = params;
await this.updateIncidents();
await this.fetchIncidentsAndStats();
}
@action
async updateIncidentsCursor(cursor: string) {
this.setIncidentsCursor(cursor);
this.updateAlertGroups();
this.fetchAlertGroups();
}
@action
@ -279,13 +275,17 @@ export class AlertGroupStore extends BaseStore {
async setIncidentsItemsPerPage() {
this.setIncidentsCursor(undefined);
this.updateAlertGroups();
this.fetchAlertGroups();
}
@action.bound
async updateAlertGroups() {
this.alertGroupsLoading = true;
async fetchAlertGroups(isPollingJob = false) {
this.rootStore.loaderStore.setLoadingAction(
isPollingJob ? ActionKey.FETCH_INCIDENTS_POLLING : ActionKey.FETCH_INCIDENTS,
true
);
const timestamp = new Date().getTime();
this.latestFetchAlertGroupsTimestamp = timestamp;
const {
results,
next: nextRaw,
@ -306,12 +306,16 @@ export class AlertGroupStore extends BaseStore {
const newAlerts = new Map(
results.map((alert: Alert) => {
const oldAlert = this.alerts.get(alert.pk) || {};
const mergedAlertData = { ...oldAlert, ...alert };
const mergedAlertData = { ...oldAlert, ...alert, undoAction: alert.undoAction };
return [alert.pk, mergedAlertData];
})
);
runInAction(() => {
// If previous fetch took longer than the next one, we ignore result of the previous fetch
if (timestamp !== this.latestFetchAlertGroupsTimestamp) {
return;
}
// @ts-ignore
this.alerts = new Map<number, Alert>([...this.alerts, ...newAlerts]);
@ -321,8 +325,10 @@ export class AlertGroupStore extends BaseStore {
results: results.map((alert: Alert) => alert.pk),
page_size,
};
this.alertGroupsLoading = false;
this.rootStore.loaderStore.setLoadingAction(
[ActionKey.FETCH_INCIDENTS, ActionKey.FETCH_INCIDENTS_POLLING],
false
);
});
}

View file

@ -3,4 +3,9 @@ export enum ActionKey {
REMOVE_COLUMN_FROM_ALERT_GROUP = 'REMOVE_COLUMN_FROM_ALERT_GROUP',
RESET_COLUMNS_FROM_ALERT_GROUP = 'RESET_COLUMNS_FROM_ALERT_GROUP',
UPDATE_PERSONAL_EVENTS = 'UPDATE_PERSONAL_EVENTS',
UPDATE_ALERT_GROUP = 'UPDATE_ALERT_GROUP',
FETCH_INCIDENTS = 'FETCH_INCIDENTS',
FETCH_INCIDENTS_POLLING = 'FETCH_INCIDENTS_POLLING',
FETCH_INCIDENTS_AND_STATS = 'FETCH_INCIDENTS_AND_STATS',
UPDATE_FILTERS_AND_FETCH_INCIDENTS = 'UPDATE_FILTERS_AND_FETCH_INCIDENTS',
}

View file

@ -0,0 +1,7 @@
import { LoaderStore } from './loader';
export class LoaderHelper {
static isLoading(store: typeof LoaderStore, actionKey: string | string[]) {
return typeof actionKey === 'string' ? store.items[actionKey] : actionKey.some((key) => store.items[key]);
}
}

View file

@ -13,13 +13,15 @@ class LoaderStoreClass {
}
@action.bound
setLoadingAction(actionKey: string, isLoading: boolean) {
this.items[actionKey] = isLoading;
setLoadingAction(actionKey: string | string[], isLoading: boolean) {
if (Array.isArray(actionKey)) {
actionKey.forEach((key) => {
this.items[key] = isLoading;
});
} else {
this.items[actionKey] = isLoading;
}
}
isLoading = (actionKey: string): boolean => {
return !!this.items[actionKey];
};
}
export const LoaderStore = new LoaderStoreClass();

View file

@ -81,6 +81,11 @@
max-width: 25%;
}
.loadingPlaceholder {
margin-bottom: 0;
text-align: center;
}
@media (max-width: 1200px) {
.col {
flex: 0 0 50%;

View file

@ -2,7 +2,15 @@ import React, { SyntheticEvent } from 'react';
import { SelectableValue } from '@grafana/data';
import { LabelTag } from '@grafana/labels';
import { Button, HorizontalGroup, Icon, RadioButtonGroup, Tooltip, VerticalGroup } from '@grafana/ui';
import {
Button,
HorizontalGroup,
Icon,
LoadingPlaceholder,
RadioButtonGroup,
Tooltip,
VerticalGroup,
} from '@grafana/ui';
import cn from 'classnames/bind';
import { capitalize } from 'lodash-es';
import { observer } from 'mobx-react';
@ -36,6 +44,8 @@ import {
AlertGroupColumnType,
} from 'models/alertgroup/alertgroup.types';
import { LabelKeyValue } from 'models/label/label.types';
import { ActionKey } from 'models/loader/action-keys';
import { LoaderHelper } from 'models/loader/loader.helpers';
import { renderRelatedUsers } from 'pages/incident/Incident.helpers';
import { AppFeature } from 'state/features';
import { PageProps, WithStoreProps } from 'state/types';
@ -66,6 +76,7 @@ interface IncidentsPageState {
showAddAlertGroupForm: boolean;
isSelectorColumnMenuOpen: boolean;
isHorizontalScrolling: boolean;
isFirstIncidentsFetchDone: boolean;
}
const POLLING_NUM_SECONDS = 15;
@ -122,6 +133,7 @@ class _IncidentsPage extends React.Component<IncidentsPageProps, IncidentsPageSt
},
isSelectorColumnMenuOpen: true,
isHorizontalScrolling: getItem(INCIDENT_HORIZONTAL_SCROLLING_STORAGE) || false,
isFirstIncidentsFetchDone: false,
};
}
@ -138,6 +150,8 @@ class _IncidentsPage extends React.Component<IncidentsPageProps, IncidentsPageSt
if (store.hasFeature(AppFeature.Labels)) {
alertGroupStore.fetchTableSettings();
}
this.setPollingInterval();
}
componentWillUnmount(): void {
@ -330,16 +344,14 @@ class _IncidentsPage extends React.Component<IncidentsPageProps, IncidentsPageSt
this.setState({
filters,
selectedIncidentIds: [],
affectedRows: {},
});
if (!isOnMount) {
this.setPagination(1, alertGroupStore.alertsSearchResult['default'].page_size);
}
this.clearPollingInterval();
this.setPollingInterval(filters, isOnMount);
await this.fetchIncidentData(filters, isOnMount);
await this.fetchIncidentData(filters);
if (isOnMount) {
this.setPagination(start, start + alertGroupStore.alertsSearchResult['default'].page_size - 1);
@ -355,10 +367,14 @@ class _IncidentsPage extends React.Component<IncidentsPageProps, IncidentsPageSt
});
};
fetchIncidentData = async (filters: IncidentsFiltersType, isOnMount: boolean) => {
fetchIncidentData = async (filters: IncidentsFiltersType) => {
const { store } = this.props;
await store.alertGroupStore.updateIncidentFilters(filters, isOnMount); // this line fetches the incidents
await store.alertGroupStore.updateIncidentFiltersAndRefetchIncidentsAndStats(
filters,
!this.state.isFirstIncidentsFetchDone
);
LocationHelper.update({ ...store.alertGroupStore.incidentFilters }, 'partial');
this.setState({ isFirstIncidentsFetchDone: true });
};
onChangeCursor = (cursor: string, direction: 'prev' | 'next') => {
@ -418,6 +434,7 @@ class _IncidentsPage extends React.Component<IncidentsPageProps, IncidentsPageSt
const { results } = store.alertGroupStore.getAlertSearchResult('default');
const hasSelected = selectedIncidentIds.length > 0;
const isLoading = LoaderHelper.isLoading(store.loaderStore, ActionKey.FETCH_INCIDENTS);
const hasInvalidatedAlert = Boolean(
(results && results.some((alert: AlertType) => alert.undoAction)) || Object.keys(affectedRows).length
);
@ -475,20 +492,19 @@ class _IncidentsPage extends React.Component<IncidentsPageProps, IncidentsPageSt
</div>
<div className={cx('fields-dropdown')}>
<RenderConditionally shouldRender={hasInvalidatedAlert}>
<RenderConditionally shouldRender={!isLoading && hasInvalidatedAlert}>
<HorizontalGroup spacing="xs">
<Text type="secondary">Results out of date</Text>
<Button
className={cx('btn-results')}
disabled={store.alertGroupStore.alertGroupsLoading}
variant="primary"
onClick={this.onIncidentsUpdateClick}
>
<Button className={cx('btn-results')} variant="primary" onClick={this.onIncidentsUpdateClick}>
Refresh
</Button>
</HorizontalGroup>
</RenderConditionally>
<RenderConditionally shouldRender={isLoading}>
<LoadingPlaceholder text="Loading..." className={cx('loadingPlaceholder')} />
</RenderConditionally>
<RenderConditionally shouldRender={store.hasFeature(AppFeature.Labels)}>
<RadioButtonGroup
options={TABLE_SCROLL_OPTIONS}
@ -505,10 +521,11 @@ class _IncidentsPage extends React.Component<IncidentsPageProps, IncidentsPageSt
renderTable() {
const { selectedIncidentIds, pagination, isHorizontalScrolling } = this.state;
const { alertGroupStore, filtersStore } = this.props.store;
const { alertGroupStore, filtersStore, loaderStore } = this.props.store;
const { results, prev, next } = alertGroupStore.getAlertSearchResult('default');
const isLoading = alertGroupStore.alertGroupsLoading || filtersStore.options['incidents'] === undefined;
const isLoading =
LoaderHelper.isLoading(loaderStore, ActionKey.FETCH_INCIDENTS) || filtersStore.options['incidents'] === undefined;
if (results && !results.length) {
return (
@ -538,7 +555,6 @@ class _IncidentsPage extends React.Component<IncidentsPageProps, IncidentsPageSt
{this.renderBulkActions()}
<GTable
emptyText={isLoading ? 'Loading...' : 'No alert groups found'}
loading={isLoading}
className={cx({ 'horizontal-scroll-table': isHorizontalScrolling })}
rowSelection={{
selectedRowKeys: selectedIncidentIds,
@ -912,7 +928,7 @@ class _IncidentsPage extends React.Component<IncidentsPageProps, IncidentsPageSt
this.setPollingInterval();
store.alertGroupStore.liveUpdatesPaused = true;
store.alertGroupStore.setLiveUpdatesPaused(true);
const delay = typeof event === 'number' ? event : 0;
this.setState(
@ -940,7 +956,7 @@ class _IncidentsPage extends React.Component<IncidentsPageProps, IncidentsPageSt
const { store } = this.props;
this.setState({ affectedRows: {} }, () => {
store.alertGroupStore.updateIncidents();
store.alertGroupStore.fetchIncidentsAndStats();
});
};
@ -949,13 +965,20 @@ class _IncidentsPage extends React.Component<IncidentsPageProps, IncidentsPageSt
this.pollingIntervalId = null;
}
setPollingInterval(filters: IncidentsFiltersType = this.state.filters, isOnMount = false) {
setPollingInterval() {
const startPolling = (delayed = false) => {
this.pollingIntervalId = setTimeout(
async () => {
const isBrowserWindowInactive = document.hidden;
if (!isBrowserWindowInactive) {
await this.fetchIncidentData(filters, isOnMount);
if (
!isBrowserWindowInactive &&
!LoaderHelper.isLoading(this.props.store.loaderStore, [
ActionKey.FETCH_INCIDENTS,
ActionKey.FETCH_INCIDENTS_POLLING,
]) &&
!this.props.store.alertGroupStore.liveUpdatesPaused
) {
await this.props.store.alertGroupStore.fetchIncidentsAndStats(true);
}
if (this.pollingIntervalId === null) {

View file

@ -7,7 +7,7 @@ const FIVE_SECS = 5_000;
export const useAlertCreationChecker = () => {
const {
alertGroupStore: { updateAlertGroups, alerts },
alertGroupStore: { fetchAlertGroups, alerts },
} = useStore();
const [isFirstAlertCheckDone, setIsFirstAlertCheckDone] = useState(false);
@ -20,7 +20,7 @@ export const useAlertCreationChecker = () => {
useEffect(() => {
const fetch = async () => {
if (!isAnyAlertCreatedMoreThan20SecsAgo) {
await updateAlertGroups();
await fetchAlertGroups();
}
setIsFirstAlertCheckDone(true);
};

View file

@ -9,3 +9,7 @@
.spaceTop {
margin-top: 16px;
}
.alertBox {
margin-top: 32px;
}

View file

@ -131,9 +131,11 @@ const NoDatasourceWarning = () => {
);
return alertVisible ? (
<Alert onRemove={() => setAlertVisible(false)} severity="warning" title="" className={styles.alertBox}>
Insights data has missing Prometheus configuration. Open OnCall {docsLink} to see how to setup it.
</Alert>
<div className={styles.alertBox}>
<Alert onRemove={() => setAlertVisible(false)} severity="warning" title="">
Insights data has missing Prometheus configuration. Open OnCall {docsLink} to see how to setup it.
</Alert>
</div>
) : null;
};

View file

@ -270,7 +270,6 @@ class _IntegrationsPage extends React.Component<IntegrationsProps, IntegrationsS
)}
<GTable
emptyText={count === undefined ? 'Loading...' : 'No integrations found'}
loading={count === undefined}
data-testid="integrations-table"
rowKey="id"
data={results}

View file

@ -104,7 +104,6 @@ class _SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSt
<GTable
columns={this.getTableColumns()}
data={results}
loading={!results}
pagination={{
page,
total: results ? Math.ceil((count || 0) / page_size) : 0,

View file

@ -3,13 +3,19 @@ import { openErrorNotification, openNotification, openWarningNotification } from
export function AutoLoadingState(actionKey: string) {
return function (_target: object, _key: string, descriptor: PropertyDescriptor) {
let nbOfPendingActions = 0;
const originalFunction = descriptor.value;
descriptor.value = async function (...args: any) {
LoaderStore.setLoadingAction(actionKey, true);
nbOfPendingActions++;
try {
await originalFunction.apply(this, args);
} finally {
LoaderStore.setLoadingAction(actionKey, false);
nbOfPendingActions--;
// if there are other pending actions with the same key, wait till the last one is done
if (nbOfPendingActions === 0) {
LoaderStore.setLoadingAction(actionKey, false);
}
}
};
};

View file

@ -3,6 +3,7 @@ import React, { useEffect, useRef, useState } from 'react';
import { useLocation } from 'react-router-dom';
import { ActionKey } from 'models/loader/action-keys';
import { LoaderHelper } from 'models/loader/loader.helpers';
import { useStore } from 'state/useStore';
export function useForceUpdate() {
@ -74,8 +75,6 @@ export function useDebouncedCallback<A extends any[]>(callback: (...args: A) =>
}
export const useIsLoading = (actionKey: ActionKey) => {
const {
loaderStore: { isLoading },
} = useStore();
return isLoading(actionKey);
const { loaderStore } = useStore();
return LoaderHelper.isLoading(loaderStore, actionKey);
};