Skip to content

Commit d1aca0f

Browse files
harshithaduraiharshithadurai
andauthored
feat(dashboards): frontend to restrict dashboard edit access to teams (#80344)
Addresses: #79828 and #79833 Corresponding backend change: #80136 - Add teams as options to the `EditAccessSelector` component - Refactor use of `isCreatorOnlyEditable` to use `isEditableByEveryone` - Add `updateDashbaordPermissions()` function to make a separate `PUT` request to dashboard details endpoint that sends only the perms object - Rename 'Everyone' option to 'All users', and modify tests as well Before: <img width="200" alt="Screenshot 2024-11-07 at 10 26 47 AM" src="https://github.com/user-attachments/assets/9aacc1d6-3002-439b-9a03-c39ec952b955"> After: <img width="200" alt="Screenshot 2024-11-07 at 10 25 38 AM" src="https://github.com/user-attachments/assets/1358752c-30e7-48d0-ba64-f36f15b183b3"> --------- Co-authored-by: harshithadurai <[email protected]>
1 parent afc0909 commit d1aca0f

File tree

7 files changed

+446
-172
lines changed

7 files changed

+446
-172
lines changed

static/app/actionCreators/dashboards.tsx

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,8 @@ export function createDashboard(
4242
newDashboard: DashboardDetails,
4343
duplicate?: boolean
4444
): Promise<DashboardDetails> {
45-
const {
46-
title,
47-
widgets,
48-
projects,
49-
environment,
50-
period,
51-
start,
52-
end,
53-
filters,
54-
utc,
55-
permissions,
56-
} = newDashboard;
45+
const {title, widgets, projects, environment, period, start, end, filters, utc} =
46+
newDashboard;
5747

5848
const promise: Promise<DashboardDetails> = api.requestPromise(
5949
`/organizations/${orgSlug}/dashboards/`,
@@ -70,7 +60,6 @@ export function createDashboard(
7060
end,
7161
filters,
7262
utc,
73-
permissions,
7463
},
7564
query: {
7665
project: projects,
@@ -138,18 +127,8 @@ export function updateDashboard(
138127
orgId: string,
139128
dashboard: DashboardDetails
140129
): Promise<DashboardDetails> {
141-
const {
142-
title,
143-
widgets,
144-
projects,
145-
environment,
146-
period,
147-
start,
148-
end,
149-
filters,
150-
utc,
151-
permissions,
152-
} = dashboard;
130+
const {title, widgets, projects, environment, period, start, end, filters, utc} =
131+
dashboard;
153132
const data = {
154133
title,
155134
widgets: widgets.map(widget => omit(widget, ['tempId'])),
@@ -160,7 +139,6 @@ export function updateDashboard(
160139
end,
161140
filters,
162141
utc,
163-
permissions,
164142
};
165143

166144
const promise: Promise<DashboardDetails> = api.requestPromise(
@@ -239,6 +217,37 @@ export function validateWidgetRequest(
239217
] as const;
240218
}
241219

220+
export function updateDashboardPermissions(
221+
api: Client,
222+
orgId: string,
223+
dashboard: DashboardDetails
224+
): Promise<DashboardDetails> {
225+
const {permissions} = dashboard;
226+
const data = {
227+
permissions,
228+
};
229+
const promise: Promise<DashboardDetails> = api.requestPromise(
230+
`/organizations/${orgId}/dashboards/${dashboard.id}/`,
231+
{
232+
method: 'PUT',
233+
data,
234+
}
235+
);
236+
237+
promise.catch(response => {
238+
const errorResponse = response?.responseJSON ?? null;
239+
240+
if (errorResponse) {
241+
const errors = flattenErrors(errorResponse, {});
242+
addErrorMessage(errors[Object.keys(errors)[0]] as string);
243+
} else {
244+
addErrorMessage(t('Unable to update dashboard permissions'));
245+
}
246+
});
247+
248+
return promise;
249+
}
250+
242251
export function validateWidget(
243252
api: Client,
244253
orgId: string,

static/app/views/dashboards/controls.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {trackAnalytics} from 'sentry/utils/analytics';
1717
import {hasCustomMetrics} from 'sentry/utils/metrics/features';
1818
import useOrganization from 'sentry/utils/useOrganization';
1919
import {useUser} from 'sentry/utils/useUser';
20+
import {useUserTeams} from 'sentry/utils/useUserTeams';
2021
import {AddWidgetButton} from 'sentry/views/dashboards/addWidget';
2122
import EditAccessSelector from 'sentry/views/dashboards/editAccessSelector';
2223
import {DataSet} from 'sentry/views/dashboards/widgetBuilder/utils';
@@ -71,6 +72,7 @@ function Controls({
7172

7273
const organization = useOrganization();
7374
const currentUser = useUser();
75+
const {teams: userTeams} = useUserTeams();
7476

7577
if ([DashboardState.EDIT, DashboardState.PENDING_DELETE].includes(dashboardState)) {
7678
return (
@@ -147,7 +149,12 @@ function Controls({
147149

148150
let hasEditAccess = true;
149151
if (organization.features.includes('dashboards-edit-access')) {
150-
hasEditAccess = checkUserHasEditAccess(dashboard, currentUser, organization);
152+
hasEditAccess = checkUserHasEditAccess(
153+
dashboard,
154+
currentUser,
155+
userTeams,
156+
organization
157+
);
151158
}
152159

153160
return (

static/app/views/dashboards/detail.spec.tsx

Lines changed: 103 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {OrganizationFixture} from 'sentry-fixture/organization';
44
import {ProjectFixture} from 'sentry-fixture/project';
55
import {ReleaseFixture} from 'sentry-fixture/release';
66
import {RouteComponentPropsFixture} from 'sentry-fixture/routeComponentPropsFixture';
7+
import {TeamFixture} from 'sentry-fixture/team';
78
import {UserFixture} from 'sentry-fixture/user';
89
import {WidgetFixture} from 'sentry-fixture/widget';
910

@@ -21,6 +22,7 @@ import * as modals from 'sentry/actionCreators/modal';
2122
import ConfigStore from 'sentry/stores/configStore';
2223
import PageFiltersStore from 'sentry/stores/pageFiltersStore';
2324
import ProjectsStore from 'sentry/stores/projectsStore';
25+
import TeamStore from 'sentry/stores/teamStore';
2426
import {browserHistory} from 'sentry/utils/browserHistory';
2527
import CreateDashboard from 'sentry/views/dashboards/create';
2628
import {handleUpdateDashboardSplit} from 'sentry/views/dashboards/detail';
@@ -1667,7 +1669,7 @@ describe('Dashboards > Detail', function () {
16671669

16681670
await userEvent.click(await screen.findByText('Edit Access:'));
16691671
expect(screen.getByText('Creator')).toBeInTheDocument();
1670-
expect(screen.getByText('Everyone')).toBeInTheDocument();
1672+
expect(screen.getByText('All users')).toBeInTheDocument();
16711673
});
16721674

16731675
it('creates and updates new permissions for dashboard with no edit perms initialized', async function () {
@@ -1699,28 +1701,27 @@ describe('Dashboards > Detail', function () {
16991701
);
17001702
await userEvent.click(await screen.findByText('Edit Access:'));
17011703

1702-
// deselects 'Everyone' so only creator has edit access
1703-
expect(await screen.findByText('Everyone')).toBeEnabled();
1704-
expect(await screen.findByRole('option', {name: 'Everyone'})).toHaveAttribute(
1704+
// deselects 'All users' so only creator has edit access
1705+
expect(await screen.findByText('All users')).toBeEnabled();
1706+
expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute(
17051707
'aria-selected',
17061708
'true'
17071709
);
1708-
await userEvent.click(screen.getByRole('option', {name: 'Everyone'}));
1709-
expect(await screen.findByRole('option', {name: 'Everyone'})).toHaveAttribute(
1710+
await userEvent.click(screen.getByRole('option', {name: 'All users'}));
1711+
expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute(
17101712
'aria-selected',
17111713
'false'
17121714
);
17131715

1714-
// clicks out of dropdown to trigger onClose()
1715-
await userEvent.click(await screen.findByText('Edit Access:'));
1716+
await userEvent.click(await screen.findByText('Save Changes'));
17161717

17171718
await waitFor(() => {
17181719
expect(mockPUT).toHaveBeenCalledTimes(1);
17191720
expect(mockPUT).toHaveBeenCalledWith(
17201721
'/organizations/org-slug/dashboards/1/',
17211722
expect.objectContaining({
17221723
data: expect.objectContaining({
1723-
permissions: {isCreatorOnlyEditable: true},
1724+
permissions: {isEditableByEveryone: false, teamsWithEditAccess: []},
17241725
}),
17251726
})
17261727
);
@@ -1739,7 +1740,7 @@ describe('Dashboards > Detail', function () {
17391740
id: '1',
17401741
title: 'Custom Errors',
17411742
createdBy: UserFixture({id: '781629'}),
1742-
permissions: {isCreatorOnlyEditable: true},
1743+
permissions: {isEditableByEveryone: false},
17431744
}),
17441745
});
17451746

@@ -1768,28 +1769,111 @@ describe('Dashboards > Detail', function () {
17681769
);
17691770
await userEvent.click(await screen.findByText('Edit Access:'));
17701771

1771-
// selects 'Everyone' so everyone has edit access
1772-
expect(await screen.findByText('Everyone')).toBeEnabled();
1773-
expect(await screen.findByRole('option', {name: 'Everyone'})).toHaveAttribute(
1772+
// selects 'All users' so everyone has edit access
1773+
expect(await screen.findByText('All users')).toBeEnabled();
1774+
expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute(
17741775
'aria-selected',
17751776
'false'
17761777
);
1777-
await userEvent.click(screen.getByRole('option', {name: 'Everyone'}));
1778-
expect(await screen.findByRole('option', {name: 'Everyone'})).toHaveAttribute(
1778+
await userEvent.click(screen.getByRole('option', {name: 'All users'}));
1779+
expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute(
17791780
'aria-selected',
17801781
'true'
17811782
);
17821783

1783-
// clicks out of dropdown to trigger onClose()
1784+
await userEvent.click(await screen.findByText('Save Changes'));
1785+
1786+
await waitFor(() => {
1787+
expect(mockPUT).toHaveBeenCalledTimes(1);
1788+
expect(mockPUT).toHaveBeenCalledWith(
1789+
'/organizations/org-slug/dashboards/1/',
1790+
expect.objectContaining({
1791+
data: expect.objectContaining({
1792+
permissions: {isEditableByEveryone: true, teamsWithEditAccess: []},
1793+
}),
1794+
})
1795+
);
1796+
});
1797+
});
1798+
1799+
it('creator can update permissions with teams for dashboard', async function () {
1800+
const mockPUT = MockApiClient.addMockResponse({
1801+
url: '/organizations/org-slug/dashboards/1/',
1802+
method: 'PUT',
1803+
body: DashboardFixture([], {id: '1', title: 'Custom Errors'}),
1804+
});
1805+
MockApiClient.addMockResponse({
1806+
url: '/organizations/org-slug/dashboards/1/',
1807+
body: DashboardFixture([], {
1808+
id: '1',
1809+
title: 'Custom Errors',
1810+
createdBy: UserFixture({id: '781629'}),
1811+
permissions: {isEditableByEveryone: false},
1812+
}),
1813+
});
1814+
1815+
const currentUser = UserFixture({id: '781629'});
1816+
ConfigStore.set('user', currentUser);
1817+
1818+
const teamData = [
1819+
{
1820+
id: '1',
1821+
slug: 'team1',
1822+
name: 'Team 1',
1823+
},
1824+
{
1825+
id: '2',
1826+
slug: 'team2',
1827+
name: 'Team 2',
1828+
},
1829+
{
1830+
id: '3',
1831+
slug: 'team3',
1832+
name: 'Team 3',
1833+
},
1834+
];
1835+
const teams = teamData.map(data => TeamFixture(data));
1836+
1837+
TeamStore.loadInitialData(teams);
1838+
1839+
render(
1840+
<ViewEditDashboard
1841+
{...RouteComponentPropsFixture()}
1842+
organization={{
1843+
...initialData.organization,
1844+
features: ['dashboards-edit-access', ...initialData.organization.features],
1845+
}}
1846+
params={{orgId: 'org-slug', dashboardId: '1'}}
1847+
router={initialData.router}
1848+
location={initialData.router.location}
1849+
>
1850+
{null}
1851+
</ViewEditDashboard>,
1852+
{
1853+
router: initialData.router,
1854+
organization: {
1855+
features: ['dashboards-edit-access', ...initialData.organization.features],
1856+
},
1857+
}
1858+
);
17841859
await userEvent.click(await screen.findByText('Edit Access:'));
17851860

1861+
expect(await screen.findByText('All users')).toBeEnabled();
1862+
expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute(
1863+
'aria-selected',
1864+
'false'
1865+
);
1866+
await userEvent.click(screen.getByRole('option', {name: '#team1'}));
1867+
await userEvent.click(screen.getByRole('option', {name: '#team2'}));
1868+
await userEvent.click(await screen.findByText('Save Changes'));
1869+
17861870
await waitFor(() => {
17871871
expect(mockPUT).toHaveBeenCalledTimes(1);
17881872
expect(mockPUT).toHaveBeenCalledWith(
17891873
'/organizations/org-slug/dashboards/1/',
17901874
expect.objectContaining({
17911875
data: expect.objectContaining({
1792-
permissions: {isCreatorOnlyEditable: false},
1876+
permissions: {isEditableByEveryone: false, teamsWithEditAccess: [1, 2]},
17931877
}),
17941878
})
17951879
);
@@ -1804,7 +1888,7 @@ describe('Dashboards > Detail', function () {
18041888
id: '1',
18051889
title: 'Custom Errors',
18061890
createdBy: UserFixture({id: '238900'}),
1807-
permissions: {isCreatorOnlyEditable: true},
1891+
permissions: {isEditableByEveryone: false},
18081892
}),
18091893
],
18101894
});
@@ -1814,7 +1898,7 @@ describe('Dashboards > Detail', function () {
18141898
id: '1',
18151899
title: 'Custom Errors',
18161900
createdBy: UserFixture({id: '238900'}),
1817-
permissions: {isCreatorOnlyEditable: true},
1901+
permissions: {isEditableByEveryone: false},
18181902
}),
18191903
});
18201904

static/app/views/dashboards/detail.tsx

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
createDashboard,
1010
deleteDashboard,
1111
updateDashboard,
12+
updateDashboardPermissions,
1213
} from 'sentry/actionCreators/dashboards';
1314
import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator';
1415
import {openWidgetViewerModal} from 'sentry/actionCreators/modal';
@@ -28,7 +29,7 @@ import {t} from 'sentry/locale';
2829
import {space} from 'sentry/styles/space';
2930
import type {PageFilters} from 'sentry/types/core';
3031
import type {PlainRoute, RouteComponentProps} from 'sentry/types/legacyReactRouter';
31-
import type {Organization} from 'sentry/types/organization';
32+
import type {Organization, Team} from 'sentry/types/organization';
3233
import type {Project} from 'sentry/types/project';
3334
import {defined} from 'sentry/utils';
3435
import {trackAnalytics} from 'sentry/utils/analytics';
@@ -173,17 +174,24 @@ export function handleUpdateDashboardSplit({
173174
export function checkUserHasEditAccess(
174175
dashboard: DashboardDetails,
175176
currentUser: User,
177+
userTeams: Team[],
176178
organization: Organization
177179
): boolean {
178180
if (
179181
!organization.features.includes('dashboards-edit-access') ||
180-
!dashboard.permissions
182+
!dashboard.permissions ||
183+
dashboard.permissions.isEditableByEveryone ||
184+
dashboard.createdBy?.id === currentUser.id
181185
) {
182186
return true;
183187
}
184-
return dashboard.permissions.isCreatorOnlyEditable
185-
? dashboard.createdBy?.id === currentUser.id
186-
: !dashboard.permissions.isCreatorOnlyEditable;
188+
if (dashboard.permissions.teamsWithEditAccess?.length) {
189+
const userTeamIds = userTeams.map(team => Number(team.id));
190+
dashboard.permissions.teamsWithEditAccess.some(teamId =>
191+
userTeamIds.includes(teamId)
192+
);
193+
}
194+
return false;
187195
}
188196

189197
class DashboardDetail extends Component<Props, State> {
@@ -622,7 +630,7 @@ class DashboardDetail extends Component<Props, State> {
622630
const dashboardCopy = cloneDashboard(dashboard);
623631
dashboardCopy.permissions = newDashboardPermissions;
624632

625-
updateDashboard(api, organization.slug, dashboardCopy).then(
633+
updateDashboardPermissions(api, organization.slug, dashboardCopy).then(
626634
(newDashboard: DashboardDetails) => {
627635
addSuccessMessage(t('Dashboard Edit Access updated.'));
628636
this.props.onDashboardUpdate?.(newDashboard);

0 commit comments

Comments
 (0)