ref(hybrid-cloud): Restrict notification settings UX to a single organization (#50279)

mikejihbe · vbro · getsantry[bot] · web-flow · commit aa870e32734b · 2023-07-17T09:50:10.000-07:00
Refactor the notification settings page to restrict viewing notification settings to a single organization. This is required to simplify some user notification setting APIs and make them compatible with hybrid cloud. Currently the user notifications APIs return "virtual" notification settings for every project. This page doesn't really use those settings, it instead renders projects from the list of projects and merges in what gets returned. I think it should work fine if we only return actual notification settings records, which will allow us to simplify the user notification settings endpoints by removing these "virtual" entries -- assuming this was the only use case. <img width="1231" alt="Issue_Alert_Notifications_—_Sentry" src="https://github.com/getsentry/sentry/assets/52534/3c59964c-faf2-41d5-8677-fb290063017f"> --------- Co-authored-by: Valery Brobbey <valery.brobbey@sentry.io> Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com> Co-authored-by: Zachary Collins <zachary.collins@sentry.io> Co-authored-by: Zach Collins <recursive.cookie.jar@gmail.com> Co-authored-by: Mark Story <mark@mark-story.com>
diff --git a/static/app/views/settings/account/accountNotificationFineTuning.tsx b/static/app/views/settings/account/accountNotificationFineTuning.tsx
@@ -19,7 +19,9 @@ import {
   ACCOUNT_NOTIFICATION_FIELDS,
   FineTuneField,
 } from 'sentry/views/settings/account/notifications/fields';
-import NotificationSettingsByType from 'sentry/views/settings/account/notifications/notificationSettingsByType';
+import NotificationSettingsByType, {
+  OrganizationSelectHeader,
+} from 'sentry/views/settings/account/notifications/notificationSettingsByType';
 import {
   getNotificationTypeFromPathname,
   groupByOrganization,
@@ -71,7 +73,6 @@ function AccountNotificationsByProject({projects, field}: ANBPProps) {
     <Fragment>
       {data.map(({name, projects: projectFields}) => (
         <div key={name}>
-          <PanelHeader>{name}</PanelHeader>
           {projectFields.map(f => (
             <PanelBodyLineItem key={f.name}>
               <SelectField
@@ -135,20 +136,33 @@ type State = DeprecatedAsyncView['state'] & {
   emails: UserEmail[] | null;
   fineTuneData: Record<string, any> | null;
   notifications: Record<string, any> | null;
+  organizationId: string;
   projects: Project[] | null;
 };
 
 class AccountNotificationFineTuning extends DeprecatedAsyncView<Props, State> {
+  getDefaultState() {
+    return {
+      ...super.getDefaultState(),
+      emails: [],
+      fineTuneData: null,
+      notifications: [],
+      projects: [],
+      organizationId: this.props.organizations[0].id,
+    };
+  }
+
   getEndpoints(): ReturnType<DeprecatedAsyncView['getEndpoints']> {
     const {fineTuneType: pathnameType} = this.props.params;
+    const orgId = this.state?.organizationId || this.props.organizations[0].id;
     const fineTuneType = getNotificationTypeFromPathname(pathnameType);
     const endpoints = [
       ['notifications', '/users/me/notifications/'],
       ['fineTuneData', `/users/me/notifications/${fineTuneType}/`],
     ];
 
     if (isGroupedByProject(fineTuneType)) {
-      endpoints.push(['projects', '/projects/']);
+      endpoints.push(['projects', `/projects/?organization_id=${orgId}`]);
     }
 
     endpoints.push(['emails', '/users/me/emails/']);
@@ -178,6 +192,14 @@ class AccountNotificationFineTuning extends DeprecatedAsyncView<Props, State> {
     );
   }
 
+  handleOrgChange = (option: {label: string; value: string}) => {
+    this.setState({organizationId: option.value});
+    const self = this;
+    setTimeout(() => {
+      self.reloadData();
+    }, 0);
+  };
+
   renderBody() {
     const {params} = this.props;
     const {fineTuneType: pathnameType} = params;
@@ -204,7 +226,6 @@ class AccountNotificationFineTuning extends DeprecatedAsyncView<Props, State> {
     if (!notifications || !fineTuneData) {
       return null;
     }
-
     return (
       <div>
         <SettingsPageHeader title={title} />
@@ -227,19 +248,25 @@ class AccountNotificationFineTuning extends DeprecatedAsyncView<Props, State> {
             </Form>
           )}
         <Panel>
+          <PanelHeader hasButtons={isProject}>
+            {isProject ? (
+              <Fragment>
+                <OrganizationSelectHeader
+                  organizations={this.props.organizations}
+                  organizationId={this.state.organizationId}
+                  handleOrgChange={this.handleOrgChange}
+                />
+                {this.renderSearchInput({
+                  placeholder: t('Search Projects'),
+                  url,
+                  stateKey,
+                })}
+              </Fragment>
+            ) : (
+              <Heading>{t('Organizations')}</Heading>
+            )}
+          </PanelHeader>
           <PanelBody>
-            <PanelHeader hasButtons={isProject}>
-              <Heading>{isProject ? t('Projects') : t('Organizations')}</Heading>
-              <div>
-                {isProject &&
-                  this.renderSearchInput({
-                    placeholder: t('Search Projects'),
-                    url,
-                    stateKey,
-                  })}
-              </div>
-            </PanelHeader>
-
             <Form
               saveOnBlur
               apiMethod="PUT"
@@ -271,4 +298,4 @@ const Heading = styled('div')`
   flex: 1;
 `;
 
-export default AccountNotificationFineTuning;
+export default withOrganizations(AccountNotificationFineTuning);
diff --git a/static/app/views/settings/account/notifications/notificationSettings.tsx b/static/app/views/settings/account/notifications/notificationSettings.tsx
@@ -54,7 +54,7 @@ class NotificationSettings extends DeprecatedAsyncComponent<Props, State> {
 
   getEndpoints(): ReturnType<DeprecatedAsyncComponent['getEndpoints']> {
     return [
-      ['notificationSettings', `/users/me/notification-settings/`],
+      ['notificationSettings', `/users/me/notification-settings/`, {v2: 'serializer'}],
       ['legacyData', '/users/me/notifications/'],
     ];
   }
diff --git a/static/app/views/settings/account/notifications/notificationSettingsByOrganization.tsx b/static/app/views/settings/account/notifications/notificationSettingsByOrganization.tsx
@@ -1,12 +1,12 @@
 import Form from 'sentry/components/forms/form';
-import JsonForm from 'sentry/components/forms/jsonForm';
 import {t} from 'sentry/locale';
 import {OrganizationSummary} from 'sentry/types';
 import withOrganizations from 'sentry/utils/withOrganizations';
 import {
   NotificationSettingsByProviderObject,
   NotificationSettingsObject,
 } from 'sentry/views/settings/account/notifications/constants';
+import {StyledJsonForm} from 'sentry/views/settings/account/notifications/notificationSettingsByProjects';
 import {
   getParentData,
   getParentField,
@@ -38,11 +38,16 @@ function NotificationSettingsByOrganization({
       initialData={getParentData(notificationType, notificationSettings, organizations)}
       onSubmitSuccess={onSubmitSuccess}
     >
-      <JsonForm
+      <StyledJsonForm
         title={t('Organizations')}
-        fields={organizations.map(organization =>
-          getParentField(notificationType, notificationSettings, organization, onChange)
-        )}
+        fields={organizations.map(organization => {
+          return getParentField(
+            notificationType,
+            notificationSettings,
+            organization,
+            onChange
+          );
+        })}
       />
     </Form>
   );
diff --git a/static/app/views/settings/account/notifications/notificationSettingsByProjects.spec.tsx b/static/app/views/settings/account/notifications/notificationSettingsByProjects.spec.tsx
@@ -5,10 +5,10 @@ import {Project} from 'sentry/types';
 import NotificationSettingsByProjects from 'sentry/views/settings/account/notifications/notificationSettingsByProjects';
 
 const renderComponent = (projects: Project[]) => {
-  const {routerContext} = initializeOrg();
+  const {routerContext, organization} = initializeOrg();
 
   MockApiClient.addMockResponse({
-    url: '/projects/',
+    url: `/projects/`,
     method: 'GET',
     body: projects,
   });
@@ -28,6 +28,9 @@ const renderComponent = (projects: Project[]) => {
       notificationSettings={notificationSettings}
       onChange={jest.fn()}
       onSubmitSuccess={jest.fn()}
+      organizationId={organization.id}
+      organizations={[organization]}
+      handleOrgChange={jest.fn()}
     />,
     {context: routerContext}
   );
diff --git a/static/app/views/settings/account/notifications/notificationSettingsByProjects.tsx b/static/app/views/settings/account/notifications/notificationSettingsByProjects.tsx
@@ -6,15 +6,19 @@ import EmptyMessage from 'sentry/components/emptyMessage';
 import Form from 'sentry/components/forms/form';
 import JsonForm from 'sentry/components/forms/jsonForm';
 import Pagination from 'sentry/components/pagination';
+import Panel from 'sentry/components/panels/panel';
+import PanelBody from 'sentry/components/panels/panelBody';
+import PanelHeader from 'sentry/components/panels/panelHeader';
 import {t} from 'sentry/locale';
-import {Project} from 'sentry/types';
+import {Organization, Project} from 'sentry/types';
 import {sortProjects} from 'sentry/utils';
 import {
   MIN_PROJECTS_FOR_PAGINATION,
   MIN_PROJECTS_FOR_SEARCH,
   NotificationSettingsByProviderObject,
   NotificationSettingsObject,
 } from 'sentry/views/settings/account/notifications/constants';
+import {OrganizationSelectHeader} from 'sentry/views/settings/account/notifications/notificationSettingsByType';
 import {
   getParentData,
   getParentField,
@@ -25,15 +29,22 @@ import {
   SearchWrapper,
 } from 'sentry/views/settings/components/defaultSearchBar';
 
-type Props = {
+export type NotificationSettingsByProjectsBaseProps = {
   notificationSettings: NotificationSettingsObject;
   notificationType: string;
   onChange: (
     changedData: NotificationSettingsByProviderObject,
     parentId: string
   ) => NotificationSettingsObject;
   onSubmitSuccess: () => void;
-} & DeprecatedAsyncComponent['props'];
+};
+
+export type Props = {
+  handleOrgChange: Function;
+  organizationId: string;
+  organizations: Organization[];
+} & NotificationSettingsByProjectsBaseProps &
+  DeprecatedAsyncComponent['props'];
 
 type State = {
   projects: Project[];
@@ -48,7 +59,15 @@ class NotificationSettingsByProjects extends DeprecatedAsyncComponent<Props, Sta
   }
 
   getEndpoints(): ReturnType<DeprecatedAsyncComponent['getEndpoints']> {
-    return [['projects', '/projects/']];
+    return [
+      [
+        'projects',
+        `/projects/`,
+        {
+          query: {organizationId: this.props.organizationId},
+        },
+      ],
+    ];
   }
 
   /**
@@ -74,6 +93,12 @@ class NotificationSettingsByProjects extends DeprecatedAsyncComponent<Props, Sta
     );
   };
 
+  handleOrgChange = (option: {label: string; value: string}) => {
+    // handleOrgChange(option: {label: string; value: string}) {
+    this.props.handleOrgChange(option);
+    setTimeout(() => this.reloadData(), 0);
+  };
+
   renderBody() {
     const {notificationType, notificationSettings, onChange, onSubmitSuccess} =
       this.props;
@@ -88,35 +113,50 @@ class NotificationSettingsByProjects extends DeprecatedAsyncComponent<Props, Sta
 
     return (
       <Fragment>
-        {canSearch &&
-          this.renderSearchInput({
-            stateKey: 'projects',
-            url: '/projects/',
-            placeholder: t('Search Projects'),
-            children: renderSearch,
-          })}
-        <Form
-          saveOnBlur
-          apiMethod="PUT"
-          apiEndpoint="/users/me/notification-settings/"
-          initialData={getParentData(notificationType, notificationSettings, projects)}
-          onSubmitSuccess={onSubmitSuccess}
-        >
-          {projects.length === 0 ? (
-            <EmptyMessage>{t('No projects found')}</EmptyMessage>
-          ) : (
-            Object.entries(this.getGroupedProjects()).map(([groupTitle, parents]) => (
-              <JsonForm
-                collapsible
-                key={groupTitle}
-                title={groupTitle}
-                fields={parents.map(parent =>
-                  getParentField(notificationType, notificationSettings, parent, onChange)
-                )}
-              />
-            ))
-          )}
-        </Form>
+        <PanelHeader>
+          <OrganizationSelectHeader
+            organizations={this.props.organizations}
+            organizationId={this.props.organizationId}
+            handleOrgChange={this.handleOrgChange}
+          />
+
+          {canSearch &&
+            this.renderSearchInput({
+              stateKey: 'projects',
+              url: `/projects/?organizationId=${this.props.organizationId}`,
+              placeholder: t('Search Projects'),
+              children: renderSearch,
+            })}
+        </PanelHeader>
+        <PanelBody>
+          <Form
+            saveOnBlur
+            apiMethod="PUT"
+            apiEndpoint="/users/me/notification-settings/"
+            initialData={getParentData(notificationType, notificationSettings, projects)}
+            onSubmitSuccess={onSubmitSuccess}
+          >
+            {projects.length === 0 ? (
+              <EmptyMessage>{t('No projects found')}</EmptyMessage>
+            ) : (
+              Object.entries(this.getGroupedProjects()).map(([groupTitle, parents]) => (
+                <StyledJsonForm
+                  collapsible
+                  key={groupTitle}
+                  // title={groupTitle}
+                  fields={parents.map(parent =>
+                    getParentField(
+                      notificationType,
+                      notificationSettings,
+                      parent,
+                      onChange
+                    )
+                  )}
+                />
+              ))
+            )}
+          </Form>
+        </PanelBody>
         {canSearch && shouldPaginate && (
           <Pagination pageLinks={projectsPageLinks} {...this.props} />
         )}
@@ -132,3 +172,10 @@ const StyledSearchWrapper = styled(SearchWrapper)`
     width: 100%;
   }
 `;
+
+export const StyledJsonForm = styled(JsonForm)`
+  ${Panel} {
+    border: 0;
+    margin-bottom: 0;
+  }
+`;
diff --git a/static/app/views/settings/account/notifications/notificationSettingsByType.spec.tsx b/static/app/views/settings/account/notifications/notificationSettingsByType.spec.tsx
@@ -30,7 +30,7 @@ function renderMockRequests(
   });
 
   MockApiClient.addMockResponse({
-    url: '/projects/',
+    url: `/projects/`,
     method: 'GET',
     body: [],
   });
diff --git a/static/app/views/settings/account/notifications/notificationSettingsByType.tsx b/static/app/views/settings/account/notifications/notificationSettingsByType.tsx
diff --git a/static/app/views/settings/account/notifications/utils.tsx b/static/app/views/settings/account/notifications/utils.tsx

Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ class NotificationSettings extends DeprecatedAsyncComponent<Props, State> {`
`54`	`54`
`55`	`55`	`getEndpoints(): ReturnType<DeprecatedAsyncComponent['getEndpoints']> {`
`56`	`56`	`return [`
`57`		- ['notificationSettings', `/users/me/notification-settings/`],
	`57`	+ ['notificationSettings', `/users/me/notification-settings/`, {v2: 'serializer'}],
`58`	`58`	`['legacyData', '/users/me/notifications/'],`
`59`	`59`	`];`
`60`	`60`	`}`