Browse Source

fix(alert-rules): Fix non-descriptive error messages (#21277)

Priscila Oliveira 4 years ago
parent
commit
7bfc4841d0

+ 45 - 26
src/sentry/static/sentry/app/views/settings/projectAlerts/issueEditor/index.tsx

@@ -3,6 +3,7 @@ import {browserHistory} from 'react-router';
 import React from 'react';
 import classNames from 'classnames';
 import styled from '@emotion/styled';
+import omit from 'lodash/omit';
 
 import {ALL_ENVIRONMENTS_KEY} from 'app/constants';
 import {Environment, Organization, Project, OnboardingTaskKey} from 'app/types';
@@ -15,6 +16,7 @@ import {
 } from 'app/types/alerts';
 import {IconWarning, IconChevron} from 'app/icons';
 import {Panel, PanelBody, PanelHeader} from 'app/components/panels';
+import Input from 'app/views/settings/components/forms/controls/input';
 import {
   addErrorMessage,
   addLoadingMessage,
@@ -30,14 +32,13 @@ import Button from 'app/components/button';
 import Confirm from 'app/components/confirm';
 import Form from 'app/views/settings/components/forms/form';
 import LoadingMask from 'app/components/loadingMask';
-import PanelAlert from 'app/components/panels/panelAlert';
 import SelectField from 'app/views/settings/components/forms/selectField';
-import TextField from 'app/views/settings/components/forms/textField';
 import recreateRoute from 'app/utils/recreateRoute';
 import space from 'app/styles/space';
 import withOrganization from 'app/utils/withOrganization';
 import withProject from 'app/utils/withProject';
 import {updateOnboardingTask} from 'app/actionCreators/onboardingTasks';
+import Field from 'app/views/settings/components/forms/field';
 
 import RuleNodeList from './ruleNodeList';
 
@@ -300,7 +301,7 @@ class IssueRuleEditor extends AsyncView<Props, State> {
     this.setState(state => {
       const rule = {...state.rule} as IssueAlertRule;
       rule[prop] = val;
-      return {rule};
+      return {rule, detailedError: omit(state.detailedError, prop)};
     });
   };
 
@@ -382,6 +383,21 @@ class IssueRuleEditor extends AsyncView<Props, State> {
   handleChangeFilterProperty = (ruleIndex: number, prop: string, val: string) =>
     this.handlePropertyChange('filters', ruleIndex, prop, val);
 
+  handleValidateRuleName = () => {
+    const isRuleNameEmpty = !this.state.rule?.name.trim();
+
+    if (!isRuleNameEmpty) {
+      return;
+    }
+
+    this.setState(prevState => ({
+      detailedError: {
+        ...prevState.detailedError,
+        name: [t('Field Required')],
+      },
+    }));
+  };
+
   renderLoading() {
     return this.renderBody();
   }
@@ -447,11 +463,6 @@ class IssueRuleEditor extends AsyncView<Props, State> {
             {this.state.loading && <SemiTransparentLoadingMask />}
             <Panel>
               <PanelHeader>{t('Alert Setup')}</PanelHeader>
-
-              {this.hasError('name') && (
-                <PanelAlert type="error">{t('Must enter a rule name')}</PanelAlert>
-              )}
-
               <PanelBody>
                 <SelectField
                   className={classNames({
@@ -466,30 +477,32 @@ class IssueRuleEditor extends AsyncView<Props, State> {
                   onChange={val => this.handleEnvironmentChange(val)}
                   disabled={!hasAccess}
                 />
-                <TextField
+
+                <StyledField
                   label={t('Alert name')}
                   help={t('Add a name for this alert')}
-                  name="name"
-                  defaultValue={name}
-                  required
-                  placeholder={t('My Rule Name')}
-                  onChange={val => this.handleChange('name', val)}
+                  error={detailedError?.name?.[0]}
                   disabled={!hasAccess}
-                />
+                  required
+                  stacked
+                >
+                  <Input
+                    type="text"
+                    name="name"
+                    value={name}
+                    placeholder={t('My Rule Name')}
+                    onChange={(event: React.ChangeEvent<HTMLInputElement>) =>
+                      this.handleChange('name', event.target.value)
+                    }
+                    onBlur={this.handleValidateRuleName}
+                  />
+                </StyledField>
               </PanelBody>
             </Panel>
 
             <Panel>
               <PanelHeader>{t('Alert Conditions')}</PanelHeader>
               <PanelBody>
-                {detailedError && (
-                  <PanelAlert type="error">
-                    {t(
-                      'There was an error saving your changes. Make sure all fields are valid and try again.'
-                    )}
-                  </PanelAlert>
-                )}
-
                 <Step>
                   <StepConnector />
 
@@ -562,7 +575,7 @@ class IssueRuleEditor extends AsyncView<Props, State> {
                             error={
                               this.hasError('conditions') && (
                                 <StyledAlert type="error">
-                                  {this.state.detailedError?.conditions[0]}
+                                  {detailedError?.conditions[0]}
                                 </StyledAlert>
                               )
                             }
@@ -636,7 +649,7 @@ class IssueRuleEditor extends AsyncView<Props, State> {
                           error={
                             this.hasError('filters') && (
                               <StyledAlert type="error">
-                                {this.state.detailedError?.filters[0]}
+                                {detailedError?.filters[0]}
                               </StyledAlert>
                             )
                           }
@@ -676,7 +689,7 @@ class IssueRuleEditor extends AsyncView<Props, State> {
                         error={
                           this.hasError('actions') && (
                             <StyledAlert type="error">
-                              {this.state.detailedError?.actions[0]}
+                              {detailedError?.actions[0]}
                             </StyledAlert>
                           )
                         }
@@ -787,3 +800,9 @@ const SemiTransparentLoadingMask = styled(LoadingMask)`
   opacity: 0.6;
   z-index: 1; /* Needed so that it sits above form elements */
 `;
+
+const StyledField = styled(Field)`
+  :last-child {
+    padding-bottom: ${space(2)};
+  }
+`;