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)
grafana · Oct 30, 2023 · 871860c · 871860c
1 parent c78ab58
commit 871860c
Show file tree

Hide file tree

Showing 13 changed files with 286 additions and 252 deletions.
diff --git 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
@@ -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<RemoteFiltersProps, RemoteFiltersState> {
   }
 
   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<RemoteFiltersProps, RemoteFiltersState> {
     }
 
     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<RemoteFiltersProps, RemoteFiltersState> {
   debouncedOnChange = debounce(this.onChange, 500);
 }
 
-export default withMobXProviderContext(RemoteFilters);
+export default withMobXProviderContext(RemoteFilters) as unknown as React.ComponentClass<
+  Omit<RemoteFiltersProps, 'store'>
+>;
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
@@ -28,7 +28,7 @@ export class AlertReceiveChannelStore extends BaseStore {
   searchResult: Array<AlertReceiveChannel['id']>;
 
   @observable.shallow
-  paginatedSearchResult: { count?: number; results?: Array<AlertReceiveChannel['id']> } = {};
+  paginatedSearchResult: { count?: number; results?: Array<AlertReceiveChannel['id']>; 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
@@ -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<string, Alert>();
@@ -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
@@ -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
@@ -17,7 +17,7 @@ import { User } from './user.types';
 
 export class UserStore extends BaseStore {
   @observable.shallow
-  searchResult: { count?: number; results?: Array<User['pk']> } = {};
+  searchResult: { count?: number; results?: Array<User['pk']>; 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]),
     };