From 0e4206d4aca16978c5682333732babb2dc7f6f95 Mon Sep 17 00:00:00 2001
From: Rares Mardare <rares.mardare@grafana.com>
Date: Mon, 21 Aug 2023 17:22:10 +0300
Subject: [PATCH 1/5] prevent duplicate calls on /users, fixed initial search

---
 CHANGELOG.md                                  |  1 +
 .../components/UsersFilters/UsersFilters.tsx  |  5 +-
 grafana-plugin/src/pages/users/Users.tsx      | 59 +++++++++++--------
 3 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1b50fa49d..529101f8c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Changed HTTP Endpoint to Email for inbound email integrations
   ([#2816](https://github.com/grafana/oncall/issues/2816))
+- Fixed initial search on Users page ([#2842](https://github.com/grafana/oncall/issues/2842))
 
 ## v1.3.25 (2023-08-18)
 
diff --git a/grafana-plugin/src/components/UsersFilters/UsersFilters.tsx b/grafana-plugin/src/components/UsersFilters/UsersFilters.tsx
index e7834dcbc..a39c5194c 100644
--- a/grafana-plugin/src/components/UsersFilters/UsersFilters.tsx
+++ b/grafana-plugin/src/components/UsersFilters/UsersFilters.tsx
@@ -11,10 +11,11 @@ interface UsersFiltersProps {
   value: any;
   onChange: (filters: any) => void;
   className?: string;
+  isLoading?: boolean;
 }
 
 const UsersFilters = (props: UsersFiltersProps) => {
-  const { value = { searchTerm: '' }, onChange, className } = props;
+  const { value = { searchTerm: '' }, onChange, className, isLoading } = props;
 
   const onSearchTermChangeCallback = useCallback(
     (e: ChangeEvent<HTMLInputElement>) => {
@@ -31,6 +32,8 @@ const UsersFilters = (props: UsersFiltersProps) => {
   return (
     <div className={cx('root', className)}>
       <Input
+        loading={isLoading}
+        disabled={isLoading}
         prefix={<Icon name="search" />}
         className={cx('search', 'control')}
         placeholder="Search users..."
diff --git a/grafana-plugin/src/pages/users/Users.tsx b/grafana-plugin/src/pages/users/Users.tsx
index 69b417942..2bdf65151 100644
--- a/grafana-plugin/src/pages/users/Users.tsx
+++ b/grafana-plugin/src/pages/users/Users.tsx
@@ -47,28 +47,34 @@ interface UsersState extends PageBaseState {
     searchTerm: string;
   };
   initialUsersLoaded: boolean;
+  queuedUpdateUsers: boolean;
 }
 
 @observer
 class Users extends React.Component<UsersProps, UsersState> {
-  state: UsersState = {
-    page: 1,
-    isWrongTeam: false,
-    userPkToEdit: undefined,
-    usersFilters: {
-      searchTerm: '',
-    },
-
-    errorData: initErrorDataState(),
-    initialUsersLoaded: false,
-  };
+  constructor(props: UsersProps) {
+    super(props);
 
-  async componentDidMount() {
     const {
       query: { p },
-    } = this.props;
-    this.setState({ page: p ? Number(p) : 1 }, this.updateUsers);
+    } = props;
+
+    this.state = {
+      page: p ? Number(p) : 1,
+      isWrongTeam: false,
+      userPkToEdit: undefined,
+      usersFilters: {
+        searchTerm: '',
+      },
 
+      errorData: initErrorDataState(),
+      initialUsersLoaded: false,
+      queuedUpdateUsers: false,
+    };
+  }
+
+  async componentDidMount() {
+    this.updateUsers();
     this.parseParams();
   }
 
@@ -84,14 +90,14 @@ class Users extends React.Component<UsersProps, UsersState> {
     LocationHelper.update({ p: page }, 'partial');
     await userStore.updateItems(usersFilters, page);
 
-    this.setState({ initialUsersLoaded: true });
+    this.setState({ initialUsersLoaded: true }, () => {
+      if (this.state.queuedUpdateUsers) {
+        this.updateUsers();
+      }
+    });
   };
 
   componentDidUpdate(prevProps: UsersProps) {
-    if (!this.state.initialUsersLoaded) {
-      this.updateUsers();
-    }
-
     if (prevProps.match.params.id !== this.props.match.params.id) {
       this.parseParams();
     }
@@ -181,9 +187,12 @@ class Users extends React.Component<UsersProps, UsersState> {
     const { count, results } = userStore.getSearchResult();
     const columns = this.getTableColumns();
 
+    // disable search until we get all users retrieved so that there's no conflict on update
+    const isSearchDisabled = !initialUsersLoaded;
+
     const handleClear = () =>
       this.setState({ usersFilters: { searchTerm: '' } }, () => {
-        this.debouncedUpdateUsers();
+        this.updateUsers();
       });
 
     return (
@@ -195,6 +204,7 @@ class Users extends React.Component<UsersProps, UsersState> {
                 className={cx('users-filters')}
                 value={usersFilters}
                 onChange={this.handleUsersFiltersChange}
+                isLoading={isSearchDisabled}
               />
               <Button variant="secondary" icon="times" onClick={handleClear} className={cx('searchIntegrationClear')}>
                 Clear filters
@@ -418,13 +428,16 @@ class Users extends React.Component<UsersProps, UsersState> {
   }
 
   handleChangePage = (page: number) => {
-    this.setState({ page }, this.updateUsers);
+    this.setState({ page }, this.debouncedUpdateUsers);
   };
 
   debouncedUpdateUsers = debounce(this.updateUsers, 500);
 
   handleUsersFiltersChange = (usersFilters: any) => {
     this.setState({ usersFilters, page: 1 }, () => {
+      this.setState({ queuedUpdateUsers: !this.state.initialUsersLoaded });
+      if (!this.state.initialUsersLoaded) return;
+
       this.debouncedUpdateUsers();
     });
   };
@@ -435,10 +448,6 @@ class Users extends React.Component<UsersProps, UsersState> {
 
     history.push(`${PLUGIN_ROOT}/users`);
   };
-
-  handleUserUpdate = () => {
-    this.updateUsers();
-  };
 }
 
 export default withRouter(withMobXProviderContext(Users));

From 1e7d9e78bc5e5a4852e32deff374dde104800ea2 Mon Sep 17 00:00:00 2001
From: Rares Mardare <rares.mardare@grafana.com>
Date: Mon, 21 Aug 2023 18:14:28 +0300
Subject: [PATCH 2/5] better handling

---
 .../components/UsersFilters/UsersFilters.tsx  |  1 -
 grafana-plugin/src/pages/users/Users.tsx      | 20 ++++++++++---------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/grafana-plugin/src/components/UsersFilters/UsersFilters.tsx b/grafana-plugin/src/components/UsersFilters/UsersFilters.tsx
index a39c5194c..3773480ce 100644
--- a/grafana-plugin/src/components/UsersFilters/UsersFilters.tsx
+++ b/grafana-plugin/src/components/UsersFilters/UsersFilters.tsx
@@ -33,7 +33,6 @@ const UsersFilters = (props: UsersFiltersProps) => {
     <div className={cx('root', className)}>
       <Input
         loading={isLoading}
-        disabled={isLoading}
         prefix={<Icon name="search" />}
         className={cx('search', 'control')}
         placeholder="Search users..."
diff --git a/grafana-plugin/src/pages/users/Users.tsx b/grafana-plugin/src/pages/users/Users.tsx
index 2bdf65151..060a70467 100644
--- a/grafana-plugin/src/pages/users/Users.tsx
+++ b/grafana-plugin/src/pages/users/Users.tsx
@@ -90,8 +90,9 @@ class Users extends React.Component<UsersProps, UsersState> {
     LocationHelper.update({ p: page }, 'partial');
     await userStore.updateItems(usersFilters, page);
 
-    this.setState({ initialUsersLoaded: true }, () => {
-      if (this.state.queuedUpdateUsers) {
+    const { queuedUpdateUsers } = this.state;
+    this.setState({ initialUsersLoaded: true, queuedUpdateUsers: false }, () => {
+      if (queuedUpdateUsers) {
         this.updateUsers();
       }
     });
@@ -182,14 +183,13 @@ class Users extends React.Component<UsersProps, UsersState> {
     const {
       store: { userStore },
     } = this.props;
-    const { usersFilters, page, initialUsersLoaded, userPkToEdit } = this.state;
+
+    const { usersFilters, page, initialUsersLoaded, userPkToEdit, queuedUpdateUsers } = this.state;
+
 
     const { count, results } = userStore.getSearchResult();
     const columns = this.getTableColumns();
 
-    // disable search until we get all users retrieved so that there's no conflict on update
-    const isSearchDisabled = !initialUsersLoaded;
-
     const handleClear = () =>
       this.setState({ usersFilters: { searchTerm: '' } }, () => {
         this.updateUsers();
@@ -203,8 +203,8 @@ class Users extends React.Component<UsersProps, UsersState> {
               <UsersFilters
                 className={cx('users-filters')}
                 value={usersFilters}
+                isLoading={queuedUpdateUsers}
                 onChange={this.handleUsersFiltersChange}
-                isLoading={isSearchDisabled}
               />
               <Button variant="secondary" icon="times" onClick={handleClear} className={cx('searchIntegrationClear')}>
                 Clear filters
@@ -435,8 +435,10 @@ class Users extends React.Component<UsersProps, UsersState> {
 
   handleUsersFiltersChange = (usersFilters: any) => {
     this.setState({ usersFilters, page: 1 }, () => {
-      this.setState({ queuedUpdateUsers: !this.state.initialUsersLoaded });
-      if (!this.state.initialUsersLoaded) return;
+      if (!this.state.initialUsersLoaded) {
+        // queue delayed users update
+        return this.setState({ queuedUpdateUsers: true });
+      }
 
       this.debouncedUpdateUsers();
     });

From a2cda95f82816a0bda5d2415085b8cddaee32f1d Mon Sep 17 00:00:00 2001
From: Rares Mardare <rares.mardare@grafana.com>
Date: Mon, 21 Aug 2023 18:21:45 +0300
Subject: [PATCH 3/5] updateUsers instead of debounced

---
 grafana-plugin/src/pages/users/Users.tsx | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/grafana-plugin/src/pages/users/Users.tsx b/grafana-plugin/src/pages/users/Users.tsx
index 060a70467..7a2b388e9 100644
--- a/grafana-plugin/src/pages/users/Users.tsx
+++ b/grafana-plugin/src/pages/users/Users.tsx
@@ -186,7 +186,6 @@ class Users extends React.Component<UsersProps, UsersState> {
 
     const { usersFilters, page, initialUsersLoaded, userPkToEdit, queuedUpdateUsers } = this.state;
 
-
     const { count, results } = userStore.getSearchResult();
     const columns = this.getTableColumns();
 
@@ -428,7 +427,7 @@ class Users extends React.Component<UsersProps, UsersState> {
   }
 
   handleChangePage = (page: number) => {
-    this.setState({ page }, this.debouncedUpdateUsers);
+    this.setState({ page }, this.updateUsers);
   };
 
   debouncedUpdateUsers = debounce(this.updateUsers, 500);

From 347956c88d9d944e9a42655efb87eae0ff5fc575 Mon Sep 17 00:00:00 2001
From: Rares Mardare <rares.mardare@grafana.com>
Date: Tue, 22 Aug 2023 15:48:31 +0300
Subject: [PATCH 4/5] concurrency fix

---
 .../src/pages/schedule/Schedule.tsx           |  20 +--
 .../src/plugin/GrafanaPluginRootPage.tsx      | 160 +++++++++---------
 2 files changed, 85 insertions(+), 95 deletions(-)

diff --git a/grafana-plugin/src/pages/schedule/Schedule.tsx b/grafana-plugin/src/pages/schedule/Schedule.tsx
index b8c56e83c..876de5526 100644
--- a/grafana-plugin/src/pages/schedule/Schedule.tsx
+++ b/grafana-plugin/src/pages/schedule/Schedule.tsx
@@ -1,15 +1,6 @@
 import React from 'react';
 
-import {
-  Button,
-  HorizontalGroup,
-  VerticalGroup,
-  IconButton,
-  ToolbarButton,
-  Icon,
-  Modal,
-  LoadingPlaceholder,
-} from '@grafana/ui';
+import { Button, HorizontalGroup, VerticalGroup, IconButton, ToolbarButton, Icon, Modal } from '@grafana/ui';
 import cn from 'classnames/bind';
 import dayjs from 'dayjs';
 import { observer } from 'mobx-react';
@@ -48,9 +39,7 @@ import styles from './Schedule.module.css';
 
 const cx = cn.bind(styles);
 
-interface SchedulePageProps extends PageProps, WithStoreProps, RouteComponentProps<{ id: string }> {
-  basicDataLoaded: boolean;
-}
+interface SchedulePageProps extends PageProps, WithStoreProps, RouteComponentProps<{ id: string }> {}
 
 interface SchedulePageState extends PageBaseState {
   startMoment: dayjs.Dayjs;
@@ -123,7 +112,6 @@ class SchedulePage extends React.Component<SchedulePageProps, SchedulePageState>
       match: {
         params: { id: scheduleId },
       },
-      basicDataLoaded,
     } = this.props;
 
     const {
@@ -160,9 +148,7 @@ class SchedulePage extends React.Component<SchedulePageProps, SchedulePageState>
       !!shiftIdToShowOverridesForm ||
       shiftIdToShowRotationForm;
 
-    return !basicDataLoaded ? (
-      <LoadingPlaceholder text="Loading..." />
-    ) : (
+    return (
       <PageErrorHandlingWrapper errorData={errorData} objectName="schedule" pageName="schedules">
         {() => (
           <>
diff --git a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx
index 83c206cc5..59ede1846 100644
--- a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx
+++ b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx
@@ -1,5 +1,6 @@
 import React, { useEffect, useState } from 'react';
 
+import { LoadingPlaceholder } from '@grafana/ui';
 import classnames from 'classnames';
 import dayjs from 'dayjs';
 import customParseFormat from 'dayjs/plugin/customParseFormat';
@@ -76,6 +77,8 @@ export const Root = observer((props: AppRootProps) => {
     updateBasicData();
   }, []);
 
+  const location = useLocation();
+
   useEffect(() => {
     let link = document.createElement('link');
     link.type = 'text/css';
@@ -101,13 +104,9 @@ export const Root = observer((props: AppRootProps) => {
     setBasicDataLoaded(true);
   };
 
-  const location = useLocation();
-
   const page = getMatchedPage(location.pathname);
-
   const pagePermissionAction = pages[page]?.action;
   const userHasAccess = pagePermissionAction ? isUserActionAllowed(pagePermissionAction) : true;
-
   const query = getQueryParams();
 
   return (
@@ -126,80 +125,85 @@ export const Root = observer((props: AppRootProps) => {
         })}
       >
         {userHasAccess ? (
-          <Switch>
-            <Route path={getRoutesForPage('alert-groups')} exact>
-              <Incidents query={query} />
-            </Route>
-            <Route path={getRoutesForPage('alert-group')} exact>
-              <Incident query={query} />
-            </Route>
-            <Route path={getRoutesForPage('users')} exact>
-              <Users query={query} />
-            </Route>
-            <Route path={getRoutesForPage('integrations')} exact>
-              <Integrations query={query} />
-            </Route>
-            <Route path={getRoutesForPage('integration')} exact>
-              <Integration query={query} />
-            </Route>
-            <Route path={getRoutesForPage('escalations')} exact>
-              <EscalationChains query={query} />
-            </Route>
-            <Route path={getRoutesForPage('schedules')} exact>
-              <Schedules query={query} />
-            </Route>
-            <Route path={getRoutesForPage('schedule')} exact>
-              <Schedule query={query} basicDataLoaded={basicDataLoaded} />
-            </Route>
-            <Route path={getRoutesForPage('outgoing_webhooks')} exact>
-              <OutgoingWebhooks query={query} />
-            </Route>
-            <Route path={getRoutesForPage('maintenance')} exact>
-              <Maintenance />
-            </Route>
-            <Route path={getRoutesForPage('settings')} exact>
-              <SettingsPage />
-            </Route>
-            <Route path={getRoutesForPage('chat-ops')} exact>
-              <ChatOps query={query} />
-            </Route>
-            <Route path={getRoutesForPage('live-settings')} exact>
-              <LiveSettings />
-            </Route>
-            <Route path={getRoutesForPage('cloud')} exact>
-              <CloudPage />
-            </Route>
-
-            {/* Backwards compatibility redirect routes */}
-            <Route
-              path={getRoutesForPage('incident')}
-              exact
-              render={({ location }) => (
-                <Redirect
-                  to={{
-                    ...location,
-                    pathname: location.pathname.replace(/incident/, 'alert-group'),
-                  }}
-                ></Redirect>
-              )}
-            ></Route>
-            <Route
-              path={getRoutesForPage('incidents')}
-              exact
-              render={({ location }) => (
-                <Redirect
-                  to={{
-                    ...location,
-                    pathname: location.pathname.replace(/incidents/, 'alert-groups'),
-                  }}
-                ></Redirect>
-              )}
-            ></Route>
-
-            <Route path="*">
-              <NoMatch />
-            </Route>
-          </Switch>
+          // Otherwise we'll run into concurrency issues
+          !basicDataLoaded ? (
+            <LoadingPlaceholder text="Loading..." />
+          ) : (
+            <Switch>
+              <Route path={getRoutesForPage('alert-groups')} exact>
+                <Incidents query={query} />
+              </Route>
+              <Route path={getRoutesForPage('alert-group')} exact>
+                <Incident query={query} />
+              </Route>
+              <Route path={getRoutesForPage('users')} exact>
+                <Users query={query} />
+              </Route>
+              <Route path={getRoutesForPage('integrations')} exact>
+                <Integrations query={query} />
+              </Route>
+              <Route path={getRoutesForPage('integration')} exact>
+                <Integration query={query} />
+              </Route>
+              <Route path={getRoutesForPage('escalations')} exact>
+                <EscalationChains query={query} />
+              </Route>
+              <Route path={getRoutesForPage('schedules')} exact>
+                <Schedules query={query} />
+              </Route>
+              <Route path={getRoutesForPage('schedule')} exact>
+                <Schedule query={query} />
+              </Route>
+              <Route path={getRoutesForPage('outgoing_webhooks')} exact>
+                <OutgoingWebhooks query={query} />
+              </Route>
+              <Route path={getRoutesForPage('maintenance')} exact>
+                <Maintenance />
+              </Route>
+              <Route path={getRoutesForPage('settings')} exact>
+                <SettingsPage />
+              </Route>
+              <Route path={getRoutesForPage('chat-ops')} exact>
+                <ChatOps query={query} />
+              </Route>
+              <Route path={getRoutesForPage('live-settings')} exact>
+                <LiveSettings />
+              </Route>
+              <Route path={getRoutesForPage('cloud')} exact>
+                <CloudPage />
+              </Route>
+
+              {/* Backwards compatibility redirect routes */}
+              <Route
+                path={getRoutesForPage('incident')}
+                exact
+                render={({ location }) => (
+                  <Redirect
+                    to={{
+                      ...location,
+                      pathname: location.pathname.replace(/incident/, 'alert-group'),
+                    }}
+                  ></Redirect>
+                )}
+              ></Route>
+              <Route
+                path={getRoutesForPage('incidents')}
+                exact
+                render={({ location }) => (
+                  <Redirect
+                    to={{
+                      ...location,
+                      pathname: location.pathname.replace(/incidents/, 'alert-groups'),
+                    }}
+                  ></Redirect>
+                )}
+              ></Route>
+
+              <Route path="*">
+                <NoMatch />
+              </Route>
+            </Switch>
+          )
         ) : (
           <Unauthorized requiredUserAction={pagePermissionAction} />
         )}

From 2fc8cccb079294d06863b0224241c807e7c1e02c Mon Sep 17 00:00:00 2001
From: Rares Mardare <rares.mardare@grafana.com>
Date: Tue, 22 Aug 2023 16:10:29 +0300
Subject: [PATCH 5/5] added an e2e test for searching users

---
 .../e2e-tests/users/usersActions.test.ts           | 14 ++++++++++++++
 .../src/components/UsersFilters/UsersFilters.tsx   |  1 +
 2 files changed, 15 insertions(+)

diff --git a/grafana-plugin/e2e-tests/users/usersActions.test.ts b/grafana-plugin/e2e-tests/users/usersActions.test.ts
index 41b55aa42..fe5243722 100644
--- a/grafana-plugin/e2e-tests/users/usersActions.test.ts
+++ b/grafana-plugin/e2e-tests/users/usersActions.test.ts
@@ -57,6 +57,20 @@ test.describe('Users screen actions', () => {
     await _testButtons(editorRolePage.page, 'button.edit-other-profile-button:not([disabled])');
   });
 
+  test('Search updates the table view', async ({ adminRolePage }) => {
+    const { page } = adminRolePage;
+    await goToOnCallPage(page, 'users');
+
+    const searchInput = page.locator(`[data-testid="search-users"]`);
+
+    await searchInput.fill('oncall');
+    await page.waitForTimeout(5000);
+
+    const result = page.locator(`[data-testid="users-username"]`);
+
+    expect(await result.count()).toBe(1);
+  });
+
   /*
    * Helper methods
    */
diff --git a/grafana-plugin/src/components/UsersFilters/UsersFilters.tsx b/grafana-plugin/src/components/UsersFilters/UsersFilters.tsx
index 3773480ce..85e1cf835 100644
--- a/grafana-plugin/src/components/UsersFilters/UsersFilters.tsx
+++ b/grafana-plugin/src/components/UsersFilters/UsersFilters.tsx
@@ -38,6 +38,7 @@ const UsersFilters = (props: UsersFiltersProps) => {
         placeholder="Search users..."
         value={value.searchTerm}
         onChange={onSearchTermChangeCallback}
+        data-testid="search-users"
       />
     </div>
   );