Merge pull request #5660 from getsentry/ui/more-form-improvements

ui: various improvements to forms

src/sentry/static/sentry/app/components/forms/apiForm.jsx

@@ -43,13 +43,12 @@ export default class ApiForm extends Form {
           method: this.props.apiMethod,
           data: data,
           success: result => {
+            IndicatorStore.remove(loadingIndicator);
             this.onSubmitSuccess(result);
           },
           error: error => {
-            this.onSubmitError(error);
-          },
-          complete: () => {
             IndicatorStore.remove(loadingIndicator);
+            this.onSubmitError(error);
           }
         });
       }

src/sentry/static/sentry/app/components/forms/form.jsx

@@ -6,6 +6,8 @@ import {t} from '../../locale';
 
 export default class Form extends React.Component {
   static propTypes = {
+    cancelLabel: React.PropTypes.string,
+    onCancel: React.PropTypes.func,
     onSubmit: React.PropTypes.func.isRequired,
     onSubmitSuccess: React.PropTypes.func,
     onSubmitError: React.PropTypes.func,
@@ -13,14 +15,17 @@ export default class Form extends React.Component {
     submitLabel: React.PropTypes.string,
     footerClass: React.PropTypes.string,
     extraButton: React.PropTypes.element,
-    initialData: React.PropTypes.object
+    initialData: React.PropTypes.object,
+    requireChanges: React.PropTypes.bool
   };
 
   static defaultProps = {
+    cancelLabel: t('Cancel'),
     submitLabel: t('Save Changes'),
     submitDisabled: false,
     footerClass: 'form-actions align-right',
-    className: 'form-stacked'
+    className: 'form-stacked',
+    requireChanges: false
   };
 
   static childContextTypes = {
@@ -88,7 +93,10 @@ export default class Form extends React.Component {
   render() {
     let isSaving = this.state.state === FormState.SAVING;
     let {initialData, data} = this.state;
-    let hasChanges = Object.keys(data).length && !underscore.isEqual(data, initialData);
+    let {requireChanges} = this.props;
+    let hasChanges = requireChanges
+      ? Object.keys(data).length && !underscore.isEqual(data, initialData)
+      : true;
     return (
       <form onSubmit={this.onSubmit} className={this.props.className}>
         {this.state.state === FormState.ERROR &&
@@ -105,6 +113,14 @@ export default class Form extends React.Component {
             type="submit">
             {this.props.submitLabel}
           </button>
+          {this.props.onCancel &&
+            <button
+              className="btn btn-default"
+              disabled={isSaving}
+              onClick={this.props.onCancel}
+              style={{marginLeft: 5}}>
+              {this.props.cancelLabel}
+            </button>}
           {this.props.extraButton}
         </div>
       </form>

src/sentry/static/sentry/app/components/forms/passwordField.jsx

@@ -1,6 +1,6 @@
 import React from 'react';
 import InputField from './inputField';
-import {FormState} from './state';
+import FormState from './state';
 
 // TODO(dcramer): im not entirely sure this is working correctly with
 // value propagation in all scenarios

src/sentry/static/sentry/app/options.jsx

@@ -150,7 +150,7 @@ function optionsForSection(section) {
   return definitions.filter(option => option.key.split('.')[0] === section.key);
 }
 
-export function getOptionField(option, onChange, value, field) {
+export function getOptionField(option, field, value, onChange) {
   let meta = {...getOption(option), ...field};
   let Field = meta.component || TextField;
   return (

src/sentry/static/sentry/app/views/adminSettings.jsx

@@ -1,13 +1,10 @@
 import React from 'react';
 import _ from 'underscore';
 
-import AlertActions from '../actions/alertActions';
-import ApiMixin from '../mixins/apiMixin';
-import IndicatorStore from '../stores/indicatorStore';
-import LoadingIndicator from '../components/loadingIndicator';
+import AsyncView from './asyncView';
 import {t} from '../locale';
 import {getOption, getOptionField} from '../options';
-import {Form} from '../components/forms';
+import {ApiForm} from '../components/forms';
 
 const optionsAvailable = [
   'system.url-prefix',
@@ -21,190 +18,52 @@ const optionsAvailable = [
   'api.rate-limit.org-create'
 ];
 
-const SettingsList = React.createClass({
-  propTypes: {
-    formDisabled: React.PropTypes.bool,
-    options: React.PropTypes.object.isRequired,
-    onSubmit: React.PropTypes.func.isRequired
-  },
+export default class AdminSettings extends AsyncView {
+  getEndpoint() {
+    return '/internal/options/';
+  }
+
+  renderBody() {
+    let {data} = this.state;
 
-  getInitialState() {
-    let options = this.props.options;
-    let formData = {};
-    let required = [];
+    let initialData = {};
     let fields = {};
     for (let key of optionsAvailable) {
       // TODO(dcramer): we should not be mutating options
-      let option = options[key] || {field: {}};
+      let option = data[key] || {field: {}};
       if (_.isUndefined(option.value) || option.value === '') {
         let defn = getOption(key);
-        formData[key] = defn.defaultValue ? defn.defaultValue() : '';
+        initialData[key] = defn.defaultValue ? defn.defaultValue() : '';
       } else {
-        formData[key] = option.value;
-      }
-      if (option.field.required) {
-        required.push(key);
+        initialData[key] = option.value;
       }
-      fields[key] = getOptionField(
-        key,
-        this.onFieldChange.bind(this, key),
-        formData[key],
-        option.field
-      );
+      fields[key] = getOptionField(key, option.field);
     }
 
-    return {
-      required: required,
-      formData: formData,
-      fields: fields
-    };
-  },
-
-  onFieldChange(name, value) {
-    let formData = this.state.formData;
-    formData[name] = value;
-    this.setState({
-      formData: formData
-    });
-  },
-
-  onSubmit(e) {
-    this.props.onSubmit(this.state.formData);
-  },
-
-  render() {
-    let {fields, required, formData} = this.state;
-    let formValid = !required.filter(option => !formData[option]).length;
-    let submitDisabled = !formValid || this.props.formDisabled;
-
-    return (
-      <Form onSubmit={this.onSubmit} submitDisabled={submitDisabled}>
-        <h4>General</h4>
-        {fields['system.url-prefix']}
-        {fields['system.admin-email']}
-        {fields['system.support-email']}
-        {fields['system.security-email']}
-        {fields['system.rate-limit']}
-
-        <h4>Security & Abuse</h4>
-        {fields['auth.allow-registration']}
-        {fields['auth.ip-rate-limit']}
-        {fields['auth.user-rate-limit']}
-        {fields['api.rate-limit.org-create']}
-      </Form>
-    );
-  }
-});
-
-const AdminSettings = React.createClass({
-  mixins: [ApiMixin],
-
-  getInitialState() {
-    return {
-      loading: true,
-      error: false,
-      submitInProgress: false,
-      submitError: null,
-      options: {}
-    };
-  },
-
-  componentWillMount() {
-    this.fetchData();
-  },
-
-  remountComponent() {
-    this.setState(this.getInitialState(), this.fetchData);
-  },
-
-  fetchData(callback) {
-    this.api.request('/internal/options/', {
-      method: 'GET',
-      success: data => {
-        this.setState({
-          options: data,
-          loading: false,
-          error: false
-        });
-      },
-      error: () => {
-        this.setState({
-          loading: false,
-          error: true
-        });
-      }
-    });
-  },
-
-  onSubmit(formData) {
-    this.setState({
-      submitInProgress: true,
-      submitError: false
-    });
-    let loadingIndicator = IndicatorStore.add(t('Saving changes..'));
-
-    // We only want to send back the values which weren't disabled
-    formData = _.pick(formData, (value, key) => {
-      return !this.state.options[key].field.disabled;
-    });
-    this.api.request('/internal/options/', {
-      method: 'PUT',
-      data: formData,
-      success: () => {
-        this.setState({
-          submitInProgress: false
-        });
-        AlertActions.addAlert({
-          message: t('Your changes were saved, and will propagate to services shortly.'),
-          type: 'success'
-        });
-      },
-      error: () => {
-        this.setState({
-          submitInProgress: false,
-          submitError: true
-        });
-      },
-      complete: () => {
-        IndicatorStore.remove(loadingIndicator);
-      }
-    });
-  },
-
-  render() {
-    let {error, loading, options, submitError, submitInProgress} = this.state;
-
     return (
       <div>
         <h3>{t('Settings')}</h3>
 
-        {loading
-          ? <LoadingIndicator>
-              {t('Please wait while we load configuration.')}
-            </LoadingIndicator>
-          : error
-              ? <div className="loading-error">
-                  <span className="icon" />
-                  {t(
-                    'We were unable to load the required configuration from the Sentry server. Please take a look at the service logs.'
-                  )}
-                </div>
-              : <div>
-                  {submitError &&
-                    <div className="alert alert-block alert-error">
-                      {t(
-                        'We were unable to submit your changes to the Sentry server. Please take a look at the service logs.'
-                      )}
-                    </div>}
-                  <SettingsList
-                    options={options}
-                    onSubmit={this.onSubmit}
-                    formDisabled={submitInProgress}
-                  />
-                </div>}
+        <ApiForm
+          apiMethod="PUT"
+          apiEndpoint={this.getEndpoint()}
+          onSubmit={this.onSubmit}
+          initialData={initialData}
+          requireChanges={true}>
+          <h4>General</h4>
+          {fields['system.url-prefix']}
+          {fields['system.admin-email']}
+          {fields['system.support-email']}
+          {fields['system.security-email']}
+          {fields['system.rate-limit']}
+
+          <h4>Security & Abuse</h4>
+          {fields['auth.allow-registration']}
+          {fields['auth.ip-rate-limit']}
+          {fields['auth.user-rate-limit']}
+          {fields['api.rate-limit.org-create']}
+        </ApiForm>
       </div>
     );
   }
-});
-
-export default AdminSettings;
+}

src/sentry/static/sentry/app/views/apiNewToken.jsx

@@ -1,11 +1,9 @@
 import React from 'react';
-import DocumentTitle from 'react-document-title';
 import {browserHistory} from 'react-router';
 
-import ApiMixin from '../mixins/apiMixin';
-import IndicatorStore from '../stores/indicatorStore';
+import AsyncView from './asyncView';
 import NarrowLayout from '../components/narrowLayout';
-import {FormState, MultipleCheckboxField} from '../components/forms';
+import {ApiForm, MultipleCheckboxField} from '../components/forms';
 import {t, tct} from '../locale';
 
 const SCOPES = new Set([
@@ -35,167 +33,55 @@ const DEFAULT_SCOPES = new Set([
   'member:read'
 ]);
 
-const TokenForm = React.createClass({
-  propTypes: {
-    initialData: React.PropTypes.object,
-    onSave: React.PropTypes.func.isRequired,
-    onCancel: React.PropTypes.func.isRequired
-  },
-
-  mixins: [ApiMixin],
-
-  getInitialState() {
-    return {
-      formData: Object.assign({}, this.props.initialData),
-      errors: {}
-    };
-  },
-
-  onFieldChange(name, value) {
-    let formData = this.state.formData;
-    formData[name] = value;
-    this.setState({
-      formData: formData
-    });
-  },
-
-  onSubmit(e) {
-    e.preventDefault();
-
-    if (this.state.state == FormState.SAVING) {
-      return;
-    }
-    this.setState(
-      {
-        state: FormState.SAVING
-      },
-      () => {
-        let loadingIndicator = IndicatorStore.add(t('Saving changes..'));
-        this.api.request('/api-tokens/', {
-          method: 'POST',
-          data: this.state.formData,
-          success: data => {
-            this.setState({
-              state: FormState.READY,
-              errors: {}
-            });
-            IndicatorStore.remove(loadingIndicator);
-            this.props.onSave(data);
-          },
-          error: error => {
-            this.setState({
-              state: FormState.ERROR,
-              errors: error.responseJSON
-            });
-            IndicatorStore.remove(loadingIndicator);
-          }
-        });
-      }
-    );
-  },
-
-  render() {
-    let isSaving = this.state.state === FormState.SAVING;
-    let errors = this.state.errors;
-
-    return (
-      <form onSubmit={this.onSubmit} className="form-stacked api-new-token">
-        {this.state.state === FormState.ERROR &&
-          <div className="alert alert-error alert-block">
-            {t(
-              'Unable to save your changes. Please ensure all fields are valid and try again.'
-            )}
-          </div>}
-        <fieldset>
-          <MultipleCheckboxField
-            key="scopes"
-            name="scopes"
-            choices={Array.from(SCOPES.keys()).map(s => [s, s])}
-            label={t('Scopes')}
-            value={this.state.formData.scopes}
-            required={true}
-            error={errors.scopes}
-            onChange={this.onFieldChange.bind(this, 'scopes')}
-          />
-        </fieldset>
-        <fieldset className="form-actions">
-          <button
-            className="btn btn-default"
-            disabled={isSaving}
-            onClick={this.props.onCancel}>
-            {t('Cancel')}
-          </button>
-          <button
-            type="submit"
-            className="btn btn-primary pull-right"
-            disabled={isSaving}>
-            {t('Save Changes')}
-          </button>
-        </fieldset>
-      </form>
-    );
-  }
-});
-
-const ApiNewToken = React.createClass({
-  mixins: [ApiMixin],
-
-  getInitialState() {
-    return {
-      loading: true,
-      error: false
-    };
-  },
-
+export default class ApiNewToken extends AsyncView {
   getTitle() {
-    return 'Sentry API';
-  },
+    return 'Create API Token';
+  }
 
   onCancel() {
     browserHistory.pushState(null, '/api/');
-  },
+  }
 
-  onSave() {
+  onSubmitSuccess() {
     browserHistory.pushState(null, '/api/');
-  },
+  }
 
-  render() {
+  renderBody() {
     let defaultScopes = Array.from(DEFAULT_SCOPES);
     defaultScopes.sort();
 
     return (
-      <DocumentTitle title={this.getTitle()}>
-        <NarrowLayout>
-          <h3>{t('Create New Token')}</h3>
-
-          <hr />
-
-          <p>
-            {t(
-              "Authentication tokens allow you to perform actions against the Sentry API on behalf of your account. They're the easiest way to get started using the API."
-            )}
-          </p>
-          <p>
-            {tct(
-              'For more information on how to use the web API, see our [link:documentation].',
-              {
-                link: <a href="https://docs.sentry.io/hosted/api/" />
-              }
-            )}
-          </p>
-
-          <TokenForm
-            initialData={{
-              scopes: defaultScopes
-            }}
-            onCancel={this.onCancel}
-            onSave={this.onSave}
+      <NarrowLayout>
+        <h3>{t('Create New Token')}</h3>
+        <hr />
+        <p>
+          {t(
+            "Authentication tokens allow you to perform actions against the Sentry API on behalf of your account. They're the easiest way to get started using the API."
+          )}
+        </p>
+        <p>
+          {tct(
+            'For more information on how to use the web API, see our [link:documentation].',
+            {
+              link: <a href="https://docs.sentry.io/hosted/api/" />
+            }
+          )}
+        </p>
+        <ApiForm
+          apiMethod="POST"
+          apiEndpoint="/api-tokens/"
+          className="form-stacked api-new-token"
+          initialData={{scopes: defaultScopes}}
+          onSubmitSuccess={this.onSubmitSuccess}
+          onCancel={this.onCancel}>
+          <MultipleCheckboxField
+            name="scopes"
+            choices={Array.from(SCOPES.keys()).map(s => [s, s])}
+            label={t('Scopes')}
+            required={true}
           />
-
-        </NarrowLayout>
-      </DocumentTitle>
+        </ApiForm>
+      </NarrowLayout>
     );
   }
-});
-
-export default ApiNewToken;
+}

src/sentry/static/sentry/app/views/asyncView.jsx

@@ -63,10 +63,10 @@ class AsyncView extends React.Component {
             data: data
           });
         },
-        error: () => {
+        error: error => {
           this.setState({
             loading: false,
-            error: true
+            error: error
           });
         }
       });
@@ -89,8 +89,8 @@ class AsyncView extends React.Component {
     return <LoadingIndicator />;
   }
 
-  renderError(err) {
-    return <RouteError error={err} component={this} onRetry={this.remountComponent} />;
+  renderError(error) {
+    return <RouteError error={error} component={this} onRetry={this.remountComponent} />;
   }
 
   render() {
@@ -98,7 +98,7 @@ class AsyncView extends React.Component {
       <DocumentTitle title={this.getTitle()}>
         {this.state.loading
           ? this.renderLoading()
-          : this.state.error ? this.renderError() : this.renderBody()}
+          : this.state.error ? this.renderError(this.state.error) : this.renderBody()}
       </DocumentTitle>
     );
   }

src/sentry/static/sentry/app/views/installWizard.jsx

@@ -46,9 +46,9 @@ const InstallWizardSettings = React.createClass({
       }
       fields[key] = getOptionField(
         key,
-        this.onFieldChange.bind(this, key),
+        option.field,
         option.value,
-        option.field
+        this.onFieldChange.bind(this, key)
       );
       // options is used for submitting to the server, and we dont submit values
       // that are deleted

src/sentry/static/sentry/app/views/teamSettings.jsx

@@ -1,50 +1,50 @@
 import React from 'react';
 
+import AsyncView from './asyncView';
 import {ApiForm, TextField} from '../components/forms';
 import {t} from '../locale';
 
-export default React.createClass({
-  propTypes: {
+export default class TeamSettings extends AsyncView {
+  static propTypes = {
+    ...AsyncView.propTypes,
     team: React.PropTypes.object.isRequired,
     onTeamChange: React.PropTypes.func.isRequired
-  },
+  };
 
-  render() {
+  getTitle() {
+    return 'Team Settings';
+  }
+
+  renderBody() {
     let {orgId, teamId} = this.props.params;
     let team = this.props.team;
 
     return (
-      <div>
-        <div className="box">
-          <div className="box-content with-padding">
-            <ApiForm
-              apiMethod="PUT"
-              apiEndpoint={`/teams/${orgId}/${teamId}/`}
-              initialData={{
-                name: team.name,
-                slug: team.slug
-              }}
-              onSubmitSuccess={this.props.onTeamChange}
-              fields={[
-                {
-                  name: 'name',
-                  label: t('Name'),
-                  placeholder: t('e.g. API Team'),
-                  required: true,
-                  component: TextField
-                },
-                {
-                  name: 'slug',
-                  label: t('Short name'),
-                  placeholder: t('e.g. api-team'),
-                  required: true,
-                  component: TextField
-                }
-              ]}
+      <div className="box">
+        <div className="box-content with-padding">
+          <ApiForm
+            apiMethod="PUT"
+            apiEndpoint={`/teams/${orgId}/${teamId}/`}
+            initialData={{
+              name: team.name,
+              slug: team.slug
+            }}
+            onSubmitSuccess={this.props.onTeamChange}>
+            <TextField
+              name="name"
+              label={t('Name')}
+              placeholder={t('e.g. API Team')}
+              required={true}
+            />
+            <TextField
+              name="slug"
+              label={t('Short name')}
+              placeholder={t('e.g. api-team')}
+              required={true}
             />
-          </div>
+          </ApiForm>
         </div>
       </div>
     );
   }
-});
+}

tests/js/setup.js

@@ -16,5 +16,12 @@ window.TestStubs = {
     goForward: sinon.spy(),
     setRouteLeaveHook: sinon.spy(),
     isActive: sinon.spy()
-  })
+  }),
+  Team: (...params) => {
+    return {
+      slug: 'team-slug',
+      name: 'Team Name',
+      ...params
+    };
+  }
 };