Skip to content

Commit 3b01bf5

Browse files
fix(dashboards): Skip dashboard API requests when user has no project access (#112855)
- Extracts a reusable `useHasProjectAccess` hook from `NoProjectMessage` to consolidate the project access check logic - Uses the hook to disable dashboard API queries on dashboard manage page and side nav when no project access is available - Previously these requests fired unconditionally even though `NoProjectMessage` would discard the rendered results DAIN-1416
1 parent aded5c4 commit 3b01bf5

File tree

6 files changed

+95
-11
lines changed

6 files changed

+95
-11
lines changed

static/app/components/noProjectMessage.tsx

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import * as Layout from 'sentry/components/layouts/thirds';
99
import {t} from 'sentry/locale';
1010
import type {Organization} from 'sentry/types/organization';
1111
import {useCanCreateProject} from 'sentry/utils/useCanCreateProject';
12+
import {useHasProjectAccess} from 'sentry/utils/useHasProjectAccess';
1213
import {useProjects} from 'sentry/utils/useProjects';
13-
import {useUser} from 'sentry/utils/useUser';
1414
import {makeProjectsPathname} from 'sentry/views/projects/pathname';
1515

1616
type Props = {
@@ -24,17 +24,15 @@ export function NoProjectMessage({
2424
organization,
2525
superuserNeedsToBeProjectMember,
2626
}: Props) {
27-
const user = useUser();
28-
const {projects, initiallyLoaded: projectsLoaded} = useProjects();
27+
const {projects} = useProjects();
28+
const {hasProjectAccess, projectsLoaded} = useHasProjectAccess({
29+
superuserNeedsToBeProjectMember,
30+
});
2931

3032
const canUserCreateProject = useCanCreateProject();
3133
const canJoinTeam = organization.access.includes('team:read');
3234

3335
const orgHasProjects = !!projects?.length;
34-
const hasProjectAccess =
35-
user.isSuperuser && !superuserNeedsToBeProjectMember
36-
? !!projects?.some(p => p.hasAccess)
37-
: !!projects?.some(p => p.isMember && p.hasAccess);
3836

3937
if (hasProjectAccess || !projectsLoaded) {
4038
return <Fragment>{children}</Fragment>;
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import {ProjectFixture} from 'sentry-fixture/project';
2+
3+
import {renderHookWithProviders} from 'sentry-test/reactTestingLibrary';
4+
5+
import {ConfigStore} from 'sentry/stores/configStore';
6+
import {ProjectsStore} from 'sentry/stores/projectsStore';
7+
8+
import {useHasProjectAccess} from './useHasProjectAccess';
9+
10+
describe('useHasProjectAccess', () => {
11+
beforeEach(() => {
12+
ProjectsStore.reset();
13+
ConfigStore.set('user', {...ConfigStore.get('user'), isSuperuser: false});
14+
});
15+
16+
it('returns false when there are no projects', () => {
17+
ProjectsStore.loadInitialData([]);
18+
19+
const {result} = renderHookWithProviders(() => useHasProjectAccess());
20+
21+
expect(result.current.hasProjectAccess).toBe(false);
22+
expect(result.current.projectsLoaded).toBe(true);
23+
});
24+
25+
it('returns true when user is a member of a project with access', () => {
26+
ProjectsStore.loadInitialData([ProjectFixture({hasAccess: true, isMember: true})]);
27+
28+
const {result} = renderHookWithProviders(() => useHasProjectAccess());
29+
30+
expect(result.current.hasProjectAccess).toBe(true);
31+
});
32+
});
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import {useProjects} from 'sentry/utils/useProjects';
2+
import {useUser} from 'sentry/utils/useUser';
3+
4+
type Options = {
5+
/**
6+
* When true, superusers must also be a project member to count as having access.
7+
*/
8+
superuserNeedsToBeProjectMember?: boolean;
9+
};
10+
11+
/**
12+
* Returns whether the current user has access to at least one project,
13+
* and whether the project list has finished loading.
14+
*/
15+
export function useHasProjectAccess(options?: Options) {
16+
const user = useUser();
17+
const {projects, initiallyLoaded: projectsLoaded} = useProjects();
18+
19+
const hasProjectAccess =
20+
user.isSuperuser && !options?.superuserNeedsToBeProjectMember
21+
? !!projects?.some(p => p.hasAccess)
22+
: !!projects?.some(p => p.isMember && p.hasAccess);
23+
24+
return {hasProjectAccess, projectsLoaded};
25+
}

static/app/views/dashboards/hooks/useGetStarredDashboards.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type {Organization} from 'sentry/types/organization';
22
import {getApiUrl} from 'sentry/utils/api/getApiUrl';
33
import type {ApiQueryKey} from 'sentry/utils/queryClient';
44
import {useApiQuery} from 'sentry/utils/queryClient';
5+
import {useHasProjectAccess} from 'sentry/utils/useHasProjectAccess';
56
import {useOrganization} from 'sentry/utils/useOrganization';
67
import type {DashboardListItem} from 'sentry/views/dashboards/types';
78

@@ -29,7 +30,10 @@ export function getStarredDashboardsQueryKey(organization: Organization): ApiQue
2930

3031
export function useGetStarredDashboards() {
3132
const organization = useOrganization();
33+
const {hasProjectAccess, projectsLoaded} = useHasProjectAccess();
34+
3235
return useApiQuery<DashboardListItem[]>(getStarredDashboardsQueryKey(organization), {
3336
staleTime: Infinity,
37+
enabled: hasProjectAccess || !projectsLoaded,
3438
});
3539
}

static/app/views/dashboards/manage/index.spec.tsx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,25 @@ describe('Dashboards > Detail', () => {
112112
).toBeInTheDocument();
113113
});
114114

115+
it('does not fetch dashboards when there are no projects', async () => {
116+
act(() => ProjectsStore.loadInitialData([]));
117+
118+
const dashboardsRequest = MockApiClient.addMockResponse({
119+
url: '/organizations/org-slug/dashboards/',
120+
body: [],
121+
});
122+
123+
render(<ManageDashboards />, {
124+
organization: mockAuthorizedOrg,
125+
});
126+
127+
expect(
128+
await screen.findByText('You need at least one project to use this view')
129+
).toBeInTheDocument();
130+
131+
expect(dashboardsRequest).not.toHaveBeenCalled();
132+
});
133+
115134
it('creates new dashboard', async () => {
116135
const org = OrganizationFixture({features: FEATURES});
117136
const mockNavigate = jest.fn();

static/app/views/dashboards/manage/index.tsx

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {decodeScalar} from 'sentry/utils/queryString';
3939
import {scheduleMicroTask} from 'sentry/utils/scheduleMicroTask';
4040
import {normalizeUrl} from 'sentry/utils/url/normalizeUrl';
4141
import {useApi} from 'sentry/utils/useApi';
42+
import {useHasProjectAccess} from 'sentry/utils/useHasProjectAccess';
4243
import {useLocalStorageState} from 'sentry/utils/useLocalStorageState';
4344
import {useLocation} from 'sentry/utils/useLocation';
4445
import {useNavigate} from 'sentry/utils/useNavigate';
@@ -189,6 +190,8 @@ function ManageDashboards() {
189190
columnCount: DASHBOARD_GRID_DEFAULT_NUM_COLUMNS,
190191
});
191192

193+
const {hasProjectAccess, projectsLoaded} = useHasProjectAccess();
194+
192195
const sortOptions = getSortOptions({
193196
organization,
194197
dashboardsLayout,
@@ -222,10 +225,12 @@ function ManageDashboards() {
222225
],
223226
{
224227
staleTime: 0,
225-
enabled: !(
226-
organization.features.includes('dashboards-starred-reordering') &&
227-
dashboardsLayout === TABLE
228-
),
228+
enabled:
229+
(hasProjectAccess || !projectsLoaded) &&
230+
!(
231+
organization.features.includes('dashboards-starred-reordering') &&
232+
dashboardsLayout === TABLE
233+
),
229234
}
230235
);
231236

@@ -257,6 +262,7 @@ function ManageDashboards() {
257262
cursor: decodeScalar(location.query[OWNED_CURSOR_KEY], ''),
258263
sort: getActiveSort()!.value,
259264
enabled:
265+
(hasProjectAccess || !projectsLoaded) &&
260266
organization.features.includes('dashboards-starred-reordering') &&
261267
dashboardsLayout === TABLE,
262268
});

0 commit comments

Comments
 (0)