Skip to content

Commit

Permalink
Revert "Revert "feat(ui): Remove ids option from useTeams (#57151)""
Browse files Browse the repository at this point in the history
This reverts commit 20ecb00.
  • Loading branch information
scttcper committed Oct 2, 2023
1 parent 20ecb00 commit 150d83c
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {t} from 'sentry/locale';
import {Project} from 'sentry/types';
import {useMembers} from 'sentry/utils/useMembers';
import {useTeams} from 'sentry/utils/useTeams';
import {useTeamsById} from 'sentry/utils/useTeamsById';

import FormContext from '../formContext';

Expand Down Expand Up @@ -60,7 +61,7 @@ function SentryMemberTeamSelectorField({
currentItems?.filter(item => item.startsWith('team:')).map(user => user.slice(5)),
[currentItems]
);
useTeams({ids: ensureTeamIds});
useTeamsById({ids: ensureTeamIds});

const {
teams,
Expand Down
35 changes: 0 additions & 35 deletions static/app/utils/useTeams.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,41 +107,6 @@ describe('useTeams', function () {
expect(teams).toEqual(expect.arrayContaining(mockTeams));
});

it('can load teams by id', async function () {
const requestedTeams = [TestStubs.Team({id: '2', slug: 'requested-team'})];
const mockRequest = MockApiClient.addMockResponse({
url: `/organizations/${org.slug}/teams/`,
method: 'GET',
body: requestedTeams,
});

TeamStore.loadInitialData(mockTeams);

const {result, waitFor} = reactHooks.renderHook(useTeams, {
initialProps: {ids: ['2']},
});

expect(result.current.initiallyLoaded).toBe(false);
expect(mockRequest).toHaveBeenCalled();

await waitFor(() => expect(result.current.teams.length).toBe(1));

const {teams} = result.current;
expect(teams).toEqual(expect.arrayContaining(requestedTeams));
});

it('only loads ids when needed', function () {
TeamStore.loadInitialData(mockTeams);

const {result} = reactHooks.renderHook(useTeams, {
initialProps: {ids: [mockTeams[0].id]},
});

const {teams, initiallyLoaded} = result.current;
expect(initiallyLoaded).toBe(true);
expect(teams).toEqual(expect.arrayContaining(mockTeams));
});

it('correctly returns hasMore before and after store update', async function () {
const {result, waitFor} = reactHooks.renderHook(useTeams);

Expand Down
32 changes: 12 additions & 20 deletions static/app/utils/useTeams.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,6 @@ type Result = {
} & Pick<State, 'fetching' | 'hasMore' | 'fetchError' | 'initiallyLoaded'>;

type Options = {
/**
* When provided, fetches specified teams by id if necessary and only provides those teams.
*
* @deprecated use `useTeamsById({ids: []})`
*/
ids?: string[];
/**
* Number of teams to return when not using `props.slugs`
*/
Expand Down Expand Up @@ -99,6 +93,7 @@ type FetchTeamOptions = {

/**
* Helper function to actually load teams
*
*/
async function fetchTeams(
api: Client,
Expand Down Expand Up @@ -164,28 +159,28 @@ async function fetchTeams(
* loaded, so you should use this hook with the intention of providing specific
* slugs, or loading more through search.
*
* @deprecated use the alternatives to this hook (except for search and pagination)
* new alternatives:
* - useTeamsById({ids: []}) - get teams by id
* - useTeamsById({slugs: []}) - get teams by slug
* - useTeamsById() - just reading from the teams store
* - useUserTeams() - same as `provideUserTeams: true`
*
*/
export function useTeams({limit, slugs, ids, provideUserTeams}: Options = {}) {
export function useTeams({limit, slugs, provideUserTeams}: Options = {}) {
const api = useApi();
const {organization} = useLegacyStore(OrganizationStore);
const store = useLegacyStore(TeamStore);

const orgId = organization?.slug;

const storeSlugs = useMemo(() => new Set(store.teams.map(t => t.slug)), [store.teams]);
const storeIds = useMemo(() => new Set(store.teams.map(t => t.id)), [store.teams]);

const slugsToLoad = useMemo(
() => slugs?.filter(slug => !storeSlugs.has(slug)) ?? [],
[slugs, storeSlugs]
);

const idsToLoad = useMemo(
() => ids?.filter(id => !storeIds.has(id)) ?? [],
[ids, storeIds]
);

const shouldLoadByQuery = slugsToLoad.length > 0 || idsToLoad.length > 0;
const shouldLoadByQuery = slugsToLoad.length > 0;
const shouldLoadUserTeams = provideUserTeams && !store.loadedUserTeams;

// If we don't need to make a request either for slugs or user teams, set
Expand Down Expand Up @@ -236,7 +231,6 @@ export function useTeams({limit, slugs, ids, provideUserTeams}: Options = {}) {
try {
const {results, hasMore, nextCursor} = await fetchTeams(api, orgId, {
slugs: slugsToLoad,
ids: idsToLoad,
limit,
});

Expand All @@ -262,7 +256,7 @@ export function useTeams({limit, slugs, ids, provideUserTeams}: Options = {}) {
}));
}
},
[api, idsToLoad, limit, orgId, slugsToLoad, store.teams]
[api, limit, orgId, slugsToLoad, store.teams]
);

const handleFetchAdditionalTeams = useCallback(
Expand Down Expand Up @@ -370,12 +364,10 @@ export function useTeams({limit, slugs, ids, provideUserTeams}: Options = {}) {
const filteredTeams = useMemo(() => {
return slugs
? store.teams.filter(t => slugs.includes(t.slug))
: ids
? store.teams.filter(t => ids.includes(t.id))
: provideUserTeams && !isSuperuser
? store.teams.filter(t => t.isMember)
: store.teams;
}, [store.teams, ids, slugs, provideUserTeams, isSuperuser]);
}, [store.teams, slugs, provideUserTeams, isSuperuser]);

const result: Result = {
teams: filteredTeams,
Expand Down

0 comments on commit 150d83c

Please sign in to comment.