From 6fec67e6e11f57131759bf2323a8aa2f6123fa95 Mon Sep 17 00:00:00 2001 From: harshithadurai Date: Wed, 6 Nov 2024 17:17:13 -0500 Subject: [PATCH 1/7] add is_editable_by_everyone --- static/app/views/dashboards/detail.spec.tsx | 10 +++++----- .../app/views/dashboards/editAccessSelector.spec.tsx | 6 +++--- static/app/views/dashboards/types.tsx | 2 ++ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/static/app/views/dashboards/detail.spec.tsx b/static/app/views/dashboards/detail.spec.tsx index 138a517f71f4a2..c3b2e137738663 100644 --- a/static/app/views/dashboards/detail.spec.tsx +++ b/static/app/views/dashboards/detail.spec.tsx @@ -1716,7 +1716,7 @@ describe('Dashboards > Detail', function () { '/organizations/org-slug/dashboards/1/', expect.objectContaining({ data: expect.objectContaining({ - permissions: {isCreatorOnlyEditable: true}, + permissions: {isEditablebyEveryone: false}, }), }) ); @@ -1735,7 +1735,7 @@ describe('Dashboards > Detail', function () { id: '1', title: 'Custom Errors', createdBy: UserFixture({id: '781629'}), - permissions: {isCreatorOnlyEditable: true}, + permissions: {isCreatorOnlyEditable: true, isEditablebyEveryone: false}, }), }); @@ -1785,7 +1785,7 @@ describe('Dashboards > Detail', function () { '/organizations/org-slug/dashboards/1/', expect.objectContaining({ data: expect.objectContaining({ - permissions: {isCreatorOnlyEditable: false}, + permissions: {isEditablebyEveryone: true}, }), }) ); @@ -1800,7 +1800,7 @@ describe('Dashboards > Detail', function () { id: '1', title: 'Custom Errors', createdBy: UserFixture({id: '238900'}), - permissions: {isCreatorOnlyEditable: true}, + permissions: {isCreatorOnlyEditable: true, isEditablebyEveryone: false}, }), ], }); @@ -1810,7 +1810,7 @@ describe('Dashboards > Detail', function () { id: '1', title: 'Custom Errors', createdBy: UserFixture({id: '238900'}), - permissions: {isCreatorOnlyEditable: true}, + permissions: {isCreatorOnlyEditable: true, isEditablebyEveryone: false}, }), }); diff --git a/static/app/views/dashboards/editAccessSelector.spec.tsx b/static/app/views/dashboards/editAccessSelector.spec.tsx index 1792fb41e85b40..09a48cbcf094ef 100644 --- a/static/app/views/dashboards/editAccessSelector.spec.tsx +++ b/static/app/views/dashboards/editAccessSelector.spec.tsx @@ -93,7 +93,7 @@ describe('When EditAccessSelector is rendered', () => { id: '1', createdBy: UserFixture({id: '1'}), title: 'Custom Errors', - permissions: {isCreatorOnlyEditable: false}, // set to false + permissions: {isCreatorOnlyEditable: false, isEditablebyEveryone: true}, // set to false }); renderTestComponent(initialData, mockDashboard); await screen.findByText('Edit Access:'); @@ -105,7 +105,7 @@ describe('When EditAccessSelector is rendered', () => { id: '1', createdBy: UserFixture({id: '1'}), title: 'Custom Errors', - permissions: {isCreatorOnlyEditable: true}, // set to false + permissions: {isCreatorOnlyEditable: true, isEditablebyEveryone: false}, // set to false }); renderTestComponent(initialData, mockDashboard); await userEvent.click(await screen.findByText('Edit Access:')); @@ -134,7 +134,7 @@ describe('When EditAccessSelector is rendered', () => { id: '1', createdBy: UserFixture({id: '1', name: 'Lorem Ipsum'}), title: 'Custom Errors', - permissions: {isCreatorOnlyEditable: true}, // set to true + permissions: {isCreatorOnlyEditable: true, isEditablebyEveryone: false}, // set to true }); renderTestComponent(initialData, mockDashboard); await screen.findByText('Edit Access:'); diff --git a/static/app/views/dashboards/types.tsx b/static/app/views/dashboards/types.tsx index e1c5ddc62393b8..3c607a8d2dad8e 100644 --- a/static/app/views/dashboards/types.tsx +++ b/static/app/views/dashboards/types.tsx @@ -113,6 +113,8 @@ export type WidgetPreview = { export type DashboardPermissions = { isCreatorOnlyEditable: boolean; + isEditablebyEveryone: boolean; + teamsWithEditAccess?: number[]; }; /** From 207c98d283c8d250d5497bded6b2fc8d54e151c5 Mon Sep 17 00:00:00 2001 From: harshithadurai Date: Thu, 7 Nov 2024 10:02:54 -0500 Subject: [PATCH 2/7] teams + refactor is_everyone_editable + add new updatedashbaordpermissions function --- static/app/actionCreators/dashboards.tsx | 67 +++--- static/app/views/dashboards/detail.tsx | 9 +- .../dashboards/editAccessSelector.spec.tsx | 20 +- .../views/dashboards/editAccessSelector.tsx | 213 ++++++++++++------ static/app/views/dashboards/types.tsx | 3 +- 5 files changed, 207 insertions(+), 105 deletions(-) diff --git a/static/app/actionCreators/dashboards.tsx b/static/app/actionCreators/dashboards.tsx index 3bce4ceea61ce5..e2224d0a6b41f9 100644 --- a/static/app/actionCreators/dashboards.tsx +++ b/static/app/actionCreators/dashboards.tsx @@ -42,18 +42,8 @@ export function createDashboard( newDashboard: DashboardDetails, duplicate?: boolean ): Promise { - 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 = api.requestPromise( `/organizations/${orgSlug}/dashboards/`, @@ -70,7 +60,6 @@ export function createDashboard( end, filters, utc, - permissions, }, query: { project: projects, @@ -138,18 +127,8 @@ export function updateDashboard( orgId: string, dashboard: DashboardDetails ): Promise { - 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'])), @@ -160,7 +139,6 @@ export function updateDashboard( end, filters, utc, - permissions, }; const promise: Promise = api.requestPromise( @@ -239,6 +217,43 @@ export function validateWidgetRequest( ] as const; } +export function updateDashboardPermissions( + api: Client, + orgId: string, + dashboard: DashboardDetails +): Promise { + const {projects, environment, permissions} = dashboard; + const data = { + projects, + environment, + permissions, + }; + const promise: Promise = api.requestPromise( + `/organizations/${orgId}/dashboards/${dashboard.id}/`, + { + method: 'PUT', + data, + query: { + project: projects, + environment, + }, + } + ); + + 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, diff --git a/static/app/views/dashboards/detail.tsx b/static/app/views/dashboards/detail.tsx index 6df6432a3be62f..212a6d2c047345 100644 --- a/static/app/views/dashboards/detail.tsx +++ b/static/app/views/dashboards/detail.tsx @@ -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'; @@ -181,9 +182,9 @@ export function checkUserHasEditAccess( ) { return true; } - return dashboard.permissions.isCreatorOnlyEditable - ? dashboard.createdBy?.id === currentUser.id - : !dashboard.permissions.isCreatorOnlyEditable; + return dashboard.permissions.isEditableByEveryone + ? dashboard.permissions.isEditableByEveryone + : dashboard.createdBy?.id === currentUser.id; } class DashboardDetail extends Component { @@ -622,7 +623,7 @@ class DashboardDetail extends Component { 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); diff --git a/static/app/views/dashboards/editAccessSelector.spec.tsx b/static/app/views/dashboards/editAccessSelector.spec.tsx index 09a48cbcf094ef..27cbfb0083d8b5 100644 --- a/static/app/views/dashboards/editAccessSelector.spec.tsx +++ b/static/app/views/dashboards/editAccessSelector.spec.tsx @@ -79,7 +79,7 @@ describe('When EditAccessSelector is rendered', () => { 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('renders All badge when dashboards has no perms defined', async function () { @@ -93,7 +93,7 @@ describe('When EditAccessSelector is rendered', () => { id: '1', createdBy: UserFixture({id: '1'}), title: 'Custom Errors', - permissions: {isCreatorOnlyEditable: false, isEditablebyEveryone: true}, // set to false + permissions: {isEditableByEveryone: true}, // set to true }); renderTestComponent(initialData, mockDashboard); await screen.findByText('Edit Access:'); @@ -105,7 +105,7 @@ describe('When EditAccessSelector is rendered', () => { id: '1', createdBy: UserFixture({id: '1'}), title: 'Custom Errors', - permissions: {isCreatorOnlyEditable: true, isEditablebyEveryone: false}, // set to false + permissions: {isEditableByEveryone: false}, // set to false }); renderTestComponent(initialData, mockDashboard); await userEvent.click(await screen.findByText('Edit Access:')); @@ -113,12 +113,12 @@ describe('When EditAccessSelector is rendered', () => { expect(screen.queryByText('All')).not.toBeInTheDocument(); // Select everyone - expect(await screen.findByRole('option', {name: 'Everyone'})).toHaveAttribute( + 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' ); @@ -134,7 +134,7 @@ describe('When EditAccessSelector is rendered', () => { id: '1', createdBy: UserFixture({id: '1', name: 'Lorem Ipsum'}), title: 'Custom Errors', - permissions: {isCreatorOnlyEditable: true, isEditablebyEveryone: false}, // set to true + permissions: {isEditableByEveryone: false}, // set to true }); renderTestComponent(initialData, mockDashboard); await screen.findByText('Edit Access:'); @@ -150,12 +150,12 @@ describe('When EditAccessSelector is rendered', () => { await userEvent.click(await screen.findByText('Edit Access:')); // Everyone option should be disabled - expect(await screen.findByRole('option', {name: 'Everyone'})).toHaveAttribute( + 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', 'true' ); diff --git a/static/app/views/dashboards/editAccessSelector.tsx b/static/app/views/dashboards/editAccessSelector.tsx index 0bcb28d90fde0f..5026386c7e0081 100644 --- a/static/app/views/dashboards/editAccessSelector.tsx +++ b/static/app/views/dashboards/editAccessSelector.tsx @@ -3,14 +3,18 @@ import styled from '@emotion/styled'; import isEqual from 'lodash/isEqual'; import AvatarList from 'sentry/components/avatar/avatarList'; +import TeamAvatar from 'sentry/components/avatar/teamAvatar'; import Badge from 'sentry/components/badge/badge'; -import Checkbox from 'sentry/components/checkbox'; +import {Button} from 'sentry/components/button'; +import ButtonBar from 'sentry/components/buttonBar'; import {CompactSelect} from 'sentry/components/compactSelect'; import {CheckWrap} from 'sentry/components/compactSelect/styles'; import UserBadge from 'sentry/components/idBadge/userBadge'; import {InnerWrap, LeadingItems} from 'sentry/components/menuListItem'; import {Tooltip} from 'sentry/components/tooltip'; import {t, tct} from 'sentry/locale'; +import {space} from 'sentry/styles/space'; +import type {Team} from 'sentry/types/organization'; import type {User} from 'sentry/types/user'; import {defined} from 'sentry/utils'; import {useTeamsById} from 'sentry/utils/useTeamsById'; @@ -29,13 +33,58 @@ interface EditAccessSelectorProps { function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorProps) { const currentUser: User = useUser(); const dashboardCreator: User | undefined = dashboard.createdBy; + const isCurrentUserDashboardOwner = dashboardCreator?.id === currentUser.id; const {teams} = useTeamsById(); const teamIds: string[] = Object.values(teams).map(team => team.id); - const [selectedOptions, setselectedOptions] = useState( - dashboard.permissions?.isCreatorOnlyEditable - ? ['_creator'] - : ['_everyone', '_creator', ...teamIds] - ); + const [selectedOptions, setSelectedOptions] = useState(getSelectedOptions()); + const [isMenuOpen, setMenuOpen] = useState(false); + const [hasUnsavedChanges, setHasUnsavedChanges] = useState(false); + + // Handles state change when dropdown options are selected + const onSelectOptions = newSelectedOptions => { + let newSelectedValues = newSelectedOptions.map( + (option: {value: string}) => option.value + ); + const areAllTeamsSelected = teamIds.every(teamId => + newSelectedValues.includes(teamId) + ); + const isAllUsersOptionSelected = selectedOptions.includes('_allUsers'); + + if (!isAllUsersOptionSelected && newSelectedValues.includes('_allUsers')) { + newSelectedValues = ['_creator', '_allUsers', ...teamIds]; + } else if (isAllUsersOptionSelected && !newSelectedValues.includes('_allUsers')) { + newSelectedValues = ['_creator']; + } else if (!areAllTeamsSelected) { + newSelectedValues = newSelectedValues.filter(value => value !== '_allUsers'); + } else if (areAllTeamsSelected) { + newSelectedValues = ['_creator', '_allUsers', ...teamIds]; + } + + setSelectedOptions(newSelectedValues); + }; + + // Creates a permissions object based on the options selected + function getDashboardPermissions() { + return { + isEditableByEveryone: selectedOptions.includes('_allUsers'), + teamsWithEditAccess: selectedOptions.includes('_allUsers') + ? undefined + : selectedOptions + .filter(option => option !== '_creator') + .map(teamId => parseInt(teamId, 10)), + }; + } + + // memo? + function getSelectedOptions(): string[] { + if (!defined(dashboard.permissions) || dashboard.permissions.isEditableByEveryone) { + return ['_creator', '_allUsers', ...teamIds]; + } + const permittedTeamIds = + dashboard.permissions.teamsWithEditAccess?.map(teamId => String(teamId)) ?? []; + return ['_creator', ...permittedTeamIds]; + } + // Dashboard creator option in the dropdown const makeCreatorOption = () => ({ value: '_creator', @@ -55,97 +104,124 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP /> ), textValue: `creator_${currentUser.email}`, - disabled: dashboardCreator?.id !== currentUser.id, - // Creator option is always disabled - leadingItems: , - hideCheck: true, + disabled: true, }); - // Single team option in the dropdown [WIP] - // const makeTeamOption = (team: Team) => ({ - // value: team.id, - // label: `#${team.slug}`, - // leadingItems: , - // }); + // Single team option in the dropdown + const makeTeamOption = (team: Team) => ({ + value: team.id, + label: `#${team.slug}`, + leadingItems: , + }); - // Avatars/Badges in the Edit Selector Button + // Avatars/Badges in the Edit Access Selector Button const triggerAvatars = - selectedOptions.includes('_everyone') || !dashboardCreator ? ( + selectedOptions.includes('_allUsers') || !dashboardCreator ? ( ) : ( - + option !== '_creator') + .map(option => teams.find(team => team.id === option)) + .filter((team): team is Team => team !== undefined) + : [] + } + maxVisibleAvatars={1} + avatarSize={25} + /> ); - const dropdownOptions = [ + const allDropdownOptions = [ makeCreatorOption(), { - value: '_everyone_section', + value: '_sall_user_section', options: [ { - value: '_everyone', - label: t('Everyone'), - disabled: dashboardCreator?.id !== currentUser.id, + value: '_allUsers', + label: t('All users'), + disabled: !isCurrentUserDashboardOwner, }, ], }, - // [WIP: Selective edit access to teams] - // { - // value: '_teams', - // label: t('Teams'), - // options: teams.map(makeTeamOption), - // showToggleAllButton: true, - // disabled: true, - // }, + { + value: '_teams', + label: t('Teams'), + options: teams.map(makeTeamOption), + showToggleAllButton: isCurrentUserDashboardOwner, + disabled: !isCurrentUserDashboardOwner, + }, ]; - // Handles state change when dropdown options are selected - const onSelectOptions = newSelectedOptions => { - const newSelectedValues = newSelectedOptions.map( - (option: {value: string}) => option.value - ); - if (newSelectedValues.includes('_everyone')) { - setselectedOptions(['_everyone', '_creator', ...teamIds]); - } else if (!newSelectedValues.includes('_everyone')) { - setselectedOptions(['_creator']); - } - }; - - // Creates or modifies permissions object based on the options selected - function getDashboardPermissions() { - return { - isCreatorOnlyEditable: !selectedOptions.includes('_everyone'), - }; - } + // Save and Cancel Buttons + const dropdownFooterButtons = ( + + + + + ); const dropdownMenu = ( { onSelectOptions(newSelectedOptions); - }} - onClose={() => { - const isDefaultState = - !defined(dashboard.permissions) && selectedOptions.includes('_everyone'); - const newDashboardPermissions = getDashboardPermissions(); - if (!isDefaultState && !isEqual(newDashboardPermissions, dashboard.permissions)) { - onChangeEditAccess?.(newDashboardPermissions); - } + setHasUnsavedChanges(true); }} multiple searchable - options={dropdownOptions} + options={allDropdownOptions} value={selectedOptions} triggerLabel={[t('Edit Access:'), triggerAvatars]} searchPlaceholder={t('Search Teams')} + isOpen={isMenuOpen} + onOpenChange={() => { + setSelectedOptions(getSelectedOptions()); + setMenuOpen(!isMenuOpen); + setHasUnsavedChanges(false); + }} + menuFooter={dropdownFooterButtons} /> ); - return dashboardCreator?.id !== currentUser.id ? ( - + return isCurrentUserDashboardOwner ? ( + dropdownMenu + ) : ( + {dropdownMenu} - ) : ( - dropdownMenu ); } @@ -177,4 +253,15 @@ const StyledBadge = styled(Badge)` color: ${p => p.theme.white}; background: ${p => p.theme.purple300}; margin-right: 3px; + padding: 0; + height: 20px; + width: 20px; +`; + +const FilterButtons = styled(ButtonBar)` + display: grid; + gap: ${space(1.5)}; + margin-top: ${space(0.5)}; + margin-bottom: ${space(0.5)}; + justify-content: flex-end; `; diff --git a/static/app/views/dashboards/types.tsx b/static/app/views/dashboards/types.tsx index 3c607a8d2dad8e..dd13cb4350ee5d 100644 --- a/static/app/views/dashboards/types.tsx +++ b/static/app/views/dashboards/types.tsx @@ -112,8 +112,7 @@ export type WidgetPreview = { }; export type DashboardPermissions = { - isCreatorOnlyEditable: boolean; - isEditablebyEveryone: boolean; + isEditableByEveryone: boolean; teamsWithEditAccess?: number[]; }; From 2be78fa875962bb1095df71dd90083e1b48ebefc Mon Sep 17 00:00:00 2001 From: harshithadurai Date: Thu, 7 Nov 2024 15:04:33 -0500 Subject: [PATCH 3/7] add/update tests --- static/app/views/dashboards/detail.spec.tsx | 122 ++++++++++-- .../dashboards/editAccessSelector.spec.tsx | 174 +++++++++++++----- .../views/dashboards/editAccessSelector.tsx | 2 +- 3 files changed, 235 insertions(+), 63 deletions(-) diff --git a/static/app/views/dashboards/detail.spec.tsx b/static/app/views/dashboards/detail.spec.tsx index f743215f90a616..5f9d5d2191ff0f 100644 --- a/static/app/views/dashboards/detail.spec.tsx +++ b/static/app/views/dashboards/detail.spec.tsx @@ -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'; @@ -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'; @@ -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 () { @@ -1699,20 +1701,19 @@ 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); @@ -1720,7 +1721,7 @@ describe('Dashboards > Detail', function () { '/organizations/org-slug/dashboards/1/', expect.objectContaining({ data: expect.objectContaining({ - permissions: {isEditablebyEveryone: false}, + permissions: {isEditableByEveryone: false, teamsWithEditAccess: []}, }), }) ); @@ -1739,7 +1740,7 @@ describe('Dashboards > Detail', function () { id: '1', title: 'Custom Errors', createdBy: UserFixture({id: '781629'}), - permissions: {isCreatorOnlyEditable: true, isEditablebyEveryone: false}, + permissions: {isEditableByEveryone: false}, }), }); @@ -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( + + {null} + , + { + 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: {isEditablebyEveryone: true}, + permissions: {isEditableByEveryone: false, teamsWithEditAccess: [1, 2]}, }), }) ); @@ -1804,7 +1888,7 @@ describe('Dashboards > Detail', function () { id: '1', title: 'Custom Errors', createdBy: UserFixture({id: '238900'}), - permissions: {isCreatorOnlyEditable: true, isEditablebyEveryone: false}, + permissions: {isEditableByEveryone: false}, }), ], }); @@ -1814,7 +1898,7 @@ describe('Dashboards > Detail', function () { id: '1', title: 'Custom Errors', createdBy: UserFixture({id: '238900'}), - permissions: {isCreatorOnlyEditable: true, isEditablebyEveryone: false}, + permissions: {isEditableByEveryone: false}, }), }); diff --git a/static/app/views/dashboards/editAccessSelector.spec.tsx b/static/app/views/dashboards/editAccessSelector.spec.tsx index 27cbfb0083d8b5..5410bbc17c1c4e 100644 --- a/static/app/views/dashboards/editAccessSelector.spec.tsx +++ b/static/app/views/dashboards/editAccessSelector.spec.tsx @@ -1,46 +1,26 @@ import {DashboardFixture} from 'sentry-fixture/dashboard'; -import {LocationFixture} from 'sentry-fixture/locationFixture'; -import {OrganizationFixture} from 'sentry-fixture/organization'; +// import {LocationFixture} from 'sentry-fixture/locationFixture'; +import {TeamFixture} from 'sentry-fixture/team'; import {UserFixture} from 'sentry-fixture/user'; -import {initializeOrg} from 'sentry-test/initializeOrg'; +// import {initializeOrg} from 'sentry-test/initializeOrg'; import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; -import ConfigStore from 'sentry/stores/configStore'; - -import EditAccessSelector from './editAccessSelector'; +import TeamStore from 'sentry/stores/teamStore'; +import EditAccessSelector from 'sentry/views/dashboards/editAccessSelector'; function renderTestComponent( - initialData, mockDashboard = DashboardFixture([], { id: '1', title: 'test dashboard 2', createdBy: UserFixture({id: '35478'}), }) ) { - render( - , - { - router: initialData.router, - organization: { - user: UserFixture({id: '1'}), - features: ['dashboards-edit-access', 'dashboards-edit'], - ...initialData.organization, - }, - } - ); + render(); } -describe('When EditAccessSelector is rendered', () => { - let initialData; - const organization = OrganizationFixture({}); +describe('When EditAccessSelector is rendered with no Teams', () => { beforeEach(() => { - initialData = initializeOrg({ - organization, - router: { - location: LocationFixture(), - }, - }); MockApiClient.addMockResponse({ url: '/organizations/org-slug/dashboards/', body: [ @@ -75,7 +55,7 @@ describe('When EditAccessSelector is rendered', () => { }); it('renders with creator and everyone options', async function () { - renderTestComponent(initialData); + renderTestComponent(); await userEvent.click(await screen.findByText('Edit Access:')); expect(screen.getByText('Creator')).toBeInTheDocument(); @@ -83,7 +63,7 @@ describe('When EditAccessSelector is rendered', () => { }); it('renders All badge when dashboards has no perms defined', async function () { - renderTestComponent(initialData); + renderTestComponent(); await userEvent.click(await screen.findByText('Edit Access:')); expect(screen.getByText('All')).toBeInTheDocument(); }); @@ -95,19 +75,19 @@ describe('When EditAccessSelector is rendered', () => { title: 'Custom Errors', permissions: {isEditableByEveryone: true}, // set to true }); - renderTestComponent(initialData, mockDashboard); + renderTestComponent(mockDashboard); await screen.findByText('Edit Access:'); expect(screen.getByText('All')).toBeInTheDocument(); }); - it('renders All badge when everyone is selected', async function () { + it('renders All badge when All users is selected', async function () { const mockDashboard = DashboardFixture([], { id: '1', createdBy: UserFixture({id: '1'}), title: 'Custom Errors', permissions: {isEditableByEveryone: false}, // set to false }); - renderTestComponent(initialData, mockDashboard); + renderTestComponent(mockDashboard); await userEvent.click(await screen.findByText('Edit Access:')); expect(screen.queryByText('All')).not.toBeInTheDocument(); @@ -127,8 +107,8 @@ describe('When EditAccessSelector is rendered', () => { }); it('renders User badge when creator-only is selected', async function () { - const currentUser = UserFixture({id: '781629', name: 'John Doe'}); - ConfigStore.set('user', currentUser); + // const currentUser = UserFixture({id: '781629', name: 'John Doe'}); + // ConfigStore.set('user', currentUser); const mockDashboard = DashboardFixture([], { id: '1', @@ -136,17 +116,17 @@ describe('When EditAccessSelector is rendered', () => { title: 'Custom Errors', permissions: {isEditableByEveryone: false}, // set to true }); - renderTestComponent(initialData, mockDashboard); + renderTestComponent(mockDashboard); await screen.findByText('Edit Access:'); expect(screen.getByText('LI')).toBeInTheDocument(); // dashboard owner's initials expect(screen.queryByText('All')).not.toBeInTheDocument(); }); it('disables dropdown options when current user is not dashboard creator', async function () { - const currentUser = UserFixture({id: '781629'}); - ConfigStore.set('user', currentUser); + // const currentUser = UserFixture({id: '781629'}); + // ConfigStore.set('user', currentUser); - renderTestComponent(initialData); + renderTestComponent(); await userEvent.click(await screen.findByText('Edit Access:')); // Everyone option should be disabled @@ -160,10 +140,118 @@ describe('When EditAccessSelector is rendered', () => { 'true' ); }); +}); + +const teamData = [ + { + id: '1', + slug: 'team1', + name: 'Team 1', + }, + { + id: '2', + slug: 'team2', + name: 'Team 2', + }, + { + id: '3', + slug: 'team3', + name: 'Team 3', + }, +]; + +describe('When EditAccessSelector is rendered with Teams', function () { + const teams = teamData.map(data => TeamFixture(data)); + + beforeEach(function () { + TeamStore.loadInitialData(teams); + MockApiClient.addMockResponse({ + url: '/organizations/org-slug/dashboards/', + body: [ + { + ...DashboardFixture([], { + id: 'default-overview', + title: 'Default', + }), + }, + { + ...DashboardFixture([], { + id: '1', + title: 'test dashboard 2', + createdBy: UserFixture({id: '35478'}), + }), + }, + ], + }); + MockApiClient.addMockResponse({ + url: '/organizations/org-slug/dashboards/1/', + body: DashboardFixture([], { + id: '1', + title: 'Custom Errors', + filters: {}, + }), + }); + }); + + afterEach(() => { + MockApiClient.clearMockResponses(); + jest.clearAllMocks(); + }); - // [WIP] (Teams based access) - it('renders all teams', async function () {}); - it('selects all teams when everyone is selected', async function () {}); - it('retains team selection on re-opening selector', async function () {}); - it('makes a post request with success message when different teams are selected', async function () {}); + it('renders all teams', async function () { + renderTestComponent(); + await userEvent.click(await screen.findByText('Edit Access:')); + + expect(screen.getByText('Teams')).toBeInTheDocument(); + expect(screen.getByText('#team1')).toBeInTheDocument(); + expect(screen.getByText('#team2')).toBeInTheDocument(); + expect(screen.getByText('#team3')).toBeInTheDocument(); + }); + + it('selects all teams when all users is selected', async function () { + const mockDashboard = DashboardFixture([], { + id: '1', + createdBy: UserFixture({id: '1'}), + title: 'Custom Errors', + permissions: {isEditableByEveryone: false}, // set to false + }); + renderTestComponent(mockDashboard); + await userEvent.click(await screen.findByText('Edit Access:')); + + expect(await screen.findByRole('option', {name: '#team1'})).toHaveAttribute( + 'aria-selected', + 'false' + ); + expect(await screen.findByRole('option', {name: '#team2'})).toHaveAttribute( + 'aria-selected', + 'false' + ); + + // Select everyone + expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute( + 'aria-selected', + 'false' + ); + await userEvent.click(screen.getByRole('option', {name: 'All users'})); + expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute( + 'aria-selected', + 'true' + ); + expect(await screen.findByRole('option', {name: '#team1'})).toHaveAttribute( + 'aria-selected', + 'true' + ); + expect(await screen.findByRole('option', {name: '#team2'})).toHaveAttribute( + 'aria-selected', + 'true' + ); + }); + + it('searches teams', async function () { + renderTestComponent(); + await userEvent.click(await screen.findByText('Edit Access:')); + await userEvent.type(screen.getByPlaceholderText('Search Teams'), 'team2'); + + expect(screen.getByText('#team2')).toBeInTheDocument(); + }); }); diff --git a/static/app/views/dashboards/editAccessSelector.tsx b/static/app/views/dashboards/editAccessSelector.tsx index 5026386c7e0081..4eb4dcfb705045 100644 --- a/static/app/views/dashboards/editAccessSelector.tsx +++ b/static/app/views/dashboards/editAccessSelector.tsx @@ -68,7 +68,7 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP return { isEditableByEveryone: selectedOptions.includes('_allUsers'), teamsWithEditAccess: selectedOptions.includes('_allUsers') - ? undefined + ? [] : selectedOptions .filter(option => option !== '_creator') .map(teamId => parseInt(teamId, 10)), From 5314f0801ef1c51e2a9e121ca36cb295751a2a57 Mon Sep 17 00:00:00 2001 From: harshithadurai Date: Thu, 7 Nov 2024 15:09:48 -0500 Subject: [PATCH 4/7] remove test --- .../dashboards/editAccessSelector.spec.tsx | 26 +------------------ 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/static/app/views/dashboards/editAccessSelector.spec.tsx b/static/app/views/dashboards/editAccessSelector.spec.tsx index 5410bbc17c1c4e..becd22c7b77d35 100644 --- a/static/app/views/dashboards/editAccessSelector.spec.tsx +++ b/static/app/views/dashboards/editAccessSelector.spec.tsx @@ -1,9 +1,7 @@ import {DashboardFixture} from 'sentry-fixture/dashboard'; -// import {LocationFixture} from 'sentry-fixture/locationFixture'; import {TeamFixture} from 'sentry-fixture/team'; import {UserFixture} from 'sentry-fixture/user'; -// import {initializeOrg} from 'sentry-test/initializeOrg'; import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; import TeamStore from 'sentry/stores/teamStore'; @@ -107,39 +105,17 @@ describe('When EditAccessSelector is rendered with no Teams', () => { }); it('renders User badge when creator-only is selected', async function () { - // const currentUser = UserFixture({id: '781629', name: 'John Doe'}); - // ConfigStore.set('user', currentUser); - const mockDashboard = DashboardFixture([], { id: '1', createdBy: UserFixture({id: '1', name: 'Lorem Ipsum'}), title: 'Custom Errors', - permissions: {isEditableByEveryone: false}, // set to true + permissions: {isEditableByEveryone: false}, // set to false }); renderTestComponent(mockDashboard); await screen.findByText('Edit Access:'); expect(screen.getByText('LI')).toBeInTheDocument(); // dashboard owner's initials expect(screen.queryByText('All')).not.toBeInTheDocument(); }); - - it('disables dropdown options when current user is not dashboard creator', async function () { - // const currentUser = UserFixture({id: '781629'}); - // ConfigStore.set('user', currentUser); - - renderTestComponent(); - await userEvent.click(await screen.findByText('Edit Access:')); - - // Everyone option should be disabled - expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute( - 'aria-selected', - 'true' - ); - await userEvent.click(screen.getByRole('option', {name: 'All users'})); - expect(await screen.findByRole('option', {name: 'All users'})).toHaveAttribute( - 'aria-selected', - 'true' - ); - }); }); const teamData = [ From b9c3c0a07a3f1f79d16466a389c727663f0774d4 Mon Sep 17 00:00:00 2001 From: harshithadurai Date: Thu, 7 Nov 2024 15:52:53 -0500 Subject: [PATCH 5/7] refactor to make more readable --- .../views/dashboards/editAccessSelector.tsx | 54 +++++++++++-------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/static/app/views/dashboards/editAccessSelector.tsx b/static/app/views/dashboards/editAccessSelector.tsx index 4eb4dcfb705045..ee6b002d5d7eff 100644 --- a/static/app/views/dashboards/editAccessSelector.tsx +++ b/static/app/views/dashboards/editAccessSelector.tsx @@ -48,16 +48,23 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP const areAllTeamsSelected = teamIds.every(teamId => newSelectedValues.includes(teamId) ); - const isAllUsersOptionSelected = selectedOptions.includes('_allUsers'); - if (!isAllUsersOptionSelected && newSelectedValues.includes('_allUsers')) { + if ( + !selectedOptions.includes('_allUsers') && + newSelectedValues.includes('_allUsers') + ) { newSelectedValues = ['_creator', '_allUsers', ...teamIds]; - } else if (isAllUsersOptionSelected && !newSelectedValues.includes('_allUsers')) { + } else if ( + selectedOptions.includes('_allUsers') && + !newSelectedValues.includes('_allUsers') + ) { newSelectedValues = ['_creator']; - } else if (!areAllTeamsSelected) { - newSelectedValues = newSelectedValues.filter(value => value !== '_allUsers'); - } else if (areAllTeamsSelected) { - newSelectedValues = ['_creator', '_allUsers', ...teamIds]; + } else { + areAllTeamsSelected + ? // selecting all teams deselects 'all users' + (newSelectedValues = ['_creator', '_allUsers', ...teamIds]) + : // deselecting all teams deselects 'all users' + (newSelectedValues = newSelectedValues.filter(value => value !== '_allUsers')); } setSelectedOptions(newSelectedValues); @@ -75,7 +82,7 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP }; } - // memo? + // Gets selected options from the dropdown from dashboard object function getSelectedOptions(): string[] { if (!defined(dashboard.permissions) || dashboard.permissions.isEditableByEveryone) { return ['_creator', '_allUsers', ...teamIds]; @@ -118,21 +125,22 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP const triggerAvatars = selectedOptions.includes('_allUsers') || !dashboardCreator ? ( + ) : selectedOptions.length === 2 ? ( + // Case where we display 1 Creator Avatar + 1 Team Avatar + team.id === selectedOptions[1])!]} + maxVisibleAvatars={1} + avatarSize={25} + /> ) : ( + // Case where we display 1 Creator Avatar + a Badge with no. of teams selected option !== '_creator') - .map(option => teams.find(team => team.id === option)) - .filter((team): team is Team => team !== undefined) - : [] - } + users={Array(selectedOptions.length).fill(dashboardCreator)} maxVisibleAvatars={1} avatarSize={25} /> @@ -141,7 +149,7 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP const allDropdownOptions = [ makeCreatorOption(), { - value: '_sall_user_section', + value: '_all_users_section', options: [ { value: '_allUsers', @@ -208,9 +216,11 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP searchPlaceholder={t('Search Teams')} isOpen={isMenuOpen} onOpenChange={() => { - setSelectedOptions(getSelectedOptions()); setMenuOpen(!isMenuOpen); - setHasUnsavedChanges(false); + if (isMenuOpen) { + setSelectedOptions(getSelectedOptions()); + setHasUnsavedChanges(false); + } }} menuFooter={dropdownFooterButtons} /> From 42ed899331a0fc71c525e34e28aa4a461beaabc2 Mon Sep 17 00:00:00 2001 From: harshithadurai Date: Thu, 7 Nov 2024 16:00:50 -0500 Subject: [PATCH 6/7] update some commens --- static/app/views/dashboards/editAccessSelector.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/static/app/views/dashboards/editAccessSelector.tsx b/static/app/views/dashboards/editAccessSelector.tsx index ee6b002d5d7eff..fff1f46c1f5fa5 100644 --- a/static/app/views/dashboards/editAccessSelector.tsx +++ b/static/app/views/dashboards/editAccessSelector.tsx @@ -63,7 +63,7 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP areAllTeamsSelected ? // selecting all teams deselects 'all users' (newSelectedValues = ['_creator', '_allUsers', ...teamIds]) - : // deselecting all teams deselects 'all users' + : // deselecting any team deselects 'all users' (newSelectedValues = newSelectedValues.filter(value => value !== '_allUsers')); } @@ -82,7 +82,7 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP }; } - // Gets selected options from the dropdown from dashboard object + // Gets selected options for the dropdown from dashboard object function getSelectedOptions(): string[] { if (!defined(dashboard.permissions) || dashboard.permissions.isEditableByEveryone) { return ['_creator', '_allUsers', ...teamIds]; From 38576c8f20f05608ecddb34661b29eab9864d26b Mon Sep 17 00:00:00 2001 From: harshithadurai Date: Fri, 8 Nov 2024 13:10:51 -0500 Subject: [PATCH 7/7] minor changes --- static/app/actionCreators/dashboards.tsx | 8 +------- static/app/views/dashboards/controls.tsx | 9 ++++++++- static/app/views/dashboards/detail.tsx | 17 ++++++++++++----- .../app/views/dashboards/editAccessSelector.tsx | 6 +++++- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/static/app/actionCreators/dashboards.tsx b/static/app/actionCreators/dashboards.tsx index e2224d0a6b41f9..091381e7ac1b12 100644 --- a/static/app/actionCreators/dashboards.tsx +++ b/static/app/actionCreators/dashboards.tsx @@ -222,10 +222,8 @@ export function updateDashboardPermissions( orgId: string, dashboard: DashboardDetails ): Promise { - const {projects, environment, permissions} = dashboard; + const {permissions} = dashboard; const data = { - projects, - environment, permissions, }; const promise: Promise = api.requestPromise( @@ -233,10 +231,6 @@ export function updateDashboardPermissions( { method: 'PUT', data, - query: { - project: projects, - environment, - }, } ); diff --git a/static/app/views/dashboards/controls.tsx b/static/app/views/dashboards/controls.tsx index 36fea48dd0b974..3be5091d69d56e 100644 --- a/static/app/views/dashboards/controls.tsx +++ b/static/app/views/dashboards/controls.tsx @@ -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'; @@ -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 ( @@ -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 ( diff --git a/static/app/views/dashboards/detail.tsx b/static/app/views/dashboards/detail.tsx index 212a6d2c047345..3eefb7872b7684 100644 --- a/static/app/views/dashboards/detail.tsx +++ b/static/app/views/dashboards/detail.tsx @@ -29,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'; @@ -174,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.isEditableByEveryone - ? dashboard.permissions.isEditableByEveryone - : dashboard.createdBy?.id === currentUser.id; + 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 { diff --git a/static/app/views/dashboards/editAccessSelector.tsx b/static/app/views/dashboards/editAccessSelector.tsx index fff1f46c1f5fa5..6c72ed8ea59a96 100644 --- a/static/app/views/dashboards/editAccessSelector.tsx +++ b/static/app/views/dashboards/editAccessSelector.tsx @@ -174,10 +174,14 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP size="sm" onClick={() => { setMenuOpen(false); + if (isMenuOpen) { + setSelectedOptions(getSelectedOptions()); + setHasUnsavedChanges(false); + } }} disabled={!isCurrentUserDashboardOwner} > - {t('Cancel')}{' '} + {t('Cancel')}