Browse Source

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

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>
Mike Ihbe 1 year ago
parent
commit
aa870e3273

+ 44 - 17
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,12 +136,25 @@ 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/'],
@@ -148,7 +162,7 @@ class AccountNotificationFineTuning extends DeprecatedAsyncView<Props, State> {
     ];
 
     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);

+ 1 - 1
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/'],
     ];
   }

+ 10 - 5
static/app/views/settings/account/notifications/notificationSettingsByOrganization.tsx

@@ -1,5 +1,4 @@
 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';
@@ -7,6 +6,7 @@ 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>
   );

+ 5 - 2
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}
   );

+ 80 - 33
static/app/views/settings/account/notifications/notificationSettingsByProjects.tsx

@@ -6,8 +6,11 @@ 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,
@@ -15,6 +18,7 @@ import {
   NotificationSettingsByProviderObject,
   NotificationSettingsObject,
 } from 'sentry/views/settings/account/notifications/constants';
+import {OrganizationSelectHeader} from 'sentry/views/settings/account/notifications/notificationSettingsByType';
 import {
   getParentData,
   getParentField,
@@ -25,7 +29,7 @@ import {
   SearchWrapper,
 } from 'sentry/views/settings/components/defaultSearchBar';
 
-type Props = {
+export type NotificationSettingsByProjectsBaseProps = {
   notificationSettings: NotificationSettingsObject;
   notificationType: string;
   onChange: (
@@ -33,7 +37,14 @@ type Props = {
     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;
+  }
+`;

+ 1 - 1
static/app/views/settings/account/notifications/notificationSettingsByType.spec.tsx

@@ -30,7 +30,7 @@ function renderMockRequests(
   });
 
   MockApiClient.addMockResponse({
-    url: '/projects/',
+    url: `/projects/`,
     method: 'GET',
     body: [],
   });

+ 69 - 17
static/app/views/settings/account/notifications/notificationSettingsByType.tsx

@@ -1,9 +1,11 @@
 import {Fragment} from 'react';
 
 import DeprecatedAsyncComponent from 'sentry/components/deprecatedAsyncComponent';
+import SelectControl from 'sentry/components/forms/controls/selectControl';
 import Form from 'sentry/components/forms/form';
 import JsonForm from 'sentry/components/forms/jsonForm';
 import {Field} from 'sentry/components/forms/types';
+import Panel from 'sentry/components/panels/panel';
 import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
 import {t} from 'sentry/locale';
 import {Organization, OrganizationSummary} from 'sentry/types';
@@ -40,6 +42,45 @@ import {
 import SettingsPageHeader from 'sentry/views/settings/components/settingsPageHeader';
 import TextBlock from 'sentry/views/settings/components/text/textBlock';
 
+type OrganizationSelectHeaderProps = {
+  handleOrgChange: Function;
+  organizationId: string;
+  organizations: Organization[];
+};
+
+export function OrganizationSelectHeader({
+  handleOrgChange,
+  organizationId,
+  organizations,
+}: OrganizationSelectHeaderProps) {
+  const getOrganizationOptions = () => {
+    return organizations.map(org => {
+      return {
+        label: org.name,
+        value: org.id,
+      };
+    });
+  };
+
+  return (
+    <Fragment>
+      {t('Settings for Organization')}
+      <SelectControl
+        options={getOrganizationOptions()}
+        onChange={handleOrgChange}
+        // getOptionValue={option => option.value}
+        value={organizationId}
+        styles={{
+          container: (provided: {[x: string]: string | number | boolean}) => ({
+            ...provided,
+            minWidth: `300px`,
+          }),
+        }}
+      />
+    </Fragment>
+  );
+}
+
 type Props = {
   notificationType: string;
   organizations: Organization[];
@@ -78,6 +119,7 @@ class NotificationSettingsByType extends DeprecatedAsyncComponent<Props, State>
       notificationSettings: {},
       identities: [],
       organizationIntegrations: [],
+      organizationId: this.props.organizations[0].id,
     };
   }
 
@@ -87,7 +129,7 @@ class NotificationSettingsByType extends DeprecatedAsyncComponent<Props, State>
       [
         'notificationSettings',
         `/users/me/notification-settings/`,
-        {query: getQueryParams(notificationType)},
+        {query: getQueryParams(notificationType), v2: 'serializer'},
       ],
       ['identities', `/users/me/identities/`, {query: {provider: 'slack'}}],
       [
@@ -318,6 +360,10 @@ class NotificationSettingsByType extends DeprecatedAsyncComponent<Props, State>
     });
   };
 
+  handleOrgChange = (option: {label: string; value: string}) => {
+    this.setState({organizationId: option.value});
+  };
+
   renderBody() {
     const {notificationType} = this.props;
     const {notificationSettings} = this.state;
@@ -350,22 +396,28 @@ class NotificationSettingsByType extends DeprecatedAsyncComponent<Props, State>
             fields={this.getFields()}
           />
         </Form>
-        {!isEverythingDisabled(notificationType, notificationSettings) &&
-          (isGroupedByProject(notificationType) ? (
-            <NotificationSettingsByProjects
-              notificationType={notificationType}
-              notificationSettings={notificationSettings}
-              onChange={this.getStateToPutForParent}
-              onSubmitSuccess={() => this.trackTuningUpdated('project')}
-            />
-          ) : (
-            <NotificationSettingsByOrganization
-              notificationType={notificationType}
-              notificationSettings={notificationSettings}
-              onChange={this.getStateToPutForParent}
-              onSubmitSuccess={() => this.trackTuningUpdated('organization')}
-            />
-          ))}
+        {!isEverythingDisabled(notificationType, notificationSettings) && (
+          <Panel>
+            {isGroupedByProject(notificationType) ? (
+              <NotificationSettingsByProjects
+                notificationType={notificationType}
+                notificationSettings={notificationSettings}
+                onChange={this.getStateToPutForParent}
+                onSubmitSuccess={() => this.trackTuningUpdated('project')}
+                organizations={this.props.organizations}
+                organizationId={this.state.organizationId}
+                handleOrgChange={this.handleOrgChange}
+              />
+            ) : (
+              <NotificationSettingsByOrganization
+                notificationType={notificationType}
+                notificationSettings={notificationSettings}
+                onChange={this.getStateToPutForParent}
+                onSubmitSuccess={() => this.trackTuningUpdated('organization')}
+              />
+            )}
+          </Panel>
+        )}
       </Fragment>
     );
   }

+ 11 - 0
static/app/views/settings/account/notifications/utils.tsx

@@ -53,7 +53,18 @@ export const getFallBackValue = (notificationType: string): string => {
       return 'committed_only';
     case 'workflow':
       return 'subscribe_only';
+    case 'approval':
+      return 'always';
+    case 'quota':
+      return 'always';
+    case 'spikeProtection':
+      return 'always';
+    case 'reports':
+      return 'always';
     default:
+      // These are the expected potential settings with fallback of ''
+      // issue, quotaErrors, quotaTransactions, quotaAttachments,
+      // quotaReplays, quotaWarnings, quotaSpendAllocations
       return '';
   }
 };