Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 35 additions & 26 deletions static/app/actionCreators/dashboards.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,8 @@ export function createDashboard(
newDashboard: DashboardDetails,
duplicate?: boolean
): Promise<DashboardDetails> {
const {
title,
widgets,
projects,
environment,
period,
start,
end,
filters,
utc,
permissions,
} = newDashboard;
const {title, widgets, projects, environment, period, start, end, filters, utc} =
newDashboard;

const promise: Promise<DashboardDetails> = api.requestPromise(
`/organizations/${orgSlug}/dashboards/`,
Expand All @@ -70,7 +60,6 @@ export function createDashboard(
end,
filters,
utc,
permissions,
},
query: {
project: projects,
Expand Down Expand Up @@ -138,18 +127,8 @@ export function updateDashboard(
orgId: string,
dashboard: DashboardDetails
): Promise<DashboardDetails> {
const {
title,
widgets,
projects,
environment,
period,
start,
end,
filters,
utc,
permissions,
} = dashboard;
const {title, widgets, projects, environment, period, start, end, filters, utc} =
dashboard;
const data = {
title,
widgets: widgets.map(widget => omit(widget, ['tempId'])),
Expand All @@ -160,7 +139,6 @@ export function updateDashboard(
end,
filters,
utc,
permissions,
};

const promise: Promise<DashboardDetails> = api.requestPromise(
Expand Down Expand Up @@ -239,6 +217,37 @@ export function validateWidgetRequest(
] as const;
}

export function updateDashboardPermissions(
api: Client,
orgId: string,
dashboard: DashboardDetails
): Promise<DashboardDetails> {
const {permissions} = dashboard;
const data = {
permissions,
};
const promise: Promise<DashboardDetails> = api.requestPromise(
`/organizations/${orgId}/dashboards/${dashboard.id}/`,
{
method: 'PUT',
data,
}
);

promise.catch(response => {
const errorResponse = response?.responseJSON ?? null;

if (errorResponse) {
const errors = flattenErrors(errorResponse, {});
addErrorMessage(errors[Object.keys(errors)[0]] as string);
} else {
addErrorMessage(t('Unable to update dashboard permissions'));
}
});

return promise;
}

export function validateWidget(
api: Client,
orgId: string,
Expand Down
9 changes: 8 additions & 1 deletion static/app/views/dashboards/controls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {trackAnalytics} from 'sentry/utils/analytics';
import {hasCustomMetrics} from 'sentry/utils/metrics/features';
import useOrganization from 'sentry/utils/useOrganization';
import {useUser} from 'sentry/utils/useUser';
import {useUserTeams} from 'sentry/utils/useUserTeams';
import {AddWidgetButton} from 'sentry/views/dashboards/addWidget';
import EditAccessSelector from 'sentry/views/dashboards/editAccessSelector';
import {DataSet} from 'sentry/views/dashboards/widgetBuilder/utils';
Expand Down Expand Up @@ -71,6 +72,7 @@ function Controls({

const organization = useOrganization();
const currentUser = useUser();
const {teams: userTeams} = useUserTeams();

if ([DashboardState.EDIT, DashboardState.PENDING_DELETE].includes(dashboardState)) {
return (
Expand Down Expand Up @@ -147,7 +149,12 @@ function Controls({

let hasEditAccess = true;
if (organization.features.includes('dashboards-edit-access')) {
hasEditAccess = checkUserHasEditAccess(dashboard, currentUser, organization);
hasEditAccess = checkUserHasEditAccess(
dashboard,
currentUser,
userTeams,
organization
);
}

return (
Expand Down
122 changes: 103 additions & 19 deletions static/app/views/dashboards/detail.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {OrganizationFixture} from 'sentry-fixture/organization';
import {ProjectFixture} from 'sentry-fixture/project';
import {ReleaseFixture} from 'sentry-fixture/release';
import {RouteComponentPropsFixture} from 'sentry-fixture/routeComponentPropsFixture';
import {TeamFixture} from 'sentry-fixture/team';
import {UserFixture} from 'sentry-fixture/user';
import {WidgetFixture} from 'sentry-fixture/widget';

Expand All @@ -21,6 +22,7 @@ import * as modals from 'sentry/actionCreators/modal';
import ConfigStore from 'sentry/stores/configStore';
import PageFiltersStore from 'sentry/stores/pageFiltersStore';
import ProjectsStore from 'sentry/stores/projectsStore';
import TeamStore from 'sentry/stores/teamStore';
import {browserHistory} from 'sentry/utils/browserHistory';
import CreateDashboard from 'sentry/views/dashboards/create';
import {handleUpdateDashboardSplit} from 'sentry/views/dashboards/detail';
Expand Down Expand Up @@ -1667,7 +1669,7 @@ describe('Dashboards > Detail', function () {

await userEvent.click(await screen.findByText('Edit Access:'));
expect(screen.getByText('Creator')).toBeInTheDocument();
expect(screen.getByText('Everyone')).toBeInTheDocument();
expect(screen.getByText('All users')).toBeInTheDocument();
});

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

// deselects 'Everyone' so only creator has edit access
expect(await screen.findByText('Everyone')).toBeEnabled();
expect(await screen.findByRole('option', {name: 'Everyone'})).toHaveAttribute(
// deselects 'All users' so only creator has edit access
expect(await screen.findByText('All users')).toBeEnabled();
expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute(
'aria-selected',
'true'
);
await userEvent.click(screen.getByRole('option', {name: 'Everyone'}));
expect(await screen.findByRole('option', {name: 'Everyone'})).toHaveAttribute(
await userEvent.click(screen.getByRole('option', {name: 'All users'}));
expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute(
'aria-selected',
'false'
);

// clicks out of dropdown to trigger onClose()
await userEvent.click(await screen.findByText('Edit Access:'));
await userEvent.click(await screen.findByText('Save Changes'));

await waitFor(() => {
expect(mockPUT).toHaveBeenCalledTimes(1);
expect(mockPUT).toHaveBeenCalledWith(
'/organizations/org-slug/dashboards/1/',
expect.objectContaining({
data: expect.objectContaining({
permissions: {isCreatorOnlyEditable: true},
permissions: {isEditableByEveryone: false, teamsWithEditAccess: []},
}),
})
);
Expand All @@ -1739,7 +1740,7 @@ describe('Dashboards > Detail', function () {
id: '1',
title: 'Custom Errors',
createdBy: UserFixture({id: '781629'}),
permissions: {isCreatorOnlyEditable: true},
permissions: {isEditableByEveryone: false},
}),
});

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

// selects 'Everyone' so everyone has edit access
expect(await screen.findByText('Everyone')).toBeEnabled();
expect(await screen.findByRole('option', {name: 'Everyone'})).toHaveAttribute(
// selects 'All users' so everyone has edit access
expect(await screen.findByText('All users')).toBeEnabled();
expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute(
'aria-selected',
'false'
);
await userEvent.click(screen.getByRole('option', {name: 'Everyone'}));
expect(await screen.findByRole('option', {name: 'Everyone'})).toHaveAttribute(
await userEvent.click(screen.getByRole('option', {name: 'All users'}));
expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute(
'aria-selected',
'true'
);

// clicks out of dropdown to trigger onClose()
await userEvent.click(await screen.findByText('Save Changes'));

await waitFor(() => {
expect(mockPUT).toHaveBeenCalledTimes(1);
expect(mockPUT).toHaveBeenCalledWith(
'/organizations/org-slug/dashboards/1/',
expect.objectContaining({
data: expect.objectContaining({
permissions: {isEditableByEveryone: true, teamsWithEditAccess: []},
}),
})
);
});
});

it('creator can update permissions with teams for dashboard', async function () {
const mockPUT = MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
method: 'PUT',
body: DashboardFixture([], {id: '1', title: 'Custom Errors'}),
});
MockApiClient.addMockResponse({
url: '/organizations/org-slug/dashboards/1/',
body: DashboardFixture([], {
id: '1',
title: 'Custom Errors',
createdBy: UserFixture({id: '781629'}),
permissions: {isEditableByEveryone: false},
}),
});

const currentUser = UserFixture({id: '781629'});
ConfigStore.set('user', currentUser);

const teamData = [
{
id: '1',
slug: 'team1',
name: 'Team 1',
},
{
id: '2',
slug: 'team2',
name: 'Team 2',
},
{
id: '3',
slug: 'team3',
name: 'Team 3',
},
];
const teams = teamData.map(data => TeamFixture(data));

TeamStore.loadInitialData(teams);

render(
<ViewEditDashboard
{...RouteComponentPropsFixture()}
organization={{
...initialData.organization,
features: ['dashboards-edit-access', ...initialData.organization.features],
}}
params={{orgId: 'org-slug', dashboardId: '1'}}
router={initialData.router}
location={initialData.router.location}
>
{null}
</ViewEditDashboard>,
{
router: initialData.router,
organization: {
features: ['dashboards-edit-access', ...initialData.organization.features],
},
}
);
await userEvent.click(await screen.findByText('Edit Access:'));

expect(await screen.findByText('All users')).toBeEnabled();
expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute(
'aria-selected',
'false'
);
await userEvent.click(screen.getByRole('option', {name: '#team1'}));
await userEvent.click(screen.getByRole('option', {name: '#team2'}));
await userEvent.click(await screen.findByText('Save Changes'));

await waitFor(() => {
expect(mockPUT).toHaveBeenCalledTimes(1);
expect(mockPUT).toHaveBeenCalledWith(
'/organizations/org-slug/dashboards/1/',
expect.objectContaining({
data: expect.objectContaining({
permissions: {isCreatorOnlyEditable: false},
permissions: {isEditableByEveryone: false, teamsWithEditAccess: [1, 2]},
}),
})
);
Expand All @@ -1804,7 +1888,7 @@ describe('Dashboards > Detail', function () {
id: '1',
title: 'Custom Errors',
createdBy: UserFixture({id: '238900'}),
permissions: {isCreatorOnlyEditable: true},
permissions: {isEditableByEveryone: false},
}),
],
});
Expand All @@ -1814,7 +1898,7 @@ describe('Dashboards > Detail', function () {
id: '1',
title: 'Custom Errors',
createdBy: UserFixture({id: '238900'}),
permissions: {isCreatorOnlyEditable: true},
permissions: {isEditableByEveryone: false},
}),
});

Expand Down
20 changes: 14 additions & 6 deletions static/app/views/dashboards/detail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
createDashboard,
deleteDashboard,
updateDashboard,
updateDashboardPermissions,
} from 'sentry/actionCreators/dashboards';
import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator';
import {openWidgetViewerModal} from 'sentry/actionCreators/modal';
Expand All @@ -28,7 +29,7 @@ import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import type {PageFilters} from 'sentry/types/core';
import type {PlainRoute, RouteComponentProps} from 'sentry/types/legacyReactRouter';
import type {Organization} from 'sentry/types/organization';
import type {Organization, Team} from 'sentry/types/organization';
import type {Project} from 'sentry/types/project';
import {defined} from 'sentry/utils';
import {trackAnalytics} from 'sentry/utils/analytics';
Expand Down Expand Up @@ -173,17 +174,24 @@ export function handleUpdateDashboardSplit({
export function checkUserHasEditAccess(
dashboard: DashboardDetails,
currentUser: User,
userTeams: Team[],
organization: Organization
): boolean {
if (
!organization.features.includes('dashboards-edit-access') ||
!dashboard.permissions
!dashboard.permissions ||
dashboard.permissions.isEditableByEveryone ||
dashboard.createdBy?.id === currentUser.id
) {
return true;
}
return dashboard.permissions.isCreatorOnlyEditable
? dashboard.createdBy?.id === currentUser.id
: !dashboard.permissions.isCreatorOnlyEditable;
if (dashboard.permissions.teamsWithEditAccess?.length) {
const userTeamIds = userTeams.map(team => Number(team.id));
dashboard.permissions.teamsWithEditAccess.some(teamId =>
userTeamIds.includes(teamId)
);
}
return false;
}

class DashboardDetail extends Component<Props, State> {
Expand Down Expand Up @@ -622,7 +630,7 @@ class DashboardDetail extends Component<Props, State> {
const dashboardCopy = cloneDashboard(dashboard);
dashboardCopy.permissions = newDashboardPermissions;

updateDashboard(api, organization.slug, dashboardCopy).then(
updateDashboardPermissions(api, organization.slug, dashboardCopy).then(
(newDashboard: DashboardDetails) => {
addSuccessMessage(t('Dashboard Edit Access updated.'));
this.props.onDashboardUpdate?.(newDashboard);
Expand Down
Loading
Loading