Skip to content

chore(dashboards): Remove Edit Access feature flag references #83021

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nikkikapadia merged 1 commit into from
Jan 8, 2025
Merged

chore(dashboards): Remove Edit Access feature flag references #83021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 8 additions & 10 deletions static/app/components/modals/widgetViewerModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,16 +809,14 @@ function WidgetViewerModal(props: Props) {

const currentUser = useUser();
const {teams: userTeams} = useUserTeams();
let hasEditAccess = true;
if (organization.features.includes('dashboards-edit-access')) {
hasEditAccess = checkUserHasEditAccess(
currentUser,
userTeams,
organization,
dashboardPermissions,
dashboardCreator
);
}
const hasEditAccess = checkUserHasEditAccess(
currentUser,
userTeams,
organization,
dashboardPermissions,
dashboardCreator
);

function renderWidgetViewer() {
return (
<Fragment>
Expand Down
27 changes: 11 additions & 16 deletions static/app/views/dashboards/controls.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,16 +151,13 @@ function Controls({
? DataSet.ERRORS
: DataSet.EVENTS;

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

return (
<StyledButtonBar gap={1} key="controls">
Expand Down Expand Up @@ -213,12 +210,10 @@ function Controls({
</Feature>
)}
{dashboard.id !== 'default-overview' && (
<Feature features="dashboards-edit-access">
<EditAccessSelector
dashboard={dashboard}
onChangeEditAccess={onChangeEditAccess}
/>
</Feature>
<EditAccessSelector
dashboard={dashboard}
onChangeEditAccess={onChangeEditAccess}
/>
)}
<Button
data-test-id="dashboard-edit"
Expand Down
52 changes: 17 additions & 35 deletions static/app/views/dashboards/detail.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1049,7 +1049,7 @@ describe('Dashboards > Detail', function () {
await userEvent.click(screen.getByText('Last 7 days'));
await screen.findByText('7D');

expect(screen.queryByText('Cancel')).not.toBeInTheDocument();
expect(screen.queryByTestId('filter-bar-cancel')).not.toBeInTheDocument();
expect(screen.queryByText('Save')).not.toBeInTheDocument();
});

Expand Down Expand Up @@ -1336,7 +1336,7 @@ describe('Dashboards > Detail', function () {
await userEvent.click(screen.getByText('sentry-android-shop@1.2.0'));
await userEvent.keyboard('{Escape}');

await userEvent.click(screen.getByText('Cancel'));
await userEvent.click(screen.getByTestId('filter-bar-cancel'));
Copy link
Copy Markdown
Member Author

@nikkikapadia nikkikapadia Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to do this a bunch because the cancel button on the access selector was also getting picked up. This ensures it is the correct cancel button that we are testing for


screen.getByText('All Releases');
expect(testData.router.replace).toHaveBeenCalledWith(
Expand Down Expand Up @@ -1394,7 +1394,7 @@ describe('Dashboards > Detail', function () {
);

expect(await screen.findByText('Save')).toBeInTheDocument();
expect(screen.getByText('Cancel')).toBeInTheDocument();
expect(screen.getByTestId('filter-bar-cancel')).toBeInTheDocument();
expect(screen.getByRole('button', {name: 'Edit Dashboard'})).toBeDisabled();
});

Expand Down Expand Up @@ -1463,7 +1463,7 @@ describe('Dashboards > Detail', function () {

// Save and Cancel should not appear because alpha, beta is the same as beta, alpha
expect(screen.queryByText('Save')).not.toBeInTheDocument();
expect(screen.queryByText('Cancel')).not.toBeInTheDocument();
expect(screen.queryByTestId('filter-bar-cancel')).not.toBeInTheDocument();
});

it('uses releases from the URL query params', async function () {
Expand Down Expand Up @@ -1500,7 +1500,7 @@ describe('Dashboards > Detail', function () {

await screen.findByText(/not-selected-1/);
screen.getByText('Save');
screen.getByText('Cancel');
screen.getByTestId('filter-bar-cancel');
});

it('resets release in URL params', async function () {
Expand Down Expand Up @@ -1546,7 +1546,7 @@ describe('Dashboards > Detail', function () {
);

await screen.findByText(/not-selected-1/);
await userEvent.click(screen.getByText('Cancel'));
await userEvent.click(screen.getByTestId('filter-bar-cancel'));

// release isn't used in the redirect
expect(testData.router.replace).toHaveBeenCalledWith(
Expand Down Expand Up @@ -1681,10 +1681,7 @@ describe('Dashboards > Detail', function () {
/>,
{
router: initialData.router,
organization: {
...initialData.organization,
features: ['dashboards-edit-access'],
},
organization: initialData.organization,
}
);

Expand All @@ -1703,10 +1700,7 @@ describe('Dashboards > Detail', function () {
render(
<ViewEditDashboard
{...RouteComponentPropsFixture()}
organization={{
...initialData.organization,
features: ['dashboards-edit-access', ...initialData.organization.features],
}}
organization={initialData.organization}
params={{orgId: 'org-slug', dashboardId: '1'}}
router={initialData.router}
location={initialData.router.location}
Expand All @@ -1715,9 +1709,7 @@ describe('Dashboards > Detail', function () {
</ViewEditDashboard>,
{
router: initialData.router,
organization: {
features: ['dashboards-edit-access', ...initialData.organization.features],
},
organization: initialData.organization,
}
);
await userEvent.click(await screen.findByText('Edit Access:'));
Expand Down Expand Up @@ -1771,10 +1763,7 @@ describe('Dashboards > Detail', function () {
render(
<ViewEditDashboard
{...RouteComponentPropsFixture()}
organization={{
...initialData.organization,
features: ['dashboards-edit-access', ...initialData.organization.features],
}}
organization={initialData.organization}
params={{orgId: 'org-slug', dashboardId: '1'}}
router={initialData.router}
location={initialData.router.location}
Expand All @@ -1783,9 +1772,7 @@ describe('Dashboards > Detail', function () {
</ViewEditDashboard>,
{
router: initialData.router,
organization: {
features: ['dashboards-edit-access', ...initialData.organization.features],
},
organization: initialData.organization,
}
);
await userEvent.click(await screen.findByText('Edit Access:'));
Expand Down Expand Up @@ -1860,10 +1847,7 @@ describe('Dashboards > Detail', function () {
render(
<ViewEditDashboard
{...RouteComponentPropsFixture()}
organization={{
...initialData.organization,
features: ['dashboards-edit-access', ...initialData.organization.features],
}}
organization={initialData.organization}
params={{orgId: 'org-slug', dashboardId: '1'}}
router={initialData.router}
location={initialData.router.location}
Expand All @@ -1872,9 +1856,7 @@ describe('Dashboards > Detail', function () {
</ViewEditDashboard>,
{
router: initialData.router,
organization: {
features: ['dashboards-edit-access', ...initialData.organization.features],
},
organization: initialData.organization,
}
);
await userEvent.click(await screen.findByText('Edit Access:'));
Expand Down Expand Up @@ -1931,7 +1913,7 @@ describe('Dashboards > Detail', function () {
{...RouteComponentPropsFixture()}
organization={{
...initialData.organization,
features: ['dashboards-edit-access', ...initialData.organization.features],
features: initialData.organization.features,
access: ['org:read'],
}}
params={{orgId: 'org-slug', dashboardId: '1'}}
Expand All @@ -1943,7 +1925,7 @@ describe('Dashboards > Detail', function () {
{
router: initialData.router,
organization: {
features: ['dashboards-edit-access', ...initialData.organization.features],
features: initialData.organization.features,
access: ['org:read'],
},
}
Expand Down Expand Up @@ -1995,7 +1977,7 @@ describe('Dashboards > Detail', function () {
{...RouteComponentPropsFixture()}
organization={{
...initialData.organization,
features: ['dashboards-edit-access', ...initialData.organization.features],
features: initialData.organization.features,
access: ['org:read'],
}}
params={{orgId: 'org-slug', dashboardId: '1'}}
Expand All @@ -2007,7 +1989,7 @@ describe('Dashboards > Detail', function () {
{
router: initialData.router,
organization: {
features: ['dashboards-edit-access', ...initialData.organization.features],
features: initialData.organization.features,
access: ['org:read'],
},
}
Expand Down
1 change: 0 additions & 1 deletion static/app/views/dashboards/detail.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ export function checkUserHasEditAccess(
dashboardCreator?: User
): boolean {
if (
!organization.features.includes('dashboards-edit-access') ||
hasEveryAccess(['org:write'], {organization}) || // Managers and Owners
!dashboardPermissions ||
dashboardPermissions.isEditableByEveryone ||
Expand Down
21 changes: 10 additions & 11 deletions static/app/views/dashboards/filtersBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,13 @@ export default function FiltersBar({
const organization = useOrganization();
const currentUser = useUser();
const {teams: userTeams} = useUserTeams();
let hasEditAccess = true;
if (organization.features.includes('dashboards-edit-access')) {
hasEditAccess = checkUserHasEditAccess(
currentUser,
userTeams,
organization,
dashboardPermissions,
dashboardCreator
);
}
const hasEditAccess = checkUserHasEditAccess(
currentUser,
userTeams,
organization,
dashboardPermissions,
dashboardCreator
);

const selectedReleases =
(defined(location.query?.[DashboardFilterKeys.RELEASE])
Expand Down Expand Up @@ -130,7 +127,9 @@ export default function FiltersBar({
>
{t('Save')}
</Button>
<Button onClick={onCancel}>{t('Cancel')}</Button>
<Button data-test-id={'filter-bar-cancel'} onClick={onCancel}>
{t('Cancel')}
</Button>
</FilterButtons>
)}
</Fragment>
Expand Down
21 changes: 10 additions & 11 deletions static/app/views/dashboards/manage/dashboardTable.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ describe('Dashboards - DashboardTable', function () {
).toBeInTheDocument();
});

it('renders dashboard list', function () {
it('renders dashboard list', async function () {
render(
<DashboardTable
onDashboardsChange={jest.fn()}
Expand All @@ -137,11 +137,11 @@ describe('Dashboards - DashboardTable', function () {
/>
);

expect(screen.getByText('Dashboard 1')).toBeInTheDocument();
expect(screen.getByText('Dashboard 2')).toBeInTheDocument();
expect(await screen.findByText('Dashboard 1')).toBeInTheDocument();
expect(await screen.findByText('Dashboard 2')).toBeInTheDocument();
});

it('returns landing page url for dashboards', function () {
it('returns landing page url for dashboards', async function () {
render(
<DashboardTable
onDashboardsChange={jest.fn()}
Expand All @@ -152,17 +152,17 @@ describe('Dashboards - DashboardTable', function () {
{router}
);

expect(screen.getByRole('link', {name: 'Dashboard 1'})).toHaveAttribute(
expect(await screen.findByRole('link', {name: 'Dashboard 1'})).toHaveAttribute(
'href',
'/organizations/org-slug/dashboard/1/'
);
expect(screen.getByRole('link', {name: 'Dashboard 2'})).toHaveAttribute(
expect(await screen.findByRole('link', {name: 'Dashboard 2'})).toHaveAttribute(
'href',
'/organizations/org-slug/dashboard/2/'
);
});

it('persists global selection headers', function () {
it('persists global selection headers', async function () {
render(
<DashboardTable
onDashboardsChange={jest.fn()}
Expand All @@ -173,7 +173,7 @@ describe('Dashboards - DashboardTable', function () {
{router}
);

expect(screen.getByRole('link', {name: 'Dashboard 1'})).toHaveAttribute(
expect(await screen.findByRole('link', {name: 'Dashboard 1'})).toHaveAttribute(
'href',
'/organizations/org-slug/dashboard/1/?statsPeriod=7d'
);
Expand Down Expand Up @@ -205,7 +205,7 @@ describe('Dashboards - DashboardTable', function () {
expect(dashboardUpdateMock).toHaveBeenCalled();
});

it('cannot delete last dashboard', function () {
it('cannot delete last dashboard', async function () {
const singleDashboard = [
DashboardListItemFixture({
id: '1',
Expand All @@ -224,7 +224,7 @@ describe('Dashboards - DashboardTable', function () {
/>
);

expect(screen.getAllByTestId('dashboard-delete')[0]).toHaveAttribute(
expect((await screen.findAllByTestId('dashboard-delete'))[0]).toHaveAttribute(
'aria-disabled',
'true'
);
Expand Down Expand Up @@ -295,7 +295,6 @@ describe('Dashboards - DashboardTable', function () {
'dashboards-edit',
'discover-query',
'dashboards-table-view',
'dashboards-edit-access',
],
});

Expand Down
9 changes: 2 additions & 7 deletions static/app/views/dashboards/manage/dashboardTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,7 @@ function DashboardTable({
{key: ResponseKeys.NAME, name: t('Name'), width: COL_WIDTH_UNDEFINED},
{key: ResponseKeys.WIDGETS, name: t('Widgets'), width: COL_WIDTH_UNDEFINED},
{key: ResponseKeys.OWNER, name: t('Owner'), width: COL_WIDTH_UNDEFINED},
...(organization.features.includes('dashboards-edit-access')
? [{key: ResponseKeys.ACCESS, name: t('Access'), width: COL_WIDTH_UNDEFINED}]
: []),
{key: ResponseKeys.ACCESS, name: t('Access'), width: COL_WIDTH_UNDEFINED},
{key: ResponseKeys.CREATED, name: t('Created'), width: COL_WIDTH_UNDEFINED},
];

Expand Down Expand Up @@ -246,10 +244,7 @@ function DashboardTable({
);
}

if (
column.key === ResponseKeys.ACCESS &&
organization.features.includes('dashboards-edit-access')
) {
if (column.key === ResponseKeys.ACCESS) {
/* Handles POST request for Edit Access Selector Changes */
const onChangeEditAccess = (newDashboardPermissions: DashboardPermissions) => {
const dashboardCopy = cloneDeep(dataRow);
Expand Down
Loading