Schedule Filters tweaks (#1406)

# What this PR does Improvements over schedule filters ## Which issue(s) this PR fixes #941 ## Checklist - [ ] Tests updated - [ ] Documentation added - [x] `CHANGELOG.md` updated
grafana · Mar 1, 2023 · 1b0643f · 1b0643f
1 parent 4c31ede
commit 1b0643f
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 85 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Fixed
+
+- Schedule filters improvements ([941](https://github.com/grafana/oncall/issues/941))
+
 ## v1.1.31 (2023-03-01)
 
 ### Added

diff --git a/grafana-plugin/src/components/SchedulesFilters/SchedulesFilters.tsx b/grafana-plugin/src/components/SchedulesFilters/SchedulesFilters.tsx
@@ -25,8 +25,8 @@ const SchedulesFilters = (props: SchedulesFiltersProps) => {
     [value]
   );
   const handleStatusChange = useCallback(
-    (status) => {
-      onChange({ ...value, status });
+    (used) => {
+      onChange({ ...value, used });
     },
     [value]
   );
@@ -56,14 +56,14 @@ const SchedulesFilters = (props: SchedulesFiltersProps) => {
         <Field label="Status">
           <RadioButtonGroup
             options={[
-              { label: 'All', value: 'all' },
+              { label: 'All', value: undefined },
               {
                 label: 'Used in escalations',
-                value: 'used',
+                value: true,
               },
-              { label: 'Unused', value: 'unused' },
+              { label: 'Unused', value: false },
             ]}
-            value={value.status}
+            value={value.used}
             onChange={handleStatusChange}
           />
         </Field>

diff --git a/grafana-plugin/src/components/SchedulesFilters/SchedulesFilters.types.ts b/grafana-plugin/src/components/SchedulesFilters/SchedulesFilters.types.ts
@@ -3,5 +3,5 @@ import { ScheduleType } from 'models/schedule/schedule.types';
 export interface SchedulesFiltersType {
   searchTerm: string;
   type: ScheduleType;
-  status: string;
+  used: boolean | undefined;
 }
diff --git a/grafana-plugin/src/models/schedule/schedule.ts b/grafana-plugin/src/models/schedule/schedule.ts
@@ -1,6 +1,7 @@
 import dayjs from 'dayjs';
 import { action, observable } from 'mobx';
 
+import { SchedulesFiltersType } from 'components/SchedulesFilters/SchedulesFilters.types';
 import BaseStore from 'models/base_store';
 import { EscalationChain } from 'models/escalation_chain/escalation_chain.types';
 import { makeRequest } from 'network';
@@ -118,10 +119,21 @@ export class ScheduleStore extends BaseStore {
   }
 
   @action
-  async updateItems(f: any = { searchTerm: '', type: undefined }, page = 1) {
-    const filters = typeof f === 'string' ? { searchTerm: f } : f;
-    const { searchTerm: search, type } = filters;
-    const { count, results } = await makeRequest(this.path, { method: 'GET', params: { search: search, type, page } });
+  async updateItems(
+    f: SchedulesFiltersType | string = { searchTerm: '', type: undefined, used: undefined },
+    page = 1,
+    shouldUpdateFn: () => boolean = undefined
+  ) {
+    const filters: Partial<SchedulesFiltersType> = typeof f === 'string' ? { searchTerm: f } : f;
+    const { searchTerm: search, type, used } = filters;
+    const { count, results } = await makeRequest(this.path, {
+      method: 'GET',
+      params: { search: search, type, used, page },
+    });
+
+    if (shouldUpdateFn && !shouldUpdateFn()) {
+      return;
+    }
 
     this.items = {
       ...this.items,

diff --git a/grafana-plugin/src/models/user/user.ts b/grafana-plugin/src/models/user/user.ts
@@ -106,30 +106,34 @@ export class UserStore extends BaseStore {
 
   @action
   async updateItems(f: any = { searchTerm: '' }, page = 1) {
-    const filters = typeof f === 'string' ? { searchTerm: f } : f; // for GSelect compatibility
-    const { searchTerm: search } = filters;
-    const { count, results } = await makeRequest(this.path, {
-      params: { search, page },
-    });
-
-    this.items = {
-      ...this.items,
-      ...results.reduce(
-        (acc: { [key: number]: User }, item: User) => ({
-          ...acc,
-          [item.pk]: {
-            ...item,
-            timezone: getTimezone(item),
-          },
-        }),
-        {}
-      ),
-    };
+    return new Promise<void>(async (resolve) => {
+      const filters = typeof f === 'string' ? { searchTerm: f } : f; // for GSelect compatibility
+      const { searchTerm: search } = filters;
+      const { count, results } = await makeRequest(this.path, {
+        params: { search, page },
+      });
 
-    this.searchResult = {
-      count,
-      results: results.map((item: User) => item.pk),
-    };
+      this.items = {
+        ...this.items,
+        ...results.reduce(
+          (acc: { [key: number]: User }, item: User) => ({
+            ...acc,
+            [item.pk]: {
+              ...item,
+              timezone: getTimezone(item),
+            },
+          }),
+          {}
+        ),
+      };
+
+      this.searchResult = {
+        count,
+        results: results.map((item: User) => item.pk),
+      };
+
+      resolve();
+    });
   }
 
   getSearchResult() {

diff --git a/grafana-plugin/src/pages/schedules/Schedules.tsx b/grafana-plugin/src/pages/schedules/Schedules.tsx
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { SyntheticEvent } from 'react';
 
 import { Button, HorizontalGroup, IconButton, LoadingPlaceholder, VerticalGroup } from '@grafana/ui';
 import cn from 'classnames/bind';
@@ -36,6 +36,7 @@ import { PLUGIN_ROOT, TABLE_COLUMN_MAX_WIDTH } from 'utils/consts';
 import styles from './Schedules.module.css';
 
 const cx = cn.bind(styles);
+const FILTERS_DEBOUNCE_MS = 500;
 const ITEMS_PER_PAGE = 10;
 
 interface SchedulesPageProps extends WithStoreProps, RouteComponentProps, PageProps {}
@@ -55,9 +56,10 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
     super(props);
 
     const { store } = this.props;
+
     this.state = {
       startMoment: getStartOfWeek(store.currentTimezone),
-      filters: { searchTerm: '', status: 'all', type: undefined },
+      filters: { searchTerm: '', type: undefined, used: undefined },
       showNewScheduleSelector: false,
       expandedRowKeys: [],
       scheduleIdToEdit: undefined,
@@ -71,7 +73,10 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
       query: { p },
     } = this.props;
 
-    store.userStore.updateItems();
+    const { filters, page } = this.state;
+
+    await store.scheduleStore.updateItems(filters, page, () => filters === this.state.filters);
+
     this.setState({ page: p ? Number(p) : 1 }, this.updateSchedules);
   }
 
@@ -80,16 +85,15 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
     const { filters, page } = this.state;
 
     LocationHelper.update({ p: page }, 'partial');
+
     await store.scheduleStore.updateItems(filters, page);
   };
 
   render() {
     const { store } = this.props;
     const { filters, showNewScheduleSelector, expandedRowKeys, scheduleIdToEdit, page } = this.state;
 
-    const { scheduleStore } = store;
-
-    const { count, results } = scheduleStore.getSearchResult();
+    const { results, count } = store.scheduleStore.getSearchResult();
 
     const columns = [
       {
@@ -141,15 +145,6 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
 
     const users = store.userStore.getSearchResult().results;
 
-    const data = results
-      ? results.filter(
-          (schedule) =>
-            filters.status === 'all' ||
-            (filters.status === 'used' && schedule.number_of_escalation_chains) ||
-            (filters.status === 'unused' && !schedule.number_of_escalation_chains)
-        )
-      : undefined;
-
     return (
       <>
         <div className={cx('root')}>
@@ -173,7 +168,8 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
             </div>
             <Table
               columns={columns}
-              data={data}
+              data={results}
+              loading={!results}
               pagination={{ page, total: Math.ceil((count || 0) / ITEMS_PER_PAGE), onChange: this.handlePageChange }}
               rowKey="id"
               expandable={{
@@ -182,11 +178,7 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
                 expandedRowRender: this.renderSchedule,
                 expandRowByClick: true,
               }}
-              emptyText={
-                <div className={cx('loader')}>
-                  {data ? <Text type="secondary">Not found</Text> : <Text type="secondary">Loading schedules...</Text>}
-                </div>
-              }
+              emptyText={this.renderNotFound()}
             />
           </VerticalGroup>
         </div>
@@ -212,6 +204,14 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
     );
   }
 
+  renderNotFound() {
+    return (
+      <div className={cx('loader')}>
+        <Text type="secondary">Not found</Text>
+      </div>
+    );
+  }
+
   handleTimezoneChange = (value: Timezone) => {
     const { store } = this.props;
 
@@ -329,13 +329,6 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
             onHover={this.getUpdateRelatedEscalationChainsHandler(item.id)}
           />
         )}
-
-        {/* <ScheduleCounter
-          type="warning"
-          count={warningsCount}
-          tooltipTitle="Warnings"
-          tooltipContent="Schedule has unassigned time periods during next 7 days"
-        />*/}
       </HorizontalGroup>
     );
   };
@@ -380,23 +373,24 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
 
   renderButtons = (item: Schedule) => {
     return (
-      <HorizontalGroup>
-        <WithPermissionControl key="edit" userAction={UserActions.SchedulesWrite}>
-          <IconButton tooltip="Settings" name="cog" onClick={this.getEditScheduleClickHandler(item.id)} />
-        </WithPermissionControl>
-        <WithPermissionControl key="edit" userAction={UserActions.SchedulesWrite}>
-          <WithConfirm>
-            <IconButton tooltip="Delete" name="trash-alt" onClick={this.getDeleteScheduleClickHandler(item.id)} />
-          </WithConfirm>
-        </WithPermissionControl>
-      </HorizontalGroup>
+      /* Wrapper div for onClick event to prevent expanding schedule view on delete/edit click */
+      <div onClick={(event: SyntheticEvent) => event.stopPropagation()}>
+        <HorizontalGroup>
+          <WithPermissionControl key="edit" userAction={UserActions.SchedulesWrite}>
+            <IconButton tooltip="Settings" name="cog" onClick={this.getEditScheduleClickHandler(item.id)} />
+          </WithPermissionControl>
+          <WithPermissionControl key="edit" userAction={UserActions.SchedulesWrite}>
+            <WithConfirm>
+              <IconButton tooltip="Delete" name="trash-alt" onClick={this.getDeleteScheduleClickHandler(item.id)} />
+            </WithConfirm>
+          </WithPermissionControl>
+        </HorizontalGroup>
+      </div>
     );
   };
 
   getEditScheduleClickHandler = (id: Schedule['id']) => {
-    return (event) => {
-      event.stopPropagation();
-
+    return () => {
       this.setState({ scheduleIdToEdit: id });
     };
   };
@@ -406,33 +400,42 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
     const { scheduleStore } = store;
 
     return () => {
-      scheduleStore.delete(id).then(this.update);
+      scheduleStore.delete(id).then(() => this.update(true));
     };
   };
 
   handleSchedulesFiltersChange = (filters: SchedulesFiltersType) => {
-    this.setState({ filters }, this.debouncedUpdateSchedules);
+    this.setState({ filters }, () => this.debouncedUpdateSchedules(filters));
   };
 
-  applyFilters = () => {
-    const { filters } = this.state;
-    const { store } = this.props;
-    const { scheduleStore } = store;
-    scheduleStore.updateItems(filters);
+  applyFilters = (filters: SchedulesFiltersType) => {
+    const { scheduleStore } = this.props.store;
+    const shouldUpdateFn = () => this.state.filters === filters;
+    scheduleStore.updateItems(filters, 1, shouldUpdateFn).then(() => {
+      if (shouldUpdateFn) {
+        this.setState({ page: 1 });
+      }
+    });
   };
 
-  debouncedUpdateSchedules = debounce(this.applyFilters, 1000);
+  debouncedUpdateSchedules = debounce(this.applyFilters, FILTERS_DEBOUNCE_MS);
 
   handlePageChange = (page: number) => {
     this.setState({ page }, this.updateSchedules);
     this.setState({ expandedRowKeys: [] });
   };
 
-  update = () => {
+  update = (isRemoval = false) => {
     const { store } = this.props;
+    const { filters, page } = this.state;
     const { scheduleStore } = store;
 
-    return scheduleStore.updateItems();
+    // For removal we need to check if count is 1
+    // which means we should change the page to the previous one
+    const { results } = store.scheduleStore.getSearchResult();
+    const newPage = results.length === 1 ? Math.max(page - 1, 1) : page;
+
+    return scheduleStore.updateItems(filters, isRemoval ? newPage : page);
   };
 
   getUpdateRelatedEscalationChainsHandler = (scheduleId: Schedule['id']) => {