ref(dashboards): Fixes and refactoring for edit access selector button (#80633)

harshithadurai · web-flow · commit df0147f0df67 · 2024-11-15T11:34:54.000-05:00
#79833 Some fixes for the Edit Access dropdown button in Dashboards. 1. modified behaviour of `Save Changes` button in `EditAccessSelector.tsx` so that it's enabled only when the changed selections are not the same as the existing perms 2. pass down props so that Widget edit buttons are disabled when user does not have edit perms 3. pass down props to top level dashboard filters so that users without edit perms cannot save changes 4. pass down props to widget context menu to disable the `Edit Widget` button there <img width="500" alt="Screenshot 2024-11-13 at 11 04 49 AM" src="https://github.com/user-attachments/assets/7931fbb5-620d-46e7-bde5-b832b6978554"> <img width="500" alt="Screenshot 2024-11-14 at 4 12 23 PM" src="https://github.com/user-attachments/assets/f83c3d66-16d0-4e12-8184-a645bb4a2be7">
diff --git a/static/app/components/modals/widgetViewerModal.tsx b/static/app/components/modals/widgetViewerModal.tsx
@@ -3,6 +3,7 @@ import {components} from 'react-select';
 import {css} from '@emotion/react';
 import styled from '@emotion/styled';
 import * as Sentry from '@sentry/react';
+import type {User} from '@sentry/types';
 import {truncate} from '@sentry/utils';
 import type {DataZoomComponentOption} from 'echarts';
 import type {Location} from 'history';
@@ -54,9 +55,16 @@ import useApi from 'sentry/utils/useApi';
 import {useLocation} from 'sentry/utils/useLocation';
 import {useNavigate} from 'sentry/utils/useNavigate';
 import useProjects from 'sentry/utils/useProjects';
+import {useUser} from 'sentry/utils/useUser';
+import {useUserTeams} from 'sentry/utils/useUserTeams';
 import withPageFilters from 'sentry/utils/withPageFilters';
+import {checkUserHasEditAccess} from 'sentry/views/dashboards/detail';
 import {DiscoverSplitAlert} from 'sentry/views/dashboards/discoverSplitAlert';
-import type {DashboardFilters, Widget} from 'sentry/views/dashboards/types';
+import type {
+  DashboardFilters,
+  DashboardPermissions,
+  Widget,
+} from 'sentry/views/dashboards/types';
 import {DisplayType, WidgetType} from 'sentry/views/dashboards/types';
 import {
   dashboardFiltersToString,
@@ -104,7 +112,9 @@ export interface WidgetViewerModalOptions {
   organization: Organization;
   widget: Widget;
   widgetLegendState: WidgetLegendSelectionState;
+  dashboardCreator?: User;
   dashboardFilters?: DashboardFilters;
+  dashboardPermissions?: DashboardPermissions;
   onEdit?: () => void;
   onMetricWidgetEdit?: (widget: Widget) => void;
   pageLinks?: string;
@@ -194,6 +204,8 @@ function WidgetViewerModal(props: Props) {
     seriesResultsType,
     dashboardFilters,
     widgetLegendState,
+    dashboardPermissions,
+    dashboardCreator,
   } = props;
   const location = useLocation();
   const {projects} = useProjects();
@@ -843,6 +855,18 @@ 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
+    );
+  }
   function renderWidgetViewer() {
     return (
       <Fragment>
@@ -1059,6 +1083,7 @@ function WidgetViewerModal(props: Props) {
                                 display_type: widget.displayType,
                               });
                             }}
+                            disabled={!hasEditAccess}
                           >
                             {t('Edit Widget')}
                           </Button>
diff --git a/static/app/views/dashboards/controls.tsx b/static/app/views/dashboards/controls.tsx
@@ -150,10 +150,11 @@ function Controls({
   let hasEditAccess = true;
   if (organization.features.includes('dashboards-edit-access')) {
     hasEditAccess = checkUserHasEditAccess(
-      dashboard,
       currentUser,
       userTeams,
-      organization
+      organization,
+      dashboard.permissions,
+      dashboard.createdBy
     );
   }
 
diff --git a/static/app/views/dashboards/dashboard.tsx b/static/app/views/dashboards/dashboard.tsx
@@ -434,6 +434,8 @@ class Dashboard extends Component<Props, State> {
       <div key={key} data-grid={widget.layout}>
         <SortableWidget
           {...widgetProps}
+          dashboardPermissions={dashboard.permissions}
+          dashboardCreator={dashboard.createdBy}
           isMobile={isMobile}
           windowWidth={windowWidth}
           index={String(index)}
diff --git a/static/app/views/dashboards/detail.spec.tsx b/static/app/views/dashboards/detail.spec.tsx
@@ -1931,6 +1931,80 @@ describe('Dashboards > Detail', function () {
       expect(screen.getByRole('button', {name: 'Add Widget'})).toBeDisabled();
     });
 
+    it('disables widget edit, duplicate, and delete button when user does not have edit perms', async function () {
+      const widget = {
+        displayType: types.DisplayType.TABLE,
+        interval: '1d',
+        queries: [
+          {
+            name: 'Test Widget',
+            fields: ['count()', 'count_unique(user)', 'epm()', 'project'],
+            columns: ['project'],
+            aggregates: ['count()', 'count_unique(user)', 'epm()'],
+            conditions: '',
+            orderby: '',
+          },
+        ],
+        title: 'Transactions',
+        id: '1',
+        widgetType: types.WidgetType.DISCOVER,
+      };
+      const mockDashboard = DashboardFixture([widget], {
+        id: '1',
+        title: 'Custom Errors',
+        createdBy: UserFixture({id: '238900'}),
+        permissions: {isEditableByEveryone: false},
+      });
+      MockApiClient.addMockResponse({
+        url: '/organizations/org-slug/dashboards/',
+        body: mockDashboard,
+      });
+      MockApiClient.addMockResponse({
+        url: '/organizations/org-slug/dashboards/1/',
+        body: mockDashboard,
+      });
+
+      const currentUser = UserFixture({id: '781629'});
+      ConfigStore.set('user', currentUser);
+
+      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 screen.findByText('Edit Access:');
+      expect(screen.getByRole('button', {name: 'Edit Dashboard'})).toBeDisabled();
+      expect(screen.getByRole('button', {name: 'Add Widget'})).toBeDisabled();
+      await userEvent.click(await screen.findByLabelText('Widget actions'));
+      expect(
+        screen.getByRole('menuitemradio', {name: 'Duplicate Widget'})
+      ).toHaveAttribute('aria-disabled', 'true');
+      expect(screen.getByRole('menuitemradio', {name: 'Delete Widget'})).toHaveAttribute(
+        'aria-disabled',
+        'true'
+      );
+      expect(screen.getByRole('menuitemradio', {name: 'Edit Widget'})).toHaveAttribute(
+        'aria-disabled',
+        'true'
+      );
+    });
+
     describe('discover split', function () {
       it('calls the dashboard callbacks with the correct widgetType for discover split', function () {
         const widget = {
diff --git a/static/app/views/dashboards/detail.tsx b/static/app/views/dashboards/detail.tsx
@@ -172,24 +172,23 @@ export function handleUpdateDashboardSplit({
 
 /* Checks if current user has permissions to edit dashboard */
 export function checkUserHasEditAccess(
-  dashboard: DashboardDetails,
   currentUser: User,
   userTeams: Team[],
-  organization: Organization
+  organization: Organization,
+  dashboardPermissions?: DashboardPermissions,
+  dashboardCreator?: User
 ): boolean {
   if (
     !organization.features.includes('dashboards-edit-access') ||
-    !dashboard.permissions ||
-    dashboard.permissions.isEditableByEveryone ||
-    dashboard.createdBy?.id === currentUser.id
+    !dashboardPermissions ||
+    dashboardPermissions.isEditableByEveryone ||
+    dashboardCreator?.id === currentUser.id
   ) {
     return true;
   }
-  if (dashboard.permissions.teamsWithEditAccess?.length) {
+  if (dashboardPermissions.teamsWithEditAccess?.length) {
     const userTeamIds = userTeams.map(team => Number(team.id));
-    dashboard.permissions.teamsWithEditAccess.some(teamId =>
-      userTeamIds.includes(teamId)
-    );
+    dashboardPermissions.teamsWithEditAccess.some(teamId => userTeamIds.includes(teamId));
   }
   return false;
 }
@@ -273,6 +272,8 @@ class DashboardDetail extends Component<Props, State> {
           totalIssuesCount,
           widgetLegendState: this.state.widgetLegendState,
           dashboardFilters: getDashboardFiltersFromURL(location) ?? dashboard.filters,
+          dashboardPermissions: dashboard.permissions,
+          dashboardCreator: dashboard.createdBy,
           onMetricWidgetEdit: (updatedWidget: Widget) => {
             const widgets = [...dashboard.widgets];
 
@@ -845,6 +846,8 @@ class DashboardDetail extends Component<Props, State> {
                 </StyledPageHeader>
                 <HookHeader organization={organization} />
                 <FiltersBar
+                  dashboardPermissions={dashboard.permissions}
+                  dashboardCreator={dashboard.createdBy}
                   filters={{}} // Default Dashboards don't have filters set
                   location={location}
                   hasUnsavedChanges={false}
@@ -1018,6 +1021,8 @@ class DashboardDetail extends Component<Props, State> {
                               ) : null}
                               <FiltersBar
                                 filters={(modifiedDashboard ?? dashboard).filters}
+                                dashboardPermissions={dashboard.permissions}
+                                dashboardCreator={dashboard.createdBy}
                                 location={location}
                                 hasUnsavedChanges={hasUnsavedFilters}
                                 isEditingDashboard={
diff --git a/static/app/views/dashboards/editAccessSelector.tsx b/static/app/views/dashboards/editAccessSelector.tsx
@@ -38,7 +38,6 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP
   const teamIds: string[] = Object.values(teams).map(team => team.id);
   const [selectedOptions, setSelectedOptions] = useState<string[]>(getSelectedOptions());
   const [isMenuOpen, setMenuOpen] = useState<boolean>(false);
-  const [hasUnsavedChanges, setHasUnsavedChanges] = useState<boolean>(false);
 
   // Handles state change when dropdown options are selected
   const onSelectOptions = newSelectedOptions => {
@@ -176,7 +175,6 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP
           setMenuOpen(false);
           if (isMenuOpen) {
             setSelectedOptions(getSelectedOptions());
-            setHasUnsavedChanges(false);
           }
         }}
         disabled={!isCurrentUserDashboardOwner}
@@ -198,7 +196,10 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP
           setMenuOpen(!isMenuOpen);
         }}
         priority="primary"
-        disabled={!isCurrentUserDashboardOwner || !hasUnsavedChanges}
+        disabled={
+          !isCurrentUserDashboardOwner ||
+          isEqual(getDashboardPermissions(), dashboard.permissions)
+        }
       >
         {t('Save Changes')}
       </Button>
@@ -210,7 +211,6 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP
       size="sm"
       onChange={newSelectedOptions => {
         onSelectOptions(newSelectedOptions);
-        setHasUnsavedChanges(true);
       }}
       multiple
       searchable
@@ -223,7 +223,6 @@ function EditAccessSelector({dashboard, onChangeEditAccess}: EditAccessSelectorP
         setMenuOpen(!isMenuOpen);
         if (isMenuOpen) {
           setSelectedOptions(getSelectedOptions());
-          setHasUnsavedChanges(false);
         }
       }}
       menuFooter={dropdownFooterButtons}
diff --git a/static/app/views/dashboards/filtersBar.tsx b/static/app/views/dashboards/filtersBar.tsx
@@ -1,5 +1,6 @@
 import {Fragment} from 'react';
 import styled from '@emotion/styled';
+import type {User} from '@sentry/types';
 import type {Location} from 'history';
 
 import {Button} from 'sentry/components/button';
@@ -17,9 +18,12 @@ import {decodeList} from 'sentry/utils/queryString';
 import {ReleasesProvider} from 'sentry/utils/releases/releasesProvider';
 import useOrganization from 'sentry/utils/useOrganization';
 import usePageFilters from 'sentry/utils/usePageFilters';
+import {useUser} from 'sentry/utils/useUser';
+import {useUserTeams} from 'sentry/utils/useUserTeams';
+import {checkUserHasEditAccess} from 'sentry/views/dashboards/detail';
 
 import ReleasesSelectControl from './releasesSelectControl';
-import type {DashboardFilters} from './types';
+import type {DashboardFilters, DashboardPermissions} from './types';
 import {DashboardFilterKeys} from './types';
 
 type FiltersBarProps = {
@@ -29,12 +33,16 @@ type FiltersBarProps = {
   isPreview: boolean;
   location: Location;
   onDashboardFilterChange: (activeFilters: DashboardFilters) => void;
+  dashboardCreator?: User;
+  dashboardPermissions?: DashboardPermissions;
   onCancel?: () => void;
   onSave?: () => void;
 };
 
 export default function FiltersBar({
   filters,
+  dashboardPermissions,
+  dashboardCreator,
   hasUnsavedChanges,
   isEditingDashboard,
   isPreview,
@@ -45,6 +53,18 @@ export default function FiltersBar({
 }: FiltersBarProps) {
   const {selection} = usePageFilters();
   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 selectedReleases =
     (defined(location.query?.[DashboardFilterKeys.RELEASE])
@@ -100,7 +120,7 @@ export default function FiltersBar({
         </FilterButtons>
         {hasUnsavedChanges && !isEditingDashboard && !isPreview && (
           <FilterButtons>
-            <Button priority="primary" onClick={onSave}>
+            <Button priority="primary" onClick={onSave} disabled={!hasEditAccess}>
               {t('Save')}
             </Button>
             <Button onClick={onCancel}>{t('Cancel')}</Button>
diff --git a/static/app/views/dashboards/metrics/widgetCard.tsx b/static/app/views/dashboards/metrics/widgetCard.tsx
@@ -208,6 +208,7 @@ export function MetricWidgetCard({
                   showContextMenu
                   isPreview={false}
                   widgetLimitReached={false}
+                  hasEditAccess
                   onEdit={() => {
                     router.push({
                       pathname: `${location.pathname}${
diff --git a/static/app/views/dashboards/sortableWidget.tsx b/static/app/views/dashboards/sortableWidget.tsx
diff --git a/static/app/views/dashboards/widgetCard/index.tsx b/static/app/views/dashboards/widgetCard/index.tsx
diff --git a/static/app/views/dashboards/widgetCard/widgetCardContextMenu.tsx b/static/app/views/dashboards/widgetCard/widgetCardContextMenu.tsx
