Browse Source

feat(team-selector): Implement useTeams hook for searching (#28547)

Evan Purkhiser 3 years ago
parent
commit
4e9181a02b

+ 10 - 6
static/app/components/forms/teamSelector.tsx

@@ -1,19 +1,21 @@
 import {useEffect, useRef, useState} from 'react';
 import {StylesConfig} from 'react-select';
 import styled from '@emotion/styled';
+import debounce from 'lodash/debounce';
 
 import {addTeamToProject} from 'app/actionCreators/projects';
 import Button from 'app/components/button';
 import SelectControl, {ControlProps} from 'app/components/forms/selectControl';
 import IdBadge from 'app/components/idBadge';
 import Tooltip from 'app/components/tooltip';
+import {DEFAULT_DEBOUNCE_DURATION} from 'app/constants';
 import {IconAdd, IconUser} from 'app/icons';
 import {t} from 'app/locale';
 import space from 'app/styles/space';
 import {Organization, Project, Team} from 'app/types';
 import useApi from 'app/utils/useApi';
+import useTeams from 'app/utils/useTeams';
 import withOrganization from 'app/utils/withOrganization';
-import withTeams from 'app/utils/withTeams';
 
 const UnassignedWrapper = styled('div')`
   display: flex;
@@ -74,7 +76,6 @@ const placeholderSelectStyles: StylesConfig = {
 
 type Props = {
   organization: Organization;
-  teams: Team[];
   onChange: (value: any) => any;
   /**
    * Function to control whether a team should be shown in the dropdown
@@ -107,10 +108,11 @@ type TeamOption = {
 
 function TeamSelector(props: Props) {
   const {includeUnassigned, styles, ...extraProps} = props;
-  const {teams, teamFilter, organization, project, multiple, value, useId, onChange} =
-    props;
+  const {teamFilter, organization, project, multiple, value, useId, onChange} = props;
 
   const api = useApi();
+  const {teams, fetching, onSearch} = useTeams();
+
   const [options, setOptions] = useState<TeamOption[]>([]);
 
   // TODO(ts) This type could be improved when react-select types are better.
@@ -249,12 +251,14 @@ function TeamSelector(props: Props) {
     <SelectControl
       ref={selectRef}
       options={options}
+      onInputChange={debounce(val => void onSearch(val), DEFAULT_DEBOUNCE_DURATION)}
       isOptionDisabled={option => !!option.disabled}
       styles={{
         ...(styles ?? {}),
         ...(includeUnassigned ? unassignedSelectStyles : {}),
         ...placeholderSelectStyles,
       }}
+      isLoading={fetching}
       {...extraProps}
     />
   );
@@ -279,6 +283,6 @@ const AddToProjectButton = styled(Button)`
 export {TeamSelector};
 
 // TODO(davidenwang): this is broken due to incorrect types on react-select
-export default withTeams(withOrganization(TeamSelector)) as unknown as (
-  p: Omit<Props, 'teams' | 'organization'>
+export default withOrganization(TeamSelector) as unknown as (
+  p: Omit<Props, 'organization'>
 ) => JSX.Element;

+ 185 - 0
static/app/utils/useTeams.tsx

@@ -0,0 +1,185 @@
+import {useState} from 'react';
+import uniqBy from 'lodash/uniqBy';
+
+import TeamActions from 'app/actions/teamActions';
+import {Client} from 'app/api';
+import OrganizationStore from 'app/stores/organizationStore';
+import TeamStore from 'app/stores/teamStore';
+import {useLegacyStore} from 'app/stores/useLegacyStore';
+import {Team} from 'app/types';
+import parseLinkHeader from 'app/utils/parseLinkHeader';
+import RequestError from 'app/utils/requestError/requestError';
+import useApi from 'app/utils/useApi';
+
+type State = {
+  /**
+   * This is state for when fetching data from API
+   */
+  fetching: boolean;
+  /**
+   * The error that occurred if fetching failed
+   */
+  fetchError: null | RequestError;
+  /**
+   * Indicates that Team results (from API) are paginated and there are more
+   * Teams that are not in the initial response.
+   */
+  hasMore: null | boolean;
+  /**
+   * The last query we searched. Used to validate the cursor
+   */
+  lastSearch: null | string;
+  /**
+   * Pagination
+   */
+  nextCursor?: null | string;
+};
+
+export type Result = {
+  /**
+   * The loaded teams list
+   */
+  teams: Team[];
+  /**
+   * This is an action provided to consumers for them to update the current
+   * teams result set using a simple search query. You can allow the new
+   * results to either be appended or replace the existing results.
+   */
+  onSearch: (searchTerm: string) => Promise<void>;
+} & Pick<State, 'fetching' | 'hasMore' | 'fetchError'>;
+
+type Options = {
+  /**
+   * Number of teams to return when not using `props.slugs`
+   */
+  limit?: number;
+};
+
+type FetchTeamOptions = Pick<Options, 'limit'> & {
+  slugs?: string[];
+  cursor?: State['nextCursor'];
+  search?: State['lastSearch'];
+  lastSearch?: State['lastSearch'];
+};
+
+/**
+ * Helper function to actually load teams
+ */
+async function fetchTeams(
+  api: Client,
+  orgId: string,
+  {slugs, search, limit, lastSearch, cursor}: FetchTeamOptions = {}
+) {
+  const query: {
+    query?: string;
+    cursor?: typeof cursor;
+    per_page?: number;
+  } = {};
+
+  if (slugs !== undefined && slugs.length > 0) {
+    query.query = slugs.map(slug => `slug:${slug}`).join(' ');
+  }
+
+  if (search) {
+    query.query = `${query.query ?? ''} ${search}`.trim();
+  }
+
+  const isSameSearch = lastSearch === search || (!lastSearch && !search);
+
+  if (isSameSearch && cursor) {
+    query.cursor = cursor;
+  }
+
+  if (limit !== undefined) {
+    query.per_page = limit;
+  }
+
+  let hasMore: null | boolean = false;
+  let nextCursor: null | string = null;
+  const [data, , resp] = await api.requestPromise(`/organizations/${orgId}/teams/`, {
+    includeAllArgs: true,
+    query,
+  });
+
+  const pageLinks = resp?.getResponseHeader('Link');
+  if (pageLinks) {
+    const paginationObject = parseLinkHeader(pageLinks);
+    hasMore = paginationObject?.next?.results || paginationObject?.previous?.results;
+    nextCursor = paginationObject?.next?.cursor;
+  }
+
+  return {results: data, hasMore, nextCursor};
+}
+
+function useTeams({limit}: Options = {}) {
+  const api = useApi();
+  const {organization} = useLegacyStore(OrganizationStore);
+  const store = useLegacyStore(TeamStore);
+
+  const [state, setState] = useState<State>({
+    fetching: false,
+    hasMore: null,
+    lastSearch: null,
+    nextCursor: null,
+    fetchError: null,
+  });
+
+  async function handleSearch(search: string) {
+    const {lastSearch} = state;
+    const cursor = state.nextCursor;
+
+    if (search === '') {
+      return;
+    }
+
+    const orgId = organization?.slug;
+    if (orgId === undefined) {
+      // eslint-disable-next-line no-console
+      console.error('Cannot use useTeam.onSearch without an orgId passed to useTeam');
+      return;
+    }
+
+    setState({...state, fetching: true});
+
+    try {
+      api.clear();
+      const {results, hasMore, nextCursor} = await fetchTeams(api, orgId, {
+        search,
+        limit,
+        lastSearch,
+        cursor,
+      });
+
+      const fetchedTeams = uniqBy([...store.teams, ...results], ({slug}) => slug);
+
+      // Only update the store if we have more items
+      if (fetchedTeams.length > store.teams.length) {
+        TeamActions.loadTeams(fetchedTeams);
+      }
+
+      setState({
+        ...state,
+        hasMore,
+        fetching: false,
+        lastSearch: search,
+        nextCursor,
+      });
+    } catch (err) {
+      console.error(err); // eslint-disable-line no-console
+
+      setState({...state, fetching: false, fetchError: err});
+    }
+  }
+
+  const result: Result = {
+    teams: store.teams,
+    fetching: state.fetching || store.loading,
+    fetchError: state.fetchError,
+    hasMore: state.hasMore,
+    onSearch: handleSearch,
+  };
+
+  return result;
+}
+
+export default useTeams;

+ 10 - 31
static/app/utils/withTeams.tsx

@@ -1,49 +1,28 @@
 import * as React from 'react';
 
-import TeamStore from 'app/stores/teamStore';
 import {Team} from 'app/types';
 import getDisplayName from 'app/utils/getDisplayName';
+import useTeams from 'app/utils/useTeams';
 
 type InjectedTeamsProps = {
   teams?: Team[];
 };
 
-type State = {
-  teams: Team[];
-};
-
 /**
- * Higher order component that uses TeamStore and provides a list of teams
+ * Higher order component that provides a list of teams
  */
-function withTeams<P extends InjectedTeamsProps>(
+const withTeams = <P extends InjectedTeamsProps>(
   WrappedComponent: React.ComponentType<P>
-) {
-  class WithTeams extends React.Component<
-    Omit<P, keyof InjectedTeamsProps> & InjectedTeamsProps,
-    State
-  > {
-    static displayName = `withTeams(${getDisplayName(WrappedComponent)})`;
-
-    state = {
-      teams: TeamStore.getAll(),
+) => {
+  const WithTeams: React.FC<Omit<P, keyof InjectedTeamsProps> & InjectedTeamsProps> =
+    props => {
+      const {teams} = useTeams();
+      return <WrappedComponent teams={teams} {...(props as P)} />;
     };
 
-    componentWillUnmount() {
-      this.unsubscribe();
-    }
+  WithTeams.displayName = `withTeams(${getDisplayName(WrappedComponent)})`;
 
-    unsubscribe = TeamStore.listen(
-      () => this.setState({teams: TeamStore.getAll()}),
-      undefined
-    );
-
-    render() {
-      return (
-        <WrappedComponent teams={this.state.teams as Team[]} {...(this.props as P)} />
-      );
-    }
-  }
   return WithTeams;
-}
+};
 
 export default withTeams;

+ 2 - 1
tests/acceptance/test_organization_plugin_detail_view.py

@@ -33,7 +33,7 @@ class OrganizationPluginDetailedView(AcceptanceTestCase):
         detail_view_page = OrganizationAbstractDetailViewPage(browser=self.browser)
         detail_view_page.click_install_button()
 
-        self.browser.click('[id="react-select-2-option-0-0"]')
+        self.browser.click('[role="dialog"] [id$="option-0-0"]')
         # check if we got to the configuration page with the form
         self.browser.wait_until_not(".loading-indicator")
         self.browser.wait_until_test_id("plugin-config")
@@ -49,6 +49,7 @@ class OrganizationPluginDetailedView(AcceptanceTestCase):
         detail_view_page = OrganizationAbstractDetailViewPage(browser=self.browser)
 
         assert self.browser.element_exists('[aria-label="Configure"]')
+
         detail_view_page.uninstall()
         self.browser.wait_until('[data-test-id="toast-success"]')
         assert not self.browser.element_exists('[aria-label="Configure"]')

+ 6 - 8
tests/js/spec/components/forms/teamSelector.spec.jsx

@@ -2,6 +2,7 @@ import {act, fireEvent, mountWithTheme} from 'sentry-test/reactTestingLibrary';
 
 import {addTeamToProject} from 'app/actionCreators/projects';
 import {TeamSelector} from 'app/components/forms/teamSelector';
+import TeamStore from 'app/stores/teamStore';
 
 jest.mock('app/actionCreators/projects', () => ({
   addTeamToProject: jest.fn(),
@@ -26,16 +27,11 @@ const teamData = [
 ];
 const teams = teamData.map(data => TestStubs.Team(data));
 const project = TestStubs.Project({teams: [teams[0]]});
+const organization = TestStubs.Organization({access: ['project:write']});
 
 function createWrapper(props = {}) {
-  const organization = TestStubs.Organization({access: ['project:write']});
   return mountWithTheme(
-    <TeamSelector
-      teams={teams}
-      organization={organization}
-      name="teamSelector"
-      {...props}
-    />
+    <TeamSelector organization={organization} name="teamSelector" {...props} />
   );
 }
 
@@ -49,7 +45,9 @@ function openSelectMenu(wrapper) {
 }
 
 describe('Team Selector', function () {
-  beforeAll(function () {});
+  beforeEach(function () {
+    act(() => void TeamStore.loadInitialData(teams));
+  });
 
   it('renders options', function () {
     const wrapper = createWrapper();

+ 9 - 2
tests/js/spec/utils/discover/teamKeyTransactionField.spec.jsx

@@ -1,3 +1,5 @@
+import {act} from 'react-dom/test-utils';
+
 import {mountWithTheme} from 'sentry-test/enzyme';
 
 import * as TeamKeyTransactionManager from 'app/components/performance/teamKeyTransactionsManager';
@@ -22,7 +24,10 @@ describe('TeamKeyTransactionField', function () {
   beforeEach(function () {
     MockApiClient.clearMockResponses();
     ProjectsStore.loadInitialData([project]);
-    TeamStore.loadInitialData(teams);
+
+    act(() => {
+      TeamStore.loadInitialData(teams);
+    });
   });
 
   it('renders with all teams checked', async function () {
@@ -418,7 +423,9 @@ describe('TeamKeyTransactionField', function () {
 
   it('should render teams without access separately', async function () {
     const myTeams = [...teams, TestStubs.Team({id: '3', slug: 'team3', name: 'Team 3'})];
-    TeamStore.loadInitialData(myTeams);
+    act(() => {
+      TeamStore.loadInitialData(myTeams);
+    });
 
     MockApiClient.addMockResponse({
       method: 'GET',

+ 64 - 0
tests/js/spec/utils/useTeams.spec.tsx

@@ -0,0 +1,64 @@
+import {act, renderHook} from '@testing-library/react-hooks';
+
+import OrganizationStore from 'app/stores/organizationStore';
+import TeamStore from 'app/stores/teamStore';
+import useTeams from 'app/utils/useTeams';
+
+describe('useTeams', function () {
+  // @ts-expect-error
+  const org = TestStubs.Organization();
+
+  // @ts-expect-error
+  const mockTeams = [TestStubs.Team()];
+
+  it('provides teams from the team store', function () {
+    act(() => void TeamStore.loadInitialData(mockTeams));
+
+    const {result} = renderHook(() => useTeams());
+    const {teams} = result.current;
+
+    expect(teams).toBe(mockTeams);
+  });
+
+  it('loads more teams when using onSearch', async function () {
+    act(() => void TeamStore.loadInitialData(mockTeams));
+    act(() => void OrganizationStore.onUpdate(org, {replace: true}));
+
+    // @ts-expect-error
+    const newTeam2 = TestStubs.Team({id: '2', slug: 'test-team2'});
+    // @ts-expect-error
+    const newTeam3 = TestStubs.Team({id: '3', slug: 'test-team3'});
+
+    // @ts-expect-error
+    const mockRequest = MockApiClient.addMockResponse({
+      url: `/organizations/${org.slug}/teams/`,
+      method: 'GET',
+      query: {query: 'test'},
+      body: [newTeam2, newTeam3],
+    });
+
+    const {result, waitFor} = renderHook(() => useTeams());
+    const {onSearch} = result.current;
+
+    // Works with append
+    const onSearchPromise = act(() => onSearch('test'));
+
+    expect(result.current.fetching).toBe(true);
+    await onSearchPromise;
+    expect(result.current.fetching).toBe(false);
+
+    // Wait for state to be reflected from the store
+    await waitFor(() => result.current.teams.length === 3);
+
+    expect(mockRequest).toHaveBeenCalled();
+    expect(result.current.teams).toEqual([...mockTeams, newTeam2, newTeam3]);
+
+    // de-duplicates itesm in the query results
+    mockRequest.mockClear();
+    await act(() => onSearch('test'));
+
+    // No new items have been added
+    expect(mockRequest).toHaveBeenCalled();
+    expect(result.current.teams).toEqual([...mockTeams, newTeam2, newTeam3]);
+  });
+});

+ 8 - 2
tests/js/spec/views/onboarding/platform.spec.jsx

@@ -1,3 +1,5 @@
+import {act} from 'react-dom/test-utils';
+
 import {mountWithTheme} from 'sentry-test/enzyme';
 
 import {createProject} from 'app/actionCreators/projects';
@@ -32,7 +34,9 @@ describe('OnboardingWelcome', function () {
 
     // Select a platform to create
     wrapper.setProps({platform: 'dotnet'});
-    TeamStore.loadInitialData([{id: '1', slug: 'team-slug'}]);
+    act(() => {
+      TeamStore.loadInitialData([{id: '1', slug: 'team-slug'}]);
+    });
     expect(getButton().text()).toEqual('Create Project');
     expect(getButton().props().disabled).toBe(false);
 
@@ -71,7 +75,9 @@ describe('OnboardingWelcome', function () {
 
     const getButton = () => wrapper.find('Button[priority="primary"]');
 
-    TeamStore.loadInitialData([{id: '1', slug: 'team-slug'}]);
+    act(() => {
+      TeamStore.loadInitialData([{id: '1', slug: 'team-slug'}]);
+    });
     expect(getButton().text()).toEqual('Set Up Your Project');
     expect(getButton().props().disabled).toBe(false);
 

+ 5 - 1
tests/js/spec/views/performance/transactionSummary.spec.jsx

@@ -2,6 +2,7 @@ import {browserHistory} from 'react-router';
 
 import {mountWithTheme} from 'sentry-test/enzyme';
 import {initializeOrg} from 'sentry-test/initializeOrg';
+import {act} from 'sentry-test/reactTestingLibrary';
 
 import ProjectsStore from 'app/stores/projectsStore';
 import TeamStore from 'app/stores/teamStore';
@@ -34,7 +35,10 @@ function initializeData({features: additionalFeatures = [], query = {}} = {}) {
     },
   });
   ProjectsStore.loadInitialData(initialData.organization.projects);
-  TeamStore.loadInitialData(teams);
+
+  act(() => {
+    TeamStore.loadInitialData(teams);
+  });
   return initialData;
 }
 

+ 2 - 1
tests/js/spec/views/performance/transactionSummary/teamKeyTransactionButton.spec.jsx

@@ -1,4 +1,5 @@
 import {mountWithTheme} from 'sentry-test/enzyme';
+import {act} from 'sentry-test/reactTestingLibrary';
 
 import ProjectsStore from 'app/stores/projectsStore';
 import TeamStore from 'app/stores/teamStore';
@@ -35,7 +36,7 @@ describe('TeamKeyTransactionButton', function () {
   beforeEach(function () {
     MockApiClient.clearMockResponses();
     ProjectsStore.loadInitialData([project]);
-    TeamStore.loadInitialData(teams);
+    act(() => void TeamStore.loadInitialData(teams));
   });
 
   it('fetches key transactions with project param', async function () {