ref(ui): Shuffle things around so that we can store unsaved Triggers in Incident Rules (#15381)

* wip

* revert adding `routes` to RouterProps type

* wip

* ops

* fix merges

* fix optional trigger

src/sentry/static/sentry/app/types/index.tsx

@@ -2,7 +2,6 @@ import {SpanEntry} from 'app/components/events/interfaces/spans/types';
 import {API_SCOPES} from 'app/constants';
 import {Field} from 'app/views/settings/components/forms/type';
 import {Params} from 'react-router/lib/Router';
-import {PlainRoute} from 'react-router/lib/Route';
 import {Location} from 'history';
 
 export type ObjectStatus =
@@ -589,7 +588,6 @@ export type SentryAppComponent = {
 export type RouterProps = {
   params: Params;
   location: Location;
-  routes: PlainRoute[];
 };
 
 export type ActiveExperiments = {

src/sentry/static/sentry/app/views/settings/components/forms/form.tsx

@@ -26,8 +26,8 @@ function isRenderFunc(func: React.ReactNode | Function): func is RenderFunc {
 }
 
 type Props = {
-  apiMethod: APIRequestMethod;
-  apiEndpoint: string;
+  apiMethod?: APIRequestMethod;
+  apiEndpoint?: string;
   children: React.ReactNode | RenderFunc;
   className?: string;
   cancelLabel?: string;

src/sentry/static/sentry/app/views/settings/components/forms/model.tsx

@@ -392,10 +392,10 @@ class FormModel {
    */
   @action
   saveForm() {
-    this.validateForm();
-    if (this.isError) {
+    if (!this.validateForm()) {
       return null;
     }
+
     let saveSnapshot: SaveSnapshot = this.createSnapshot();
 
     const request = this.doApiRequest({
@@ -661,9 +661,14 @@ class FormModel {
     this.setFieldState(id, FormState.SAVING, false);
   }
 
+  /**
+   * Returns true if there are no errors
+   */
   @action
-  validateForm() {
+  validateForm(): boolean {
     Array.from(this.fieldDescriptor.keys()).forEach(id => !this.validateField(id));
+
+    return !this.isError;
   }
 
   @action

src/sentry/static/sentry/app/views/settings/incidentRules/actions.tsx

@@ -1,10 +1,10 @@
 import {Client} from 'app/api';
-import {IncidentRule, Trigger} from './types';
+import {SavedIncidentRule, Trigger} from './types';
 
 export function deleteRule(
   api: Client,
   orgId: string,
-  rule: IncidentRule
+  rule: SavedIncidentRule
 ): Promise<void> {
   return api.requestPromise(`/organizations/${orgId}/alert-rules/${rule.id}/`, {
     method: 'DELETE',

src/sentry/static/sentry/app/views/settings/incidentRules/create.tsx

@@ -1,37 +1,45 @@
 import {RouteComponentProps} from 'react-router/lib/Router';
 import React from 'react';
 
+import {AlertRuleAggregations} from 'app/views/settings/incidentRules/types';
+import {Organization, Project} from 'app/types';
 import recreateRoute from 'app/utils/recreateRoute';
+import withOrganization from 'app/utils/withOrganization';
+import withProject from 'app/utils/withProject';
 
 import RuleForm from './ruleForm';
 
-type RouteParams = {
-  orgId: string;
-  projectId: string;
+const DEFAULT_METRIC = [AlertRuleAggregations.TOTAL];
+const DEFAULT_RULE = {
+  aggregations: DEFAULT_METRIC,
+  query: '',
+  timeWindow: 60,
+  triggers: [],
 };
-type Props = {};
 
-class IncidentRulesCreate extends React.Component<
-  RouteComponentProps<RouteParams, {}> & Props
-> {
+type Props = {
+  organization: Organization;
+  project: Project;
+};
+
+class IncidentRulesCreate extends React.Component<RouteComponentProps<{}, {}> & Props> {
   handleSubmitSuccess = data => {
-    const {params, routes, location} = this.props;
+    const {params, routes, router, location} = this.props;
 
-    this.props.router.push(
-      recreateRoute(`${data.id}/`, {params, routes, location, stepBack: -1})
-    );
+    router.push(recreateRoute(`${data.id}/`, {params, routes, location, stepBack: -1}));
   };
 
   render() {
-    const {orgId, projectId} = this.props.params;
+    const {organization, project} = this.props;
 
     return (
       <RuleForm
-        orgId={orgId}
-        projectId={projectId}
+        organization={organization}
         onSubmitSuccess={this.handleSubmitSuccess}
+        rule={{...DEFAULT_RULE, projects: [project.slug]}}
       />
     );
   }
 }
-export default IncidentRulesCreate;
+
+export default withOrganization(withProject(IncidentRulesCreate));

src/sentry/static/sentry/app/views/settings/incidentRules/details.tsx

@@ -1,20 +1,10 @@
 import {RouteComponentProps} from 'react-router/lib/Router';
-import {findIndex} from 'lodash';
 import React from 'react';
-import styled, {css} from 'react-emotion';
 
-import {IncidentRule, Trigger} from 'app/views/settings/incidentRules/types';
+import {IncidentRule} from 'app/views/settings/incidentRules/types';
 import {Organization, Project} from 'app/types';
-import {addErrorMessage} from 'app/actionCreators/indicator';
-import {deleteTrigger} from 'app/views/settings/incidentRules/actions';
-import {openModal} from 'app/actionCreators/modal';
-import {t, tct} from 'app/locale';
 import AsyncView from 'app/views/asyncView';
-import Button from 'app/components/button';
 import RuleForm from 'app/views/settings/incidentRules/ruleForm';
-import SettingsPageHeader from 'app/views/settings/components/settingsPageHeader';
-import TriggersList from 'app/views/settings/incidentRules/triggers/list';
-import TriggersModal from 'app/views/settings/incidentRules/triggers/modal';
 import withOrganization from 'app/utils/withOrganization';
 import withProjects from 'app/utils/withProjects';
 
@@ -48,142 +38,15 @@ class IncidentRulesDetails extends AsyncView<
     ];
   }
 
-  openTriggersModal = (trigger?: Trigger) => {
-    const {organization, projects} = this.props;
-    const {rule} = this.state;
-
-    openModal(
-      ({closeModal}) => (
-        <TriggersModal
-          organization={organization}
-          projects={projects}
-          rule={rule}
-          trigger={trigger}
-          closeModal={closeModal}
-          onSubmitSuccess={trigger ? this.handleEditedTrigger : this.handleAddedTrigger}
-        />
-      ),
-      {
-        dialogClassName: css`
-          width: 80%;
-          margin-left: -40%;
-        `,
-      }
-    );
-  };
-
-  handleAddedTrigger = (trigger: Trigger) => {
-    this.setState(({rule}) => ({
-      rule: {
-        ...rule,
-        triggers: [...rule.triggers, trigger],
-      },
-    }));
-  };
-
-  handleEditedTrigger = (trigger: Trigger) => {
-    this.setState(({rule}) => {
-      const triggerIndex = findIndex(rule.triggers, ({id}) => id === trigger.id);
-      const triggers = [...rule.triggers];
-      triggers.splice(triggerIndex, 1, trigger);
-
-      return {
-        rule: {
-          ...rule,
-          triggers,
-        },
-      };
-    });
-  };
-
-  handleNewTrigger = () => {
-    this.openTriggersModal();
-  };
-
-  handleEditTrigger = (trigger: Trigger) => {
-    this.openTriggersModal(trigger);
-  };
-
-  handleDeleteTrigger = async (trigger: Trigger) => {
-    const {organization} = this.props;
-
-    // Optimistically update
-    const triggerIndex = findIndex(this.state.rule.triggers, ({id}) => id === trigger.id);
-    const triggersAfterDelete = [...this.state.rule.triggers];
-    triggersAfterDelete.splice(triggerIndex, 1);
-
-    this.setState(({rule}) => {
-      return {
-        rule: {
-          ...rule,
-          triggers: triggersAfterDelete,
-        },
-      };
-    });
-
-    try {
-      await deleteTrigger(this.api, organization.slug, trigger);
-    } catch (err) {
-      addErrorMessage(
-        tct('There was a problem deleting trigger: [label]', {label: trigger.label})
-      );
-
-      // Add trigger back to list
-      this.setState(({rule}) => {
-        const triggers = [...rule.triggers];
-        triggers.splice(triggerIndex, 0, trigger);
-
-        return {
-          rule: {
-            ...rule,
-            triggers,
-          },
-        };
-      });
-    }
-  };
-
   renderBody() {
-    const {orgId, projectId, incidentRuleId} = this.props.params;
+    const {organization, params} = this.props;
+    const {incidentRuleId} = params;
     const {rule} = this.state;
 
     return (
-      <React.Fragment>
-        <RuleForm
-          saveOnBlur
-          projectId={projectId}
-          orgId={orgId}
-          incidentRuleId={incidentRuleId}
-          initialData={rule}
-        />
-
-        <TriggersHeader
-          title={t('Triggers')}
-          action={
-            <Button
-              size="small"
-              priority="primary"
-              icon="icon-circle-add"
-              disabled={!rule}
-              onClick={this.handleNewTrigger}
-            >
-              {t('New Trigger')}
-            </Button>
-          }
-        />
-
-        <TriggersList
-          triggers={rule.triggers}
-          onDelete={this.handleDeleteTrigger}
-          onEdit={this.handleEditTrigger}
-        />
-      </React.Fragment>
+      <RuleForm organization={organization} incidentRuleId={incidentRuleId} rule={rule} />
     );
   }
 }
 
 export default withProjects(withOrganization(IncidentRulesDetails));
-
-const TriggersHeader = styled(SettingsPageHeader)`
-  margin: 0;
-`;

src/sentry/static/sentry/app/views/settings/incidentRules/list.tsx

@@ -13,12 +13,12 @@ import LoadingIndicator from 'app/components/loadingIndicator';
 import recreateRoute from 'app/utils/recreateRoute';
 import space from 'app/styles/space';
 
-import {IncidentRule} from './types';
+import {SavedIncidentRule} from './types';
 import {deleteRule} from './actions';
 import getMetricDisplayName from './utils/getMetricDisplayName';
 
 type State = {
-  rules: IncidentRule[];
+  rules: SavedIncidentRule[];
 } & AsyncView['state'];
 
 type RouteParams = {
@@ -35,7 +35,7 @@ class IncidentRulesList extends AsyncView<Props, State> {
     return [['rules', `/organizations/${orgId}/alert-rules/`] as [string, string]];
   }
 
-  handleRemoveRule = async (rule: IncidentRule) => {
+  handleRemoveRule = async (rule: SavedIncidentRule) => {
     const {orgId} = this.props.params;
 
     // Optimistic update

src/sentry/static/sentry/app/views/settings/incidentRules/ruleForm.tsx

@@ -1,167 +0,0 @@
-import React from 'react';
-
-import {Client} from 'app/api';
-import {Organization} from 'app/types';
-import {t} from 'app/locale';
-import Form from 'app/views/settings/components/forms/form';
-import FormField from 'app/views/settings/components/forms/formField';
-import JsonForm from 'app/views/settings/components/forms/jsonForm';
-import SearchBar from 'app/views/events/searchBar';
-import withApi from 'app/utils/withApi';
-import withOrganization from 'app/utils/withOrganization';
-
-import {AlertRuleAggregations, IncidentRule, TimeWindow} from './types';
-import getMetricDisplayName from './utils/getMetricDisplayName';
-
-const DEFAULT_METRIC = [AlertRuleAggregations.TOTAL];
-
-type Props = {
-  api: Client;
-  organization: Organization;
-  initialData?: IncidentRule;
-};
-
-type TimeWindowMapType = {[key in TimeWindow]: string};
-
-const TIME_WINDOW_MAP: TimeWindowMapType = {
-  [TimeWindow.ONE_MINUTE]: t('1 minute'),
-  [TimeWindow.FIVE_MINUTES]: t('5 minutes'),
-  [TimeWindow.TEN_MINUTES]: t('10 minutes'),
-  [TimeWindow.FIFTEEN_MINUTES]: t('15 minutes'),
-  [TimeWindow.THIRTY_MINUTES]: t('30 minutes'),
-  [TimeWindow.ONE_HOUR]: t('1 hour'),
-  [TimeWindow.TWO_HOURS]: t('2 hours'),
-  [TimeWindow.FOUR_HOURS]: t('4 hours'),
-  [TimeWindow.ONE_DAY]: t('24 hours'),
-};
-
-class RuleForm extends React.Component<Props> {
-  render() {
-    const {organization} = this.props;
-
-    return (
-      <JsonForm
-        forms={[
-          {
-            title: t('Metric'),
-            fields: [
-              {
-                name: 'name',
-                type: 'text',
-                label: t('Name'),
-                help: t('Give your Incident Rule a name so it is easy to manage later'),
-                placeholder: t('My Incident Rule Name'),
-                required: true,
-              },
-              {
-                name: 'aggregations',
-                type: 'select',
-                label: t('Metric'),
-                help: t('Choose which metric to trigger on'),
-                choices: [
-                  [
-                    AlertRuleAggregations.UNIQUE_USERS,
-                    getMetricDisplayName(AlertRuleAggregations.UNIQUE_USERS),
-                  ],
-                  [
-                    AlertRuleAggregations.TOTAL,
-                    getMetricDisplayName(AlertRuleAggregations.TOTAL),
-                  ],
-                ],
-                required: true,
-                setValue: value => (value && value.length ? value[0] : value),
-                getValue: value => [value],
-              },
-              {
-                name: 'query',
-                type: 'custom',
-                label: t('Filter'),
-                defaultValue: '',
-                placeholder: 'error.type:TypeError',
-                help: t(
-                  'You can apply standard Sentry filter syntax to filter by status, user, etc.'
-                ),
-                Component: props => {
-                  return (
-                    <FormField {...props}>
-                      {({onChange, onBlur, onKeyDown}) => {
-                        return (
-                          <SearchBar
-                            useFormWrapper={false}
-                            organization={organization}
-                            onChange={onChange}
-                            onBlur={onBlur}
-                            onKeyDown={onKeyDown}
-                            onSearch={query => onChange(query, {})}
-                          />
-                        );
-                      }}
-                    </FormField>
-                  );
-                },
-              },
-              {
-                name: 'timeWindow',
-                type: 'select',
-                label: t('Time Window'),
-                help: t('The time window to use when evaluating the Metric'),
-                choices: Object.entries(TIME_WINDOW_MAP),
-                required: true,
-              },
-            ],
-          },
-        ]}
-      />
-    );
-  }
-}
-
-type RuleFormContainerProps = {
-  initialData?: IncidentRule;
-  orgId: string;
-  projectId: string;
-  incidentRuleId?: string;
-  saveOnBlur?: boolean;
-} & React.ComponentProps<typeof RuleForm> & {
-    onSubmitSuccess?: Form['props']['onSubmitSuccess'];
-  };
-
-function RuleFormContainer({
-  orgId,
-  projectId,
-  incidentRuleId,
-  initialData,
-  saveOnBlur,
-  onSubmitSuccess,
-  ...props
-}: RuleFormContainerProps) {
-  return (
-    <Form
-      apiMethod={incidentRuleId ? 'PUT' : 'POST'}
-      apiEndpoint={`/organizations/${orgId}/alert-rules/${
-        incidentRuleId ? `${incidentRuleId}/` : ''
-      }`}
-      initialData={{
-        query: '',
-        aggregations: DEFAULT_METRIC,
-        projects: [projectId],
-        includeAllProjects: false,
-        excludedProjects: [],
-
-        // TODO(incidents): Temp values
-        alertThreshold: 5,
-        resolveThreshold: 1,
-        thresholdType: 0,
-        timeWindow: 60,
-        ...initialData,
-      }}
-      saveOnBlur={saveOnBlur}
-      onSubmitSuccess={onSubmitSuccess}
-    >
-      <RuleForm initialData={initialData} {...props} />
-    </Form>
-  );
-}
-
-export {RuleFormContainer};
-export default withApi(withOrganization(RuleFormContainer));

src/sentry/static/sentry/app/views/settings/incidentRules/ruleForm/index.tsx

@@ -0,0 +1,137 @@
+import {findIndex} from 'lodash';
+import React from 'react';
+
+import {Project} from 'app/types';
+import {addErrorMessage} from 'app/actionCreators/indicator';
+import {deleteTrigger} from 'app/views/settings/incidentRules/actions';
+import {tct} from 'app/locale';
+import Form from 'app/views/settings/components/forms/form';
+import RuleForm from 'app/views/settings/incidentRules/ruleForm/ruleForm';
+import SentryTypes from 'app/sentryTypes';
+import Triggers from 'app/views/settings/incidentRules/triggers';
+import withApi from 'app/utils/withApi';
+import withConfig from 'app/utils/withConfig';
+import withProject from 'app/utils/withProject';
+
+import {IncidentRule, Trigger} from '../types';
+
+type Props = {
+  project: Project;
+  rule: IncidentRule;
+  incidentRuleId?: string;
+} & Pick<React.ComponentProps<typeof RuleForm>, 'api' | 'config' | 'organization'> & {
+    onSubmitSuccess?: Form['props']['onSubmitSuccess'];
+  };
+
+type State = {
+  rule: IncidentRule;
+};
+
+class RuleFormContainer extends React.Component<Props, State> {
+  static contextTypes = {
+    project: SentryTypes.Project,
+  };
+
+  state = {
+    rule: this.props.rule,
+    projects: [this.props.project],
+  };
+
+  handleAddTrigger = (trigger: Trigger) => {
+    this.setState(({rule}) => ({
+      rule: {
+        ...rule,
+        triggers: [...rule.triggers, trigger],
+      },
+    }));
+  };
+
+  handleEditTrigger = (trigger: Trigger) => {
+    this.setState(({rule}) => {
+      const triggerIndex = findIndex(rule.triggers, ({id}) => id === trigger.id);
+      const triggers = [...rule.triggers];
+      triggers.splice(triggerIndex, 1, trigger);
+
+      return {
+        rule: {
+          ...rule,
+          triggers,
+        },
+      };
+    });
+  };
+
+  handleDeleteTrigger = async (trigger: Trigger) => {
+    const {api, organization} = this.props;
+
+    // Optimistically update
+    const triggerIndex = findIndex(this.state.rule.triggers, ({id}) => id === trigger.id);
+    const triggersAfterDelete = [...this.state.rule.triggers];
+    triggersAfterDelete.splice(triggerIndex, 1);
+
+    this.setState(({rule}) => {
+      return {
+        rule: {
+          ...rule,
+          triggers: triggersAfterDelete,
+        },
+      };
+    });
+
+    // Trigger is potentially unsaved if it does not have an id, so don't try to remove from server
+    if (!trigger.id) {
+      return;
+    }
+
+    try {
+      await deleteTrigger(api, organization.slug, trigger);
+    } catch (err) {
+      addErrorMessage(
+        tct('There was a problem deleting trigger: [label]', {label: trigger.label})
+      );
+
+      // Add trigger back to list
+      this.setState(({rule}) => {
+        const triggers = [...rule.triggers];
+        triggers.splice(triggerIndex, 0, trigger);
+
+        return {
+          rule: {
+            ...rule,
+            triggers,
+          },
+        };
+      });
+    }
+  };
+
+  render() {
+    const {api, config, organization, incidentRuleId, onSubmitSuccess} = this.props;
+    const {rule} = this.state;
+
+    return (
+      <Form
+        apiMethod={incidentRuleId ? 'PUT' : 'POST'}
+        apiEndpoint={`/organizations/${organization.slug}/alert-rules/${
+          incidentRuleId ? `${incidentRuleId}/` : ''
+        }`}
+        initialData={rule}
+        saveOnBlur={false}
+        onSubmitSuccess={onSubmitSuccess}
+      >
+        <RuleForm api={api} config={config} organization={organization} rule={rule} />
+        <Triggers
+          rule={rule}
+          organization={organization}
+          incidentRuleId={incidentRuleId}
+          onDelete={this.handleDeleteTrigger}
+          onEdit={this.handleEditTrigger}
+          onAdd={this.handleAddTrigger}
+        />
+      </Form>
+    );
+  }
+}
+
+export {RuleFormContainer};
+export default withConfig(withApi(withProject(RuleFormContainer)));

src/sentry/static/sentry/app/views/settings/incidentRules/ruleForm/ruleForm.tsx

@@ -0,0 +1,117 @@
+import React from 'react';
+
+import {Client} from 'app/api';
+import {Config, Organization} from 'app/types';
+import {t} from 'app/locale';
+import FormField from 'app/views/settings/components/forms/formField';
+import JsonForm from 'app/views/settings/components/forms/jsonForm';
+import SearchBar from 'app/views/events/searchBar';
+
+import {AlertRuleAggregations, IncidentRule, TimeWindow} from '../types';
+import getMetricDisplayName from '../utils/getMetricDisplayName';
+
+type Props = {
+  api: Client;
+  config: Config;
+  organization: Organization;
+  rule?: IncidentRule;
+};
+
+type TimeWindowMapType = {[key in TimeWindow]: string};
+
+const TIME_WINDOW_MAP: TimeWindowMapType = {
+  [TimeWindow.ONE_MINUTE]: t('1 minute'),
+  [TimeWindow.FIVE_MINUTES]: t('5 minutes'),
+  [TimeWindow.TEN_MINUTES]: t('10 minutes'),
+  [TimeWindow.FIFTEEN_MINUTES]: t('15 minutes'),
+  [TimeWindow.THIRTY_MINUTES]: t('30 minutes'),
+  [TimeWindow.ONE_HOUR]: t('1 hour'),
+  [TimeWindow.TWO_HOURS]: t('2 hours'),
+  [TimeWindow.FOUR_HOURS]: t('4 hours'),
+  [TimeWindow.ONE_DAY]: t('24 hours'),
+};
+
+class RuleForm extends React.Component<Props> {
+  render() {
+    const {organization} = this.props;
+
+    return (
+      <React.Fragment>
+        <JsonForm
+          forms={[
+            {
+              title: t('Metric'),
+              fields: [
+                {
+                  name: 'name',
+                  type: 'text',
+                  label: t('Name'),
+                  help: t('Give your Incident Rule a name so it is easy to manage later'),
+                  placeholder: t('My Incident Rule Name'),
+                  required: true,
+                },
+                {
+                  name: 'aggregations',
+                  type: 'select',
+                  label: t('Metric'),
+                  help: t('Choose which metric to trigger on'),
+                  choices: [
+                    [
+                      AlertRuleAggregations.UNIQUE_USERS,
+                      getMetricDisplayName(AlertRuleAggregations.UNIQUE_USERS),
+                    ],
+                    [
+                      AlertRuleAggregations.TOTAL,
+                      getMetricDisplayName(AlertRuleAggregations.TOTAL),
+                    ],
+                  ],
+                  required: true,
+                  setValue: value => (value && value.length ? value[0] : value),
+                  getValue: value => [value],
+                },
+                {
+                  name: 'query',
+                  type: 'custom',
+                  label: t('Filter'),
+                  defaultValue: '',
+                  placeholder: 'error.type:TypeError',
+                  help: t(
+                    'You can apply standard Sentry filter syntax to filter by status, user, etc.'
+                  ),
+                  Component: props => {
+                    return (
+                      <FormField {...props}>
+                        {({onChange, onBlur, onKeyDown}) => {
+                          return (
+                            <SearchBar
+                              useFormWrapper={false}
+                              organization={organization}
+                              onChange={onChange}
+                              onBlur={onBlur}
+                              onKeyDown={onKeyDown}
+                              onSearch={query => onChange(query, {})}
+                            />
+                          );
+                        }}
+                      </FormField>
+                    );
+                  },
+                },
+                {
+                  name: 'timeWindow',
+                  type: 'select',
+                  label: t('Time Window'),
+                  help: t('The time window to use when evaluating the Metric'),
+                  choices: Object.entries(TIME_WINDOW_MAP),
+                  required: true,
+                },
+              ],
+            },
+          ]}
+        />
+      </React.Fragment>
+    );
+  }
+}
+
+export default RuleForm;