From 871860c39907dd27905cfbd3281cde3f855ac2b2 Mon Sep 17 00:00:00 2001 From: Rares Mardare Date: Mon, 30 Oct 2023 16:20:14 +0200 Subject: [PATCH 1/6] Pagination refactoring/cleanup (#3205) # What this PR does - Pagination cleanup so that frontend no longer hardcodes page size - Moved pagination to the filters store instead for all pages that have pagination enabled - Prevent UI triggering polling if window is inactive or if the previous request didn't complete ## Which issue(s) this PR fixes #2931 #2462 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 3 + .../RemoteFilters/RemoteFilters.tsx | 29 +- .../alert_receive_channel.ts | 6 +- .../src/models/alertgroup/alertgroup.ts | 69 ++-- grafana-plugin/src/models/filters/filters.ts | 9 + grafana-plugin/src/models/user/user.ts | 6 +- .../src/pages/incidents/Incidents.tsx | 306 ++++++++++-------- .../src/pages/integrations/Integrations.tsx | 21 +- .../outgoing_webhooks/OutgoingWebhooks.tsx | 2 +- .../src/pages/schedules/Schedules.tsx | 35 +- grafana-plugin/src/pages/users/Users.tsx | 38 ++- .../src/state/rootBaseStore/index.ts | 13 - grafana-plugin/src/utils/consts.ts | 1 + 13 files changed, 286 insertions(+), 252 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57f84435..b38818f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Simplify Direct Paging workflow. Now when using Direct Paging you either simply specify a team, or one or more users to page by @joeyorlando ([#3128](https://github.com/grafana/oncall/pull/3128)) - Enable timing options for mobile push notifications, allow multi-select by @Ferril ([#3187](https://github.com/grafana/oncall/pull/3187)) +- Removed the hardcoding of page size on frontend ([#3205](https://github.com/grafana/oncall/pull/#3205)) +- Prevent additional polling on Incidents if the previous request didn't complete + ([#3205](https://github.com/grafana/oncall/pull/#3205)) ### Fixed diff --git a/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.tsx b/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.tsx index bd86eebc..bfcdc2eb 100644 --- a/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.tsx +++ b/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.tsx @@ -1,6 +1,6 @@ import React, { Component } from 'react'; -import { SelectableValue, TimeRange } from '@grafana/data'; +import { KeyValue, SelectableValue, TimeRange } from '@grafana/data'; import { InlineSwitch, MultiSelect, @@ -31,16 +31,15 @@ import LocationHelper from 'utils/LocationHelper'; import { PAGE } from 'utils/consts'; import { parseFilters } from './RemoteFilters.helpers'; -import { FilterOption, RemoteFiltersType } from './RemoteFilters.types'; +import { FilterOption } from './RemoteFilters.types'; import styles from './RemoteFilters.module.css'; const cx = cn.bind(styles); interface RemoteFiltersProps extends WithStoreProps { - value: RemoteFiltersType; onChange: (filters: { [key: string]: any }, isOnMount: boolean, invalidateFn: () => boolean) => void; - query: { [key: string]: any }; + query: KeyValue; page: PAGE; defaultFilters?: FiltersValues; extraFilters?: (state, setState, onFiltersValueChange) => React.ReactNode; @@ -82,11 +81,18 @@ class RemoteFilters extends Component { } async componentDidMount() { - const { query, page, store, defaultFilters } = this.props; - - const { filtersStore } = store; + const { + query, + page, + store: { filtersStore }, + defaultFilters, + } = this.props; const filterOptions = await filtersStore.updateOptionsForPage(page); + const currentTablePageNum = parseInt(filtersStore.currentTablePageNum[page] || query.p || 1, 10); + + // set the current page from filters/query or default it to 1 + filtersStore.setCurrentTablePageNum(page, currentTablePageNum); let { filters, values } = parseFilters({ ...query, ...filtersStore.globalValues }, filterOptions, query); @@ -422,10 +428,7 @@ class RemoteFilters extends Component { } const currentRequestId = this.getNewRequestId(); - - this.setState({ - lastRequestId: currentRequestId, - }); + this.setState({ lastRequestId: currentRequestId }); LocationHelper.update({ ...values }, 'partial'); onChange(values, isOnMount, this.invalidateFn.bind(this, currentRequestId)); @@ -443,4 +446,6 @@ class RemoteFilters extends Component { debouncedOnChange = debounce(this.onChange, 500); } -export default withMobXProviderContext(RemoteFilters); +export default withMobXProviderContext(RemoteFilters) as unknown as React.ComponentClass< + Omit +>; diff --git a/grafana-plugin/src/models/alert_receive_channel/alert_receive_channel.ts b/grafana-plugin/src/models/alert_receive_channel/alert_receive_channel.ts index 2978c5b0..67afa9e6 100644 --- a/grafana-plugin/src/models/alert_receive_channel/alert_receive_channel.ts +++ b/grafana-plugin/src/models/alert_receive_channel/alert_receive_channel.ts @@ -28,7 +28,7 @@ export class AlertReceiveChannelStore extends BaseStore { searchResult: Array; @observable.shallow - paginatedSearchResult: { count?: number; results?: Array } = {}; + paginatedSearchResult: { count?: number; results?: Array; page_size?: number } = {}; @observable.shallow items: { [id: string]: AlertReceiveChannel } = {}; @@ -81,6 +81,7 @@ export class AlertReceiveChannelStore extends BaseStore { } return { + page_size: this.paginatedSearchResult.page_size, count: this.paginatedSearchResult.count, results: this.paginatedSearchResult.results && @@ -133,7 +134,7 @@ export class AlertReceiveChannelStore extends BaseStore { async updatePaginatedItems(query: any = '', page = 1, updateCounters = false, invalidateFn = undefined) { const filters = typeof query === 'string' ? { search: query } : query; - const { count, results } = await makeRequest(this.path, { params: { ...filters, page } }); + const { count, results, page_size } = await makeRequest(this.path, { params: { ...filters, page } }); if (invalidateFn?.()) { return undefined; @@ -155,6 +156,7 @@ export class AlertReceiveChannelStore extends BaseStore { this.paginatedSearchResult = { count, results: results.map((item: AlertReceiveChannel) => item.id), + page_size, }; if (updateCounters) { diff --git a/grafana-plugin/src/models/alertgroup/alertgroup.ts b/grafana-plugin/src/models/alertgroup/alertgroup.ts index f0068af1..72a645d3 100644 --- a/grafana-plugin/src/models/alertgroup/alertgroup.ts +++ b/grafana-plugin/src/models/alertgroup/alertgroup.ts @@ -41,10 +41,14 @@ export class AlertGroupStore extends BaseStore { incidentsCursor?: string; @observable - incidentsItemsPerPage?: number; - - @observable - alertsSearchResult: any = {}; + alertsSearchResult: { + [key: string]: { + prev?: string; + next?: string; + results?: string[]; + page_size?: number; + }; + } = {}; @observable alerts = new Map(); @@ -89,29 +93,6 @@ export class AlertGroupStore extends BaseStore { }).catch(showApiError); } - @action // FIXME for `attach to` feature ONLY - async updateItems(query = '') { - const { results } = await makeRequest(`${this.path}`, { - params: { search: query, resolved: false, is_root: true }, - }); - - this.items = { - ...this.items, - ...results.reduce( - (acc: { [key: string]: Alert }, item: Alert) => ({ - ...acc, - [item.pk]: item, - }), - {} - ), - }; - - this.searchResult = { - ...this.searchResult, - [query]: results.map((item: Alert) => item.pk), - }; - } - async updateItem(id: Alert['pk']) { const item = await this.getById(id); @@ -220,12 +201,13 @@ export class AlertGroupStore extends BaseStore { // TODO check if methods are dublicating existing ones @action async updateIncidents() { - this.getNewIncidentsStats(); - this.getAcknowledgedIncidentsStats(); - this.getResolvedIncidentsStats(); - this.getSilencedIncidentsStats(); - - this.updateAlertGroups(); + await Promise.all([ + this.getNewIncidentsStats(), + this.getAcknowledgedIncidentsStats(), + this.getResolvedIncidentsStats(), + this.getSilencedIncidentsStats(), + this.updateAlertGroups(), + ]); this.liveUpdatesPaused = false; } @@ -238,7 +220,7 @@ export class AlertGroupStore extends BaseStore { this.incidentFilters = params; - this.updateIncidents(); + await this.updateIncidents(); } @action @@ -256,9 +238,8 @@ export class AlertGroupStore extends BaseStore { } @action - async setIncidentsItemsPerPage(value: number) { + async setIncidentsItemsPerPage() { this.setIncidentsCursor(undefined); - this.incidentsItemsPerPage = value; this.updateAlertGroups(); } @@ -271,11 +252,12 @@ export class AlertGroupStore extends BaseStore { results, next: nextRaw, previous: previousRaw, + page_size, } = await makeRequest(`${this.path}`, { params: { ...this.incidentFilters, + perpage: this.alertsSearchResult?.['default']?.page_size, cursor: this.incidentsCursor, - perpage: this.incidentsItemsPerPage, is_root: true, }, }).catch(refreshPageError); @@ -298,17 +280,24 @@ export class AlertGroupStore extends BaseStore { prev: prevCursor, next: nextCursor, results: results.map((alert: Alert) => alert.pk), + page_size, }; this.alertGroupsLoading = false; } getAlertSearchResult(query: string) { - if (!this.alertsSearchResult[query]) { - return undefined; + const result = this.alertsSearchResult[query]; + if (!result) { + return {}; } - return this.alertsSearchResult[query].results.map((pk: Alert['pk']) => this.alerts.get(pk)); + return { + prev: result.prev, + next: result.next, + page_size: result.page_size, + results: result.results.map((pk: Alert['pk']) => this.alerts.get(pk)), + }; } @action diff --git a/grafana-plugin/src/models/filters/filters.ts b/grafana-plugin/src/models/filters/filters.ts index d3acff00..472dd727 100644 --- a/grafana-plugin/src/models/filters/filters.ts +++ b/grafana-plugin/src/models/filters/filters.ts @@ -3,6 +3,7 @@ import { action, observable } from 'mobx'; import BaseStore from 'models/base_store'; import { makeRequest } from 'network'; import { RootStore } from 'state'; +import { PAGE } from 'utils/consts'; import { getItem, setItem } from 'utils/localStorage'; import { getApiPathByPage } from './filters.helpers'; @@ -17,6 +18,9 @@ export class FiltersStore extends BaseStore { @observable.shallow public values: { [page: string]: FiltersValues } = {}; + @observable.shallow + public currentTablePageNum: { [page: string]: number } = {}; + private _globalValues: FiltersValues = {}; @observable @@ -65,4 +69,9 @@ export class FiltersStore extends BaseStore { [page]: value, }; } + + @action + setCurrentTablePageNum(page: PAGE, currentTablePageNum: number) { + this.currentTablePageNum[page] = currentTablePageNum; + } } diff --git a/grafana-plugin/src/models/user/user.ts b/grafana-plugin/src/models/user/user.ts index dd92f538..1fd01a03 100644 --- a/grafana-plugin/src/models/user/user.ts +++ b/grafana-plugin/src/models/user/user.ts @@ -17,7 +17,7 @@ import { User } from './user.types'; export class UserStore extends BaseStore { @observable.shallow - searchResult: { count?: number; results?: Array } = {}; + searchResult: { count?: number; results?: Array; page_size?: number } = {}; @observable.shallow items: { [pk: string]: User } = {}; @@ -122,7 +122,7 @@ export class UserStore extends BaseStore { return; } - const { count, results } = response; + const { count, results, page_size } = response; this.items = { ...this.items, @@ -140,6 +140,7 @@ export class UserStore extends BaseStore { this.searchResult = { count, + page_size, results: results.map((item: User) => item.pk), }; @@ -148,6 +149,7 @@ export class UserStore extends BaseStore { getSearchResult() { return { + page_size: this.searchResult.page_size, count: this.searchResult.count, results: this.searchResult.results && this.searchResult.results.map((userPk: User['pk']) => this.items?.[userPk]), }; diff --git a/grafana-plugin/src/pages/incidents/Incidents.tsx b/grafana-plugin/src/pages/incidents/Incidents.tsx index 0b554315..8dc34b7d 100644 --- a/grafana-plugin/src/pages/incidents/Incidents.tsx +++ b/grafana-plugin/src/pages/incidents/Incidents.tsx @@ -1,8 +1,7 @@ -import React, { ReactElement, SyntheticEvent } from 'react'; +import React, { SyntheticEvent } from 'react'; -import { Button, HorizontalGroup, Icon, LoadingPlaceholder, VerticalGroup } from '@grafana/ui'; +import { Button, HorizontalGroup, Icon, VerticalGroup } from '@grafana/ui'; import cn from 'classnames/bind'; -import { get } from 'lodash-es'; import { observer } from 'mobx-react'; import moment from 'moment-timezone'; import Emoji from 'react-emoji-render'; @@ -40,19 +39,6 @@ interface Pagination { start: number; end: number; } - -function withSkeleton(fn: (alert: AlertType) => ReactElement | ReactElement[]) { - const WithSkeleton = (alert: AlertType) => { - if (alert.short) { - return ; - } - - return fn(alert); - }; - - return WithSkeleton; -} - interface IncidentsPageProps extends WithStoreProps, PageProps, RouteComponentProps {} interface IncidentsPageState { @@ -63,9 +49,14 @@ interface IncidentsPageState { showAddAlertGroupForm: boolean; } -const ITEMS_PER_PAGE = 25; const POLLING_NUM_SECONDS = 15; +const PAGINATION_OPTIONS = [ + { label: '25', value: 25 }, + { label: '50', value: 50 }, + { label: '100', value: 100 }, +]; + @observer class Incidents extends React.Component { constructor(props: IncidentsPageProps) { @@ -76,12 +67,10 @@ class Incidents extends React.Component query: { cursor: cursorQuery, start: startQuery, perpage: perpageQuery }, } = props; - const cursor = cursorQuery || undefined; const start = !isNaN(startQuery) ? Number(startQuery) : 1; - const itemsPerPage = !isNaN(perpageQuery) ? Number(perpageQuery) : ITEMS_PER_PAGE; + const pageSize = !isNaN(perpageQuery) ? Number(perpageQuery) : undefined; - store.alertGroupStore.incidentsCursor = cursor; - store.alertGroupStore.incidentsItemsPerPage = itemsPerPage; + store.alertGroupStore.incidentsCursor = cursorQuery || undefined; this.state = { selectedIncidentIds: [], @@ -89,16 +78,20 @@ class Incidents extends React.Component showAddAlertGroupForm: false, pagination: { start, - end: start + itemsPerPage - 1, + end: start + pageSize, }, }; - - store.alertGroupStore.updateBulkActions(); - store.alertGroupStore.updateSilenceOptions(); } private pollingIntervalId: NodeJS.Timer = undefined; + componentDidMount() { + const { alertGroupStore } = this.props.store; + + alertGroupStore.updateBulkActions(); + alertGroupStore.updateSilenceOptions(); + } + componentWillUnmount(): void { this.clearPollingInterval(); } @@ -279,8 +272,12 @@ class Incidents extends React.Component this.setState({ showAddAlertGroupForm: true }); }; - handleFiltersChange = (filters: IncidentsFiltersType, isOnMount: boolean) => { - const { store } = this.props; + handleFiltersChange = async (filters: IncidentsFiltersType, isOnMount: boolean) => { + const { + store: { alertGroupStore }, + } = this.props; + + const { start } = this.state.pagination; this.setState({ filters, @@ -288,45 +285,50 @@ class Incidents extends React.Component }); if (!isOnMount) { - this.setState({ - pagination: { - start: 1, - end: store.alertGroupStore.incidentsItemsPerPage, - }, - }); + this.setPagination(1, alertGroupStore.alertsSearchResult['default'].page_size); } this.clearPollingInterval(); this.setPollingInterval(filters, isOnMount); - this.fetchIncidentData(filters, isOnMount); + + await this.fetchIncidentData(filters, isOnMount); + + if (isOnMount) { + this.setPagination(start, start + alertGroupStore.alertsSearchResult['default'].page_size - 1); + } }; - fetchIncidentData = (filters: IncidentsFiltersType, isOnMount: boolean) => { + setPagination = (start = this.state.pagination?.start, end = this.state.pagination?.end) => { + this.setState({ + pagination: { + start, + end, + }, + }); + }; + + fetchIncidentData = async (filters: IncidentsFiltersType, isOnMount: boolean) => { const { store } = this.props; - store.alertGroupStore.updateIncidentFilters(filters, isOnMount); // this line fetches incidents + await store.alertGroupStore.updateIncidentFilters(filters, isOnMount); // this line fetches the incidents LocationHelper.update({ ...store.alertGroupStore.incidentFilters }, 'partial'); }; onChangeCursor = (cursor: string, direction: 'prev' | 'next') => { - const { store } = this.props; + const { alertGroupStore } = this.props.store; + const pageSize = alertGroupStore.alertsSearchResult['default'].page_size; - store.alertGroupStore.updateIncidentsCursor(cursor); + alertGroupStore.updateIncidentsCursor(cursor); this.setState( { selectedIncidentIds: [], pagination: { - start: - this.state.pagination.start + store.alertGroupStore.incidentsItemsPerPage * (direction === 'prev' ? -1 : 1), - end: - this.state.pagination.end + store.alertGroupStore.incidentsItemsPerPage * (direction === 'prev' ? -1 : 1), + start: this.state.pagination.start + pageSize * (direction === 'prev' ? -1 : 1), + end: this.state.pagination.end + pageSize * (direction === 'prev' ? -1 : 1), }, }, () => { - LocationHelper.update( - { start: this.state.pagination.start, perpage: store.alertGroupStore.incidentsItemsPerPage }, - 'partial' - ); + LocationHelper.update({ start: this.state.pagination.start, perpage: pageSize }, 'partial'); } ); }; @@ -334,15 +336,22 @@ class Incidents extends React.Component handleChangeItemsPerPage = (value: number) => { const { store } = this.props; - store.alertGroupStore.setIncidentsItemsPerPage(value); + store.alertGroupStore.alertsSearchResult['default'] = { + ...store.alertGroupStore.alertsSearchResult['default'], + page_size: value, + }; + + store.alertGroupStore.setIncidentsItemsPerPage(); this.setState({ selectedIncidentIds: [], pagination: { start: 1, - end: store.alertGroupStore.incidentsItemsPerPage, + end: value, }, }); + + LocationHelper.update({ start: 1, perpage: value }, 'partial'); }; renderBulkActions = () => { @@ -353,7 +362,7 @@ class Incidents extends React.Component return null; } - const results = store.alertGroupStore.getAlertSearchResult('default'); + const { results } = store.alertGroupStore.getAlertSearchResult('default'); const hasSelected = selectedIncidentIds.length > 0; const hasInvalidatedAlert = Boolean( @@ -431,14 +440,9 @@ class Incidents extends React.Component renderTable() { const { selectedIncidentIds, pagination } = this.state; - const { - store, - store: { alertGroupStore, filtersStore }, - } = this.props; + const { alertGroupStore, filtersStore } = this.props.store; - const results = alertGroupStore.getAlertSearchResult('default'); - const prev = get(alertGroupStore.alertsSearchResult, `default.prev`); - const next = get(alertGroupStore.alertsSearchResult, `default.next`); + const { results, prev, next } = alertGroupStore.getAlertSearchResult('default'); const isLoading = alertGroupStore.alertGroupsLoading || filtersStore.options['incidents'] === undefined; if (results && !results.length) { @@ -462,57 +466,6 @@ class Incidents extends React.Component ); } - const columns = [ - { - width: '140px', - title: 'Status', - key: 'time', - render: withSkeleton(this.renderStatus), - }, - { - width: '10%', - title: 'ID', - key: 'id', - render: withSkeleton(this.renderId), - }, - { - width: '35%', - title: 'Title', - key: 'title', - render: withSkeleton(this.renderTitle), - }, - { - width: '5%', - title: 'Alerts', - key: 'alerts', - render: withSkeleton(this.renderAlertsCounter), - }, - { - width: '15%', - title: 'Integration', - key: 'source', - render: withSkeleton(this.renderSource), - }, - { - width: '10%', - title: 'Created', - key: 'created', - render: withSkeleton(this.renderStartedAt), - }, - { - width: '10%', - title: 'Team', - key: 'team', - render: withSkeleton((item: AlertType) => this.renderTeam(item, store.grafanaTeamStore.items)), - }, - { - width: '15%', - title: 'Users', - key: 'users', - render: withSkeleton(renderRelatedUsers), - }, - ]; - return (
{this.renderBulkActions()} @@ -526,33 +479,25 @@ class Incidents extends React.Component }} rowKey="pk" data={results} - columns={columns} + columns={this.getTableColumns()} /> -
- -
+ {this.shouldShowPagination() && ( +
+ +
+ )}
); } - handleSelectedIncidentIdsChange = (ids: Array) => { - this.setState({ selectedIncidentIds: ids }, () => { - ids.length > 0 ? this.clearPollingInterval() : this.setPollingInterval(); - }); - }; - renderId(record: AlertType) { return ( @@ -565,11 +510,8 @@ class Incidents extends React.Component renderTitle = (record: AlertType) => { const { store, query } = this.props; - const { - pagination: { start }, - } = this.state; - - const { incidentsItemsPerPage, incidentsCursor } = store.alertGroupStore; + const { start } = this.state.pagination || {}; + const { incidentsCursor } = store.alertGroupStore; return (
@@ -580,7 +522,7 @@ class Incidents extends React.Component page: 'alert-groups', id: record.pk, cursor: incidentsCursor, - perpage: incidentsItemsPerPage, + perpage: store.alertGroupStore.alertsSearchResult?.['default']?.page_size, start, ...query, }} @@ -649,6 +591,77 @@ class Incidents extends React.Component ); } + shouldShowPagination() { + const { alertGroupStore } = this.props.store; + + return Boolean( + this.state.pagination?.start && + this.state.pagination?.end && + alertGroupStore.alertsSearchResult?.['default']?.page_size + ); + } + + handleSelectedIncidentIdsChange = (ids: Array) => { + this.setState({ selectedIncidentIds: ids }, () => { + ids.length > 0 ? this.clearPollingInterval() : this.setPollingInterval(); + }); + }; + + getTableColumns(): Array<{ width: string; title: string; key: string; render }> { + const { store } = this.props; + + return [ + { + width: '140px', + title: 'Status', + key: 'time', + render: this.renderStatus, + }, + { + width: '10%', + title: 'ID', + key: 'id', + render: this.renderId, + }, + { + width: '35%', + title: 'Title', + key: 'title', + render: this.renderTitle, + }, + { + width: '5%', + title: 'Alerts', + key: 'alerts', + render: this.renderAlertsCounter, + }, + { + width: '15%', + title: 'Integration', + key: 'source', + render: this.renderSource, + }, + { + width: '10%', + title: 'Created', + key: 'created', + render: this.renderStartedAt, + }, + { + width: '10%', + title: 'Team', + key: 'team', + render: (item: AlertType) => this.renderTeam(item, store.grafanaTeamStore.items), + }, + { + width: '15%', + title: 'Users', + key: 'users', + render: renderRelatedUsers, + }, + ]; + } + getOnActionButtonClick = (incidentId: string, action: AlertAction): ((e: SyntheticEvent) => Promise) => { const { store } = this.props; @@ -719,13 +732,28 @@ class Incidents extends React.Component clearPollingInterval() { clearInterval(this.pollingIntervalId); - this.pollingIntervalId = undefined; + this.pollingIntervalId = null; } setPollingInterval(filters: IncidentsFiltersType = this.state.filters, isOnMount = false) { - this.pollingIntervalId = setInterval(() => { - this.fetchIncidentData(filters, isOnMount); - }, POLLING_NUM_SECONDS * 1000); + const startPolling = (delayed = false) => { + this.pollingIntervalId = setTimeout( + async () => { + const isBrowserWindowInactive = document.hidden; + if (!isBrowserWindowInactive) { + await this.fetchIncidentData(filters, isOnMount); + } + + if (this.pollingIntervalId === null) { + return; + } + startPolling(isBrowserWindowInactive); + }, + delayed ? 60 * 1000 : POLLING_NUM_SECONDS * 1000 + ); + }; + + startPolling(); } } diff --git a/grafana-plugin/src/pages/integrations/Integrations.tsx b/grafana-plugin/src/pages/integrations/Integrations.tsx index f3882f25..64730ffa 100644 --- a/grafana-plugin/src/pages/integrations/Integrations.tsx +++ b/grafana-plugin/src/pages/integrations/Integrations.tsx @@ -42,7 +42,6 @@ import styles from './Integrations.module.scss'; const cx = cn.bind(styles); const FILTERS_DEBOUNCE_MS = 500; -const ITEMS_PER_PAGE = 15; interface IntegrationsState extends PageBaseState { integrationsFilters: Record; @@ -66,15 +65,11 @@ class Integrations extends React.Component constructor(props: IntegrationsProps) { super(props); - const { query, store } = props; - this.state = { integrationsFilters: { searchTerm: '' }, errorData: initErrorDataState(), confirmationModal: undefined, }; - - store.currentPage['integrations'] = Number(store.currentPage['integrations'] || query.p || 1); } async componentDidMount() { @@ -121,7 +116,7 @@ class Integrations extends React.Component update = () => { const { store } = this.props; const { integrationsFilters } = this.state; - const page = store.currentPage['integrations']; + const page = store.filtersStore.currentTablePageNum[PAGE.Integrations]; LocationHelper.update({ p: page }, 'partial'); @@ -135,7 +130,7 @@ class Integrations extends React.Component const { alertReceiveChannelId, confirmationModal } = this.state; const { alertReceiveChannelStore } = store; - const { count, results } = alertReceiveChannelStore.getPaginatedSearchResult(); + const { count, results, page_size } = alertReceiveChannelStore.getPaginatedSearchResult(); return ( <> @@ -178,8 +173,8 @@ class Integrations extends React.Component className={cx('integrations-table')} rowClassName={cx('integrations-table-row')} pagination={{ - page: store.currentPage['integrations'], - total: Math.ceil((count || 0) / ITEMS_PER_PAGE), + page: store.filtersStore.currentTablePageNum[PAGE.Integrations], + total: results ? Math.ceil((count || 0) / page_size) : 0, onChange: this.handleChangePage, }} /> @@ -520,13 +515,13 @@ class Integrations extends React.Component invalidateRequestFn = (requestedPage: number) => { const { store } = this.props; - return requestedPage !== store.getCurrentPage(PAGE.Integrations); + return requestedPage !== store.filtersStore.currentTablePageNum[PAGE.Integrations]; }; handleChangePage = (page: number) => { const { store } = this.props; - store.currentPage['integrations'] = page; + store.filtersStore.currentTablePageNum[PAGE.Integrations] = page; this.update(); }; @@ -578,12 +573,12 @@ class Integrations extends React.Component const { alertReceiveChannelStore } = store; const { integrationsFilters } = this.state; - const newPage = isOnMount ? store.getCurrentPage(PAGE.Integrations) : 1; + const newPage = isOnMount ? store.filtersStore.currentTablePageNum[PAGE.Integrations] : 1; return alertReceiveChannelStore .updatePaginatedItems(integrationsFilters, newPage, false, () => this.invalidateRequestFn(newPage)) .then(() => { - store.setCurrentPage(PAGE.Integrations, newPage); + store.filtersStore.currentTablePageNum[PAGE.Integrations] = newPage; LocationHelper.update({ p: newPage }, 'partial'); }); }; diff --git a/grafana-plugin/src/pages/outgoing_webhooks/OutgoingWebhooks.tsx b/grafana-plugin/src/pages/outgoing_webhooks/OutgoingWebhooks.tsx index 99f2c1d3..825a4232 100644 --- a/grafana-plugin/src/pages/outgoing_webhooks/OutgoingWebhooks.tsx +++ b/grafana-plugin/src/pages/outgoing_webhooks/OutgoingWebhooks.tsx @@ -234,7 +234,7 @@ class OutgoingWebhooks extends React.Component { + handleFiltersChange = (filters: FiltersValues, isOnMount: boolean) => { const { store } = this.props; const { outgoingWebhookStore } = store; diff --git a/grafana-plugin/src/pages/schedules/Schedules.tsx b/grafana-plugin/src/pages/schedules/Schedules.tsx index e6c5d52b..83910d45 100644 --- a/grafana-plugin/src/pages/schedules/Schedules.tsx +++ b/grafana-plugin/src/pages/schedules/Schedules.tsx @@ -37,7 +37,6 @@ import { PAGE, PLUGIN_ROOT, TEXT_ELLIPSIS_CLASS } from 'utils/consts'; import styles from './Schedules.module.css'; const cx = cn.bind(styles); -const PAGE_SIZE_DEFAULT = 15; interface SchedulesPageProps extends WithStoreProps, RouteComponentProps, PageProps {} @@ -47,7 +46,6 @@ interface SchedulesPageState { showNewScheduleSelector: boolean; expandedRowKeys: Array; scheduleIdToEdit?: Schedule['id']; - page: number; } @observer @@ -63,7 +61,6 @@ class SchedulesPage extends React.Component boolean) => { - this.handleSchedulesFiltersChange(filters, isOnMount, invalidateFn); - }} + onChange={this.handleSchedulesFiltersChange} />
@@ -130,7 +126,7 @@ class SchedulesPage extends React.Component boolean) => { - this.setState({ filters, page: isOnMount ? this.state.page : 1 }, () => { + handleSchedulesFiltersChange = (filters: RemoteFiltersType, _isOnMount: boolean, invalidateFn: () => boolean) => { + this.setState({ filters }, () => { this.applyFilters(invalidateFn); }); }; applyFilters = (invalidateFn?: () => boolean) => { - const { scheduleStore } = this.props.store; - const { page, filters } = this.state; + const { scheduleStore, filtersStore } = this.props.store; + const { filters } = this.state; + const currentTablePage = filtersStore.currentTablePageNum[PAGE.Schedules]; - LocationHelper.update({ p: page }, 'partial'); - scheduleStore.updateItems(filters, page, invalidateFn); + LocationHelper.update({ p: currentTablePage }, 'partial'); + scheduleStore.updateItems(filters, currentTablePage, invalidateFn); }; handlePageChange = (page: number) => { - this.setState({ page, expandedRowKeys: [] }, this.applyFilters); + const { store } = this.props; + store.filtersStore.currentTablePageNum[PAGE.Schedules] = page; + + this.setState({ expandedRowKeys: [] }, this.applyFilters); }; update = () => { const { store } = this.props; - const { page, startMoment } = this.state; + const { startMoment } = this.state; + const page = store.filtersStore.currentTablePageNum[PAGE.Schedules]; store.scheduleStore.updatePersonalEvents(store.userStore.currentUserPk, startMoment, 9, true); diff --git a/grafana-plugin/src/pages/users/Users.tsx b/grafana-plugin/src/pages/users/Users.tsx index a4f0e87e..24bf1278 100644 --- a/grafana-plugin/src/pages/users/Users.tsx +++ b/grafana-plugin/src/pages/users/Users.tsx @@ -25,7 +25,7 @@ import { PageProps, WithStoreProps } from 'state/types'; import { withMobXProviderContext } from 'state/withStore'; import LocationHelper from 'utils/LocationHelper'; import { generateMissingPermissionMessage, isUserActionAllowed, UserActions } from 'utils/authorization'; -import { PLUGIN_ROOT } from 'utils/consts'; +import { PAGE, PLUGIN_ROOT } from 'utils/consts'; import { getUserRowClassNameFn } from './Users.helpers'; @@ -35,11 +35,9 @@ const cx = cn.bind(styles); interface UsersProps extends WithStoreProps, PageProps, RouteComponentProps<{ id: string }> {} -const ITEMS_PER_PAGE = 100; const REQUIRED_PERMISSION_TO_VIEW_USERS = UserActions.UserSettingsWrite; interface UsersState extends PageBaseState { - page: number; isWrongTeam: boolean; userPkToEdit?: UserType['pk'] | 'new'; usersFilters?: { @@ -54,10 +52,10 @@ class Users extends React.Component { const { query: { p }, + store: { filtersStore }, } = props; this.state = { - page: p ? Number(p) : 1, isWrongTeam: false, userPkToEdit: undefined, usersFilters: { @@ -66,6 +64,10 @@ class Users extends React.Component { errorData: initErrorDataState(), }; + + // Users component doesn't rely on RemoteFilters + // therefore we need to initialize the page in the constructor instead + filtersStore.currentTablePageNum[PAGE.Users] = p ? Number(p) : 1; } async componentDidMount() { @@ -74,8 +76,9 @@ class Users extends React.Component { updateUsers = async (invalidateFn?: () => boolean) => { const { store } = this.props; - const { usersFilters, page } = this.state; - const { userStore } = store; + const { usersFilters } = this.state; + const { userStore, filtersStore } = store; + const page = filtersStore.currentTablePageNum[PAGE.Users]; if (!isUserActionAllowed(REQUIRED_PERMISSION_TO_VIEW_USERS)) { return; @@ -84,7 +87,6 @@ class Users extends React.Component { LocationHelper.update({ p: page }, 'partial'); await userStore.updateItems(usersFilters, page, invalidateFn); - // otherwise MobX doesn't update :( this.forceUpdate(); }; @@ -171,12 +173,14 @@ class Users extends React.Component { renderContentIfAuthorized(authorizedToViewUsers: boolean) { const { - store: { userStore }, + store: { userStore, filtersStore }, } = this.props; - const { usersFilters, page, userPkToEdit } = this.state; + const { usersFilters, userPkToEdit } = this.state; - const { count, results } = userStore.getSearchResult(); + const page = filtersStore.currentTablePageNum[PAGE.Users]; + + const { count, results, page_size } = userStore.getSearchResult(); const columns = this.getTableColumns(); const handleClear = () => @@ -209,7 +213,7 @@ class Users extends React.Component { rowClassName={getUserRowClassNameFn(userPkToEdit, userStore.currentUserPk)} pagination={{ page, - total: Math.ceil((count || 0) / ITEMS_PER_PAGE), + total: results ? Math.ceil((count || 0) / page_size) : 0, onChange: this.handleChangePage, }} /> @@ -417,11 +421,19 @@ class Users extends React.Component { } handleChangePage = (page: number) => { - this.setState({ page }, this.updateUsers); + const { filtersStore } = this.props.store; + + filtersStore.currentTablePageNum[PAGE.Users] = page; + + this.updateUsers(); }; handleUsersFiltersChange = (usersFilters: any, invalidateFn: () => boolean) => { - this.setState({ usersFilters, page: 1 }, () => { + const { filtersStore } = this.props.store; + + filtersStore.currentTablePageNum[PAGE.Users] = 1; + + this.setState({ usersFilters }, () => { this.updateUsers(invalidateFn); }); }; diff --git a/grafana-plugin/src/state/rootBaseStore/index.ts b/grafana-plugin/src/state/rootBaseStore/index.ts index c1638da9..6a771e25 100644 --- a/grafana-plugin/src/state/rootBaseStore/index.ts +++ b/grafana-plugin/src/state/rootBaseStore/index.ts @@ -37,7 +37,6 @@ import { CLOUD_VERSION_REGEX, GRAFANA_LICENSE_CLOUD, GRAFANA_LICENSE_OSS, - PAGE, PLUGIN_ROOT, } from 'utils/consts'; import FaroHelper from 'utils/faro'; @@ -80,9 +79,6 @@ export class RootBaseStore { @observable incidentsPage: any = this.initialQuery.p ? Number(this.initialQuery.p) : 1; - @observable - currentPage: { [key: string]: number } = {}; - @observable onCallApiUrl: string; @@ -312,13 +308,4 @@ export class RootBaseStore { const settings = await PluginState.getGrafanaPluginSettings(); return settings.jsonData?.onCallApiUrl; } - - getCurrentPage = (page: PAGE): number => { - return this.currentPage[page]; - }; - - @action - setCurrentPage = (page: PAGE, value: number) => { - this.currentPage[page] = value; - }; } diff --git a/grafana-plugin/src/utils/consts.ts b/grafana-plugin/src/utils/consts.ts index b045079c..6195082d 100644 --- a/grafana-plugin/src/utils/consts.ts +++ b/grafana-plugin/src/utils/consts.ts @@ -50,6 +50,7 @@ export enum PAGE { Incidents = 'incidents', Webhooks = 'webhooks', Schedules = 'schedules', + Users = 'users', } export const TEXT_ELLIPSIS_CLASS = 'overflow-child'; From d1de4bc54be76c9789aab961233404c03f3b2a7c Mon Sep 17 00:00:00 2001 From: Rares Mardare Date: Mon, 30 Oct 2023 18:12:32 +0200 Subject: [PATCH 2/6] changelog --- CHANGELOG.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b38818f9..c3a010f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Changed + +- Removed the hardcoding of page size on frontend ([#3205](https://github.com/grafana/oncall/pull/3205)) +- Prevent additional polling on Incidents if the previous request didn't complete + ([#3205](https://github.com/grafana/oncall/pull/3205)) + ## v1.3.48 (2023-10-30) ### Added @@ -19,9 +25,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Simplify Direct Paging workflow. Now when using Direct Paging you either simply specify a team, or one or more users to page by @joeyorlando ([#3128](https://github.com/grafana/oncall/pull/3128)) - Enable timing options for mobile push notifications, allow multi-select by @Ferril ([#3187](https://github.com/grafana/oncall/pull/3187)) -- Removed the hardcoding of page size on frontend ([#3205](https://github.com/grafana/oncall/pull/#3205)) -- Prevent additional polling on Incidents if the previous request didn't complete - ([#3205](https://github.com/grafana/oncall/pull/#3205)) ### Fixed From 5996141133fc4622625dac092dcf95af31d212e9 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Tue, 31 Oct 2023 12:08:07 -0300 Subject: [PATCH 3/6] Disable updates for API/terraform shifts in internal API (#3224) Related to https://github.com/grafana/oncall-private/issues/2246 --- CHANGELOG.md | 4 +++ engine/apps/api/serializers/on_call_shifts.py | 3 ++ engine/apps/api/tests/test_oncall_shift.py | 36 +++++++++++++++++++ 3 files changed, 43 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3a010f0..f035ec91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Prevent additional polling on Incidents if the previous request didn't complete ([#3205](https://github.com/grafana/oncall/pull/3205)) +### Fixed + +- Do not allow to update terraform-based shifts in web UI schedule API ([#3224](https://github.com/grafana/oncall/pull/3224)) + ## v1.3.48 (2023-10-30) ### Added diff --git a/engine/apps/api/serializers/on_call_shifts.py b/engine/apps/api/serializers/on_call_shifts.py index def64076..f2178e28 100644 --- a/engine/apps/api/serializers/on_call_shifts.py +++ b/engine/apps/api/serializers/on_call_shifts.py @@ -228,6 +228,9 @@ class OnCallShiftUpdateSerializer(OnCallShiftSerializer): read_only_fields = ["schedule", "type"] def update(self, instance, validated_data): + if not instance.schedule: + # only web-based schedule events can be updated using UI + raise serializers.ValidationError(["This event cannot be updated"]) validated_data = self._correct_validated_data(instance.type, validated_data) change_only_name = True create_or_update_last_shift = False diff --git a/engine/apps/api/tests/test_oncall_shift.py b/engine/apps/api/tests/test_oncall_shift.py index af8191fe..3e471009 100644 --- a/engine/apps/api/tests/test_oncall_shift.py +++ b/engine/apps/api/tests/test_oncall_shift.py @@ -400,6 +400,42 @@ def test_list_on_call_shift_filter_schedule_id( assert response.json() == expected_payload +@pytest.mark.django_db +def test_update_calendar_shift_is_disabled( + on_call_shift_internal_api_setup, + make_schedule, + make_on_call_shift, + make_user_auth_headers, +): + token, user1, user2, organization, _ = on_call_shift_internal_api_setup + schedule = make_schedule(organization, schedule_class=OnCallScheduleCalendar) + + client = APIClient() + start_date = timezone.now().replace(microsecond=0) + + name = "Test Shift Rotation" + on_call_shift = make_on_call_shift( + schedule.organization, + shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, + name=name, + start=start_date, + duration=timezone.timedelta(hours=1), + rotation_start=start_date, + rolling_users=[{user1.pk: user1.public_primary_key}, {user2.pk: user2.public_primary_key}], + ) + on_call_shift.schedules.add(schedule) + + client = APIClient() + + data_to_update = { + "name": name, + } + url = reverse("api-internal:oncall_shifts-detail", kwargs={"pk": on_call_shift.public_primary_key}) + + response = client.put(url, data=data_to_update, format="json", **make_user_auth_headers(user1, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @pytest.mark.django_db def test_update_future_on_call_shift( on_call_shift_internal_api_setup, From d568ad6707faac245ebeb5bed51f36d0a39f5ce7 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 31 Oct 2023 11:18:33 -0400 Subject: [PATCH 4/6] few add responders patches (#3220) # Which issue(s) this PR fixes Closes https://github.com/grafana/support-escalations/issues/8143 Fix a few minor issues introduced in #3128: - Fix slow `GET /users` internal API endpoint related to [this change](https://github.com/grafana/oncall/blob/dev/engine/apps/api/views/user.py#L239) - Fix slow `GET /teams` internal API endpoint. Introduced a `short` query parameter that only invokes `apps.schedules.ical_utils.get_oncall_users_for_multiple_schedules` when `short=false`. - Order results from `GET /teams` internal API endpoint by name (ascending) - Fix search issue when searching for teams in the add responders popup window (this was strictly a frontend issue) - CSS changes to add responders dropdown to fix lonnnggg results list: **Before** Screenshot 2023-10-31 at 10 06 20 **After** Screenshot 2023-10-31 at 10 48 12 ## Still todo The `apps.schedules.ical_utils.get_oncall_users_for_multiple_schedules` method is still very slow when an instance has a lot of users (ex. `ops`). Ideally we should refactor this method to be more efficient because we still need to call this method under some circumstances. Ex. to populate this dropdown when Direct Paging a user (note that it didn't finish loading here on `ops`): Screenshot 2023-10-30 at 18 14 59 ## 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) --- CHANGELOG.md | 4 +++ engine/apps/api/serializers/team.py | 19 ++++++++---- engine/apps/api/tests/test_team.py | 29 +++++++++++++++---- engine/apps/api/views/team.py | 14 +++++++-- engine/apps/api/views/user.py | 27 +++++++++++++---- .../AddRespondersPopup.module.scss | 1 + .../AddRespondersPopup/AddRespondersPopup.tsx | 2 +- .../src/models/grafana_team/grafana_team.ts | 28 ++++++++---------- .../models/grafana_team/grafana_team.types.ts | 2 +- grafana-plugin/src/models/user/user.types.ts | 4 +-- 10 files changed, 90 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f035ec91..52c8e2f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,9 +12,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Removed the hardcoding of page size on frontend ([#3205](https://github.com/grafana/oncall/pull/3205)) - Prevent additional polling on Incidents if the previous request didn't complete ([#3205](https://github.com/grafana/oncall/pull/3205)) +- Order results from `GET /teams` internal API endpoint by ascending name by @joeyorlando ([#3220](https://github.com/grafana/oncall/pull/3220)) ### Fixed +- Improve slow `GET /users` + `GET /teams` internal API endpoints by @joeyorlando ([#3220](https://github.com/grafana/oncall/pull/3220)) +- Fix search issue when searching for teams in the add responders popup window by @joeyorlando ([#3220](https://github.com/grafana/oncall/pull/3220)) +- CSS changes to add responders dropdown to fix long search results list by @joeyorlando ([#3220](https://github.com/grafana/oncall/pull/3220)) - Do not allow to update terraform-based shifts in web UI schedule API ([#3224](https://github.com/grafana/oncall/pull/3224)) ## v1.3.48 (2023-10-30) diff --git a/engine/apps/api/serializers/team.py b/engine/apps/api/serializers/team.py index dd7e172d..b7263bcd 100644 --- a/engine/apps/api/serializers/team.py +++ b/engine/apps/api/serializers/team.py @@ -19,21 +19,17 @@ class FastTeamSerializer(serializers.ModelSerializer): class TeamSerializer(serializers.ModelSerializer): - context: TeamSerializerContext - id = serializers.CharField(read_only=True, source="public_primary_key") - number_of_users_currently_oncall = serializers.SerializerMethodField() class Meta: model = Team - fields = ( + fields = [ "id", "name", "email", "avatar_url", "is_sharing_resources_to_all", - "number_of_users_currently_oncall", - ) + ] read_only_fields = [ "id", @@ -42,6 +38,17 @@ class TeamSerializer(serializers.ModelSerializer): "avatar_url", ] + +class TeamLongSerializer(TeamSerializer): + context: TeamSerializerContext + + number_of_users_currently_oncall = serializers.SerializerMethodField() + + class Meta(TeamSerializer.Meta): + fields = TeamSerializer.Meta.fields + [ + "number_of_users_currently_oncall", + ] + def get_number_of_users_currently_oncall(self, obj: Team) -> int: num_of_users_oncall_for_team = 0 diff --git a/engine/apps/api/tests/test_team.py b/engine/apps/api/tests/test_team.py index a5a93a9a..69a8afbe 100644 --- a/engine/apps/api/tests/test_team.py +++ b/engine/apps/api/tests/test_team.py @@ -14,16 +14,19 @@ from apps.user_management.models import Team GENERAL_TEAM = Team(public_primary_key="null", name="No team", email=None, avatar_url=None) -def get_payload_from_team(team): - return { +def get_payload_from_team(team, long=False): + payload = { "id": team.public_primary_key, "name": team.name, "email": team.email, "avatar_url": team.avatar_url, "is_sharing_resources_to_all": team.is_sharing_resources_to_all, - "number_of_users_currently_oncall": 0, } + if long: + payload.update({"number_of_users_currently_oncall": 0}) + return payload + @pytest.mark.django_db def test_list_teams( @@ -40,22 +43,36 @@ def test_list_teams( team = make_team(organization) team.users.add(user) + auth_headers = make_user_auth_headers(user, token) + general_team_payload = get_payload_from_team(GENERAL_TEAM) + general_team_long_payload = get_payload_from_team(GENERAL_TEAM, long=True) team_payload = get_payload_from_team(team) + team_long_payload = get_payload_from_team(team, long=True) client = APIClient() url = reverse("api-internal:team-list") - response = client.get(url, format="json", **make_user_auth_headers(user, token)) + response = client.get(url, format="json", **auth_headers) assert response.status_code == status.HTTP_200_OK assert response.json() == [general_team_payload, team_payload] + response = client.get(f"{url}?short=false", format="json", **auth_headers) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == [general_team_long_payload, team_long_payload] + url = reverse("api-internal:team-list") - response = client.get(f"{url}?include_no_team=false", format="json", **make_user_auth_headers(user, token)) + response = client.get(f"{url}?include_no_team=false", format="json", **auth_headers) assert response.status_code == status.HTTP_200_OK assert response.json() == [team_payload] + response = client.get(f"{url}?include_no_team=false&short=false", format="json", **auth_headers) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == [team_long_payload] + @pytest.mark.django_db def test_list_teams_only_include_notifiable_teams( @@ -146,7 +163,7 @@ def test_teams_number_of_users_currently_oncall_attribute_works_properly( _make_schedule(team=team3, oncall_users=[]) client = APIClient() - url = reverse("api-internal:team-list") + url = f"{reverse('api-internal:team-list')}?short=false" response = client.get(url, format="json", **make_user_auth_headers(user1, token)) diff --git a/engine/apps/api/views/team.py b/engine/apps/api/views/team.py index 4e6dd481..47c10e11 100644 --- a/engine/apps/api/views/team.py +++ b/engine/apps/api/views/team.py @@ -6,7 +6,7 @@ from rest_framework.response import Response from apps.alerts.paging import integration_is_notifiable from apps.api.permissions import RBACPermission -from apps.api.serializers.team import TeamSerializer +from apps.api.serializers.team import TeamLongSerializer, TeamSerializer from apps.auth_token.auth import PluginAuthentication from apps.mobile_app.auth import MobileAppAuthTokenAuthentication from apps.schedules.ical_utils import get_oncall_users_for_multiple_schedules @@ -33,6 +33,9 @@ class TeamViewSet(PublicPrimaryKeyMixin, mixins.ListModelMixin, mixins.UpdateMod def get_queryset(self): return self.request.user.available_teams + def _is_long_request(self) -> bool: + return self.request.query_params.get("short", "true").lower() == "false" + @cached_property def schedules_with_oncall_users(self): """ @@ -45,9 +48,14 @@ class TeamViewSet(PublicPrimaryKeyMixin, mixins.ListModelMixin, mixins.UpdateMod def get_serializer_context(self): context = super().get_serializer_context() - context.update({"schedules_with_oncall_users": self.schedules_with_oncall_users}) + context.update( + {"schedules_with_oncall_users": self.schedules_with_oncall_users if self._is_long_request() else {}} + ) return context + def get_serializer_class(self): + return TeamLongSerializer if self._is_long_request() else TeamSerializer + def list(self, request, *args, **kwargs): general_team = [Team(public_primary_key="null", name="No team", email=None, avatar_url=None)] queryset = self.filter_queryset(self.get_queryset()) @@ -62,6 +70,8 @@ class TeamViewSet(PublicPrimaryKeyMixin, mixins.ListModelMixin, mixins.UpdateMod queryset = queryset.filter(pk__in=team_ids) + queryset = queryset.order_by("name") + teams = list(queryset) if self.request.query_params.get("include_no_team", "true") != "false": # Adds general team to the queryset in a way that it always shows up first (even when not searched for). diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index b6df3df3..c6a51fc7 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -234,9 +234,27 @@ class UserView( """ return get_oncall_users_for_multiple_schedules(self.request.user.organization.oncall_schedules.all()) + def _get_is_currently_oncall_query_param(self) -> str: + return self.request.query_params.get("is_currently_oncall", "").lower() + + def _is_currently_oncall_request(self) -> bool: + return self._get_is_currently_oncall_query_param() in ["true", "false"] + + def _is_long_request(self) -> bool: + return self.request.query_params.get("short", "true").lower() == "false" + + def _is_currently_oncall_or_long_request(self) -> bool: + return self._is_currently_oncall_request() or self._is_long_request() + def get_serializer_context(self): context = super().get_serializer_context() - context.update({"schedules_with_oncall_users": self.schedules_with_oncall_users}) + context.update( + { + "schedules_with_oncall_users": self.schedules_with_oncall_users + if self._is_currently_oncall_or_long_request() + else {} + } + ) return context def get_serializer_class(self): @@ -247,12 +265,10 @@ class UserView( is_list_request = self.action in ["list"] is_filters_request = query_params.get("filters", "false") == "true" - is_short_request = query_params.get("short", "true") == "false" - is_currently_oncall_request = query_params.get("is_currently_oncall", "").lower() in ["true", "false"] if is_list_request and is_filters_request: return self.get_filter_serializer_class() - elif is_list_request and (is_short_request or is_currently_oncall_request): + elif is_list_request and self._is_currently_oncall_or_long_request(): return UserLongSerializer is_users_own_data = kwargs.get("pk") is not None and kwargs.get("pk") == user.public_primary_key @@ -277,11 +293,10 @@ class UserView( def list(self, request, *args, **kwargs) -> Response: queryset = self.filter_queryset(self.get_queryset()) - is_currently_oncall_query_param = request.query_params.get("is_currently_oncall", "").lower() - def _get_oncall_user_ids(): return {user.pk for _, users in self.schedules_with_oncall_users.items() for user in users} + is_currently_oncall_query_param = self._get_is_currently_oncall_query_param() if is_currently_oncall_query_param == "true": # client explicitly wants to filter out users that are on-call queryset = queryset.filter(pk__in=_get_oncall_user_ids()) diff --git a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss index 2c2f92f5..fbbd2163 100644 --- a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss +++ b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss @@ -31,6 +31,7 @@ } .table { + max-height: 150px; overflow: auto; padding: 4px 0px; diff --git a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx index d36c3532..3aa328dd 100644 --- a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx +++ b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx @@ -115,7 +115,7 @@ const AddRespondersPopup = observer( const handleSearchTermChange = useDebouncedCallback(() => { if (isCreateMode && activeOption === TabOptions.Teams) { - grafanaTeamStore.updateItems(searchTerm, false, true); + grafanaTeamStore.updateItems(searchTerm, false, true, false); } else { userStore.updateItems({ searchTerm, short: 'false' }); } diff --git a/grafana-plugin/src/models/grafana_team/grafana_team.ts b/grafana-plugin/src/models/grafana_team/grafana_team.ts index bb0b3cee..023150be 100644 --- a/grafana-plugin/src/models/grafana_team/grafana_team.ts +++ b/grafana-plugin/src/models/grafana_team/grafana_team.ts @@ -5,12 +5,14 @@ import { GrafanaTeam } from 'models/grafana_team/grafana_team.types'; import { makeRequest } from 'network'; import { RootStore } from 'state'; +type TeamItems = { [id: string]: GrafanaTeam }; + export class GrafanaTeamStore extends BaseStore { @observable - searchResult: { [key: string]: Array } = {}; + searchResult: Array = []; @observable.shallow - items: { [id: string]: GrafanaTeam } = {}; + items: TeamItems = {}; constructor(rootStore: RootStore) { super(rootStore); @@ -29,10 +31,11 @@ export class GrafanaTeamStore extends BaseStore { } @action - async updateItems(query = '', includeNoTeam = true, onlyIncludeNotifiableTeams = false) { - const result = await makeRequest(`${this.path}`, { + async updateItems(query = '', includeNoTeam = true, onlyIncludeNotifiableTeams = false, short = true) { + const result = await makeRequest(`${this.path}`, { params: { search: query, + short: short ? 'true' : 'false', include_no_team: includeNoTeam ? 'true' : 'false', only_include_notifiable_teams: onlyIncludeNotifiableTeams ? 'true' : 'false', }, @@ -40,8 +43,8 @@ export class GrafanaTeamStore extends BaseStore { this.items = { ...this.items, - ...result.reduce( - (acc: { [key: number]: GrafanaTeam }, item: GrafanaTeam) => ({ + ...result.reduce( + (acc, item) => ({ ...acc, [item.id]: item, }), @@ -49,17 +52,10 @@ export class GrafanaTeamStore extends BaseStore { ), }; - this.searchResult = { - ...this.searchResult, - [query]: result.map((item: GrafanaTeam) => item.id), - }; + this.searchResult = result.map((item: GrafanaTeam) => item.id); } - getSearchResult(query = '') { - if (!this.searchResult[query]) { - return []; - } - - return this.searchResult[query].map((teamId: GrafanaTeam['id']) => this.items[teamId]); + getSearchResult() { + return this.searchResult.map((teamId: GrafanaTeam['id']) => this.items[teamId]); } } diff --git a/grafana-plugin/src/models/grafana_team/grafana_team.types.ts b/grafana-plugin/src/models/grafana_team/grafana_team.types.ts index 8b0af307..97210b8b 100644 --- a/grafana-plugin/src/models/grafana_team/grafana_team.types.ts +++ b/grafana-plugin/src/models/grafana_team/grafana_team.types.ts @@ -4,5 +4,5 @@ export interface GrafanaTeam { email: string; avatar_url: string; is_sharing_resources_to_all: boolean; - number_of_users_currently_oncall: number; + number_of_users_currently_oncall?: number; } diff --git a/grafana-plugin/src/models/user/user.types.ts b/grafana-plugin/src/models/user/user.types.ts index a1bad61b..11d6247b 100644 --- a/grafana-plugin/src/models/user/user.types.ts +++ b/grafana-plugin/src/models/user/user.types.ts @@ -43,6 +43,6 @@ export interface User { hidden_fields?: boolean; timezone: Timezone; working_hours: { [key: string]: [] }; - is_currently_oncall: boolean; - teams: GrafanaTeam[]; + is_currently_oncall?: boolean; + teams?: GrafanaTeam[]; } From 4efe1a42c9bf867d7cfddf2e413bb7d885d4af1e Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 31 Oct 2023 11:19:46 -0400 Subject: [PATCH 5/6] remove is_restricted from frontend codebase (#3219) # What this PR does The concept of `AlertGroup.is_restricted` is deprecated and no longer used. In a future release I will remove all references to `is_restricted` from the backend (doing it in separate releases to avoid any potential issues where the frontend still references it shortly after release of it being removed from the API response) ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated (N/A) - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- .../src/models/alertgroup/alertgroup.types.ts | 1 - .../src/pages/incident/Incident.helpers.tsx | 12 ++-- .../src/pages/incident/Incident.tsx | 62 ++++--------------- 3 files changed, 18 insertions(+), 57 deletions(-) diff --git a/grafana-plugin/src/models/alertgroup/alertgroup.types.ts b/grafana-plugin/src/models/alertgroup/alertgroup.types.ts index 48369f4e..5514042e 100644 --- a/grafana-plugin/src/models/alertgroup/alertgroup.types.ts +++ b/grafana-plugin/src/models/alertgroup/alertgroup.types.ts @@ -55,7 +55,6 @@ export interface Alert { acknowledged_at: string; acknowledged_by_user: User; acknowledged_on_source: boolean; - is_restricted: boolean; channel: Channel; slack_permalink?: string; permalinks: { diff --git a/grafana-plugin/src/pages/incident/Incident.helpers.tsx b/grafana-plugin/src/pages/incident/Incident.helpers.tsx index 39b5c162..938b4841 100644 --- a/grafana-plugin/src/pages/incident/Incident.helpers.tsx +++ b/grafana-plugin/src/pages/incident/Incident.helpers.tsx @@ -153,7 +153,7 @@ export function getActionButtons(incident: AlertType, cx: any, callbacks: { [key const resolveButton = ( - @@ -161,7 +161,7 @@ export function getActionButtons(incident: AlertType, cx: any, callbacks: { [key const unacknowledgeButton = ( - @@ -169,7 +169,7 @@ export function getActionButtons(incident: AlertType, cx: any, callbacks: { [key const unresolveButton = ( - @@ -177,7 +177,7 @@ export function getActionButtons(incident: AlertType, cx: any, callbacks: { [key const acknowledgeButton = ( - @@ -188,7 +188,7 @@ export function getActionButtons(incident: AlertType, cx: any, callbacks: { [key if (incident.status === IncidentStatus.Silenced) { buttons.push( - @@ -198,7 +198,7 @@ export function getActionButtons(incident: AlertType, cx: any, callbacks: { [key ); diff --git a/grafana-plugin/src/pages/incident/Incident.tsx b/grafana-plugin/src/pages/incident/Incident.tsx index 98aca39f..42d2fc6d 100644 --- a/grafana-plugin/src/pages/incident/Incident.tsx +++ b/grafana-plugin/src/pages/incident/Incident.tsx @@ -171,7 +171,6 @@ class IncidentPage extends React.Component @@ -289,12 +288,7 @@ class IncidentPage extends React.Component {incident.root_alert_group.render_for_web.title} {' '} - @@ -310,16 +304,10 @@ class IncidentPage extends React.Component onClick={this.showAttachIncidentForm} tooltip="Attach to another Alert Group" className={cx('title-icon')} - disabled={incident.is_restricted} /> )} - + openNotification('Link copied'); }} > - + @@ -358,7 +341,7 @@ class IncidentPage extends React.Component query={{ page: 'integrations', id: incident.alert_receive_channel.id }} > @@ -427,7 +410,7 @@ class IncidentPage extends React.Component