ref(form): Update the logic for the submit button (#75776)

static/app/components/forms/form.tsx

@@ -6,7 +6,7 @@ import type {ButtonProps} from 'sentry/components/button';
 import {Button} from 'sentry/components/button';
 import FormContext from 'sentry/components/forms/formContext';
 import type {FormOptions} from 'sentry/components/forms/model';
-import FormModel from 'sentry/components/forms/model';
+import FormModel, {fieldIsRequiredMessage} from 'sentry/components/forms/model';
 import type {Data, OnSubmitCallback} from 'sentry/components/forms/types';
 import Panel from 'sentry/components/panels/panel';
 import {t} from 'sentry/locale';
@@ -80,6 +80,35 @@ export interface FormProps
   submitPriority?: ButtonProps['priority'];
 }
 
+function getSubmitButtonTitle(form: FormModel) {
+  if (form.isFormIncomplete) {
+    return t('Required fields must be filled out');
+  }
+
+  if (!form.isError) {
+    return undefined;
+  }
+
+  const errors = form.getErrors();
+  const hasRequiredFieldError = [...errors].some(
+    ([_field, message]) => message === fieldIsRequiredMessage
+  );
+
+  if (hasRequiredFieldError) {
+    const allRequiredFieldErrors = [...errors].every(
+      ([_field, message]) => message === fieldIsRequiredMessage
+    );
+
+    if (allRequiredFieldErrors) {
+      return t('Required fields must be filled out');
+    }
+
+    return t('Required fields must be filled out and inputs must be valid');
+  }
+
+  return t('Fields must contain valid inputs');
+}
+
 function Form({
   'data-test-id': dataTestId,
   allowUndo,
@@ -135,7 +164,7 @@ function Form({
     return resolvedModel;
   });
 
-  // Reset form model on un,out
+  // Reset form model on unmount
   useEffect(
     () => () => {
       if (!preventFormResetOnUnmount) {
@@ -237,9 +266,11 @@ function Form({
               <Observer>
                 {() => (
                   <Button
+                    title={getSubmitButtonTitle(formModel)}
                     data-test-id="form-submit"
                     priority={submitPriority ?? 'primary'}
                     disabled={
+                      formModel.isFormIncomplete ||
                       formModel.isError ||
                       formModel.isSaving ||
                       submitDisabled ||

static/app/components/forms/model.tsx

@@ -10,6 +10,8 @@ import {t} from 'sentry/locale';
 import type {Choice} from 'sentry/types/core';
 import {defined} from 'sentry/utils';
 
+export const fieldIsRequiredMessage = t('Field is required');
+
 type Snapshot = Map<string, FieldValue>;
 type SaveSnapshot = (() => number) | null;
 
@@ -135,11 +137,13 @@ class FormModel {
       fieldState: observable,
       formState: observable,
 
+      isFormIncomplete: computed,
       isError: computed,
       isSaving: computed,
       formData: computed,
       formChanged: computed,
 
+      validateFormCompletion: action,
       resetForm: action,
       setFieldDescriptor: action,
       removeField: action,
@@ -200,6 +204,13 @@ class FormModel {
     return this.fields;
   }
 
+  /**
+   * Are all required fields filled out
+   */
+  get isFormIncomplete() {
+    return this.formState === FormState.INCOMPLETE;
+  }
+
   /**
    * Is form saving
    */
@@ -240,7 +251,7 @@ class FormModel {
     // TODO(TS): add type to props
     this.fieldDescriptor.set(id, props);
 
-    // Set default value iff initialData for field is undefined
+    // Set default value if initialData for field is undefined
     // This must take place before checking for `props.setValue` so that it can
     // be applied to `defaultValue`
     if (
@@ -259,6 +270,8 @@ class FormModel {
       this.initialData[id] = props.setValue(this.initialData[id], props);
       this.fields.set(id, this.initialData[id]);
     }
+
+    this.validateFormCompletion();
   }
 
   /**
@@ -348,14 +361,28 @@ class FormModel {
     return this.errors.has(id) && this.errors.get(id);
   }
 
+  getErrors() {
+    return this.errors;
+  }
+
   /**
    * Returns true if not required or is required and is not empty
    */
   isValidRequiredField(id: string) {
     // Check field descriptor to see if field is required
     const isRequired = this.getDescriptor(id, 'required');
+
+    if (!isRequired) {
+      return true;
+    }
+
     const value = this.getValue(id);
-    return !isRequired || (value !== '' && defined(value));
+
+    if (Array.isArray(value)) {
+      return value.length > 0;
+    }
+
+    return value !== '' && defined(value);
   }
 
   isValidField(id: string) {
@@ -416,8 +443,6 @@ class FormModel {
       errors = validate({model: this, id, form: this.getData()}) || [];
     }
 
-    const fieldIsRequiredMessage = t('Field is required');
-
     if (!this.isValidRequiredField(id)) {
       errors.push([id, fieldIsRequiredMessage]);
     }
@@ -722,7 +747,7 @@ class FormModel {
       this.formState = FormState.ERROR;
       this.errors.set(id, error);
     } else {
-      this.formState = FormState.READY;
+      this.validateFormCompletion();
       this.errors.delete(id);
     }
 
@@ -739,6 +764,17 @@ class FormModel {
     return !this.isError;
   }
 
+  /**
+   * Validate if all required fields are filled out
+   */
+  validateFormCompletion() {
+    const formComplete = Array.from(this.fieldDescriptor.keys()).every(field =>
+      this.isValidRequiredField(field)
+    );
+
+    this.formState = !formComplete ? FormState.INCOMPLETE : FormState.READY;
+  }
+
   handleErrorResponse({responseJSON: resp}: {responseJSON?: any} = {}) {
     if (!resp) {
       return;

static/app/components/forms/state.tsx

@@ -5,6 +5,7 @@ enum FormState {
   READY = 'Ready',
   SAVING = 'Saving',
   ERROR = 'Error',
+  INCOMPLETE = 'Incomplete',
 }
 
 export default FormState;

static/app/views/alerts/rules/issue/sentryAppRuleModal.spec.tsx

@@ -35,11 +35,6 @@ describe('SentryAppRuleModal', function () {
     expect(errors).toHaveLength(0);
   };
 
-  const submitErrors = async errorCount => {
-    const errors = await _submit();
-    expect(errors).toHaveLength(errorCount);
-  };
-
   const defaultConfig: SchemaFormConfig = {
     uri: '/integration/test/',
     description: '',
@@ -132,9 +127,13 @@ describe('SentryAppRuleModal', function () {
       });
     });
 
-    it('should raise validation errors when "Save Changes" is clicked with invalid data', async function () {
+    it('submit button shall be disabled if form is incomplete', async function () {
       createWrapper();
-      await submitErrors(3);
+      expect(screen.getByRole('button', {name: 'Save Changes'})).toBeDisabled();
+      await userEvent.hover(screen.getByRole('button', {name: 'Save Changes'}));
+      expect(
+        await screen.findByText('Required fields must be filled out')
+      ).toBeInTheDocument();
     });
 
     it('should submit when "Save Changes" is clicked with valid data', async function () {

static/app/views/alerts/rules/issue/ticketRuleModal.spec.tsx

@@ -36,12 +36,6 @@ describe('ProjectAlerts -> TicketRuleModal', function () {
     expect(closeModal).toHaveBeenCalled();
   };
 
-  const submitErrors = async errorCount => {
-    await doSubmit();
-    expect(screen.getAllByText('Field is required')).toHaveLength(errorCount);
-    expect(closeModal).toHaveBeenCalledTimes(0);
-  };
-
   const addMockConfigsAPICall = (otherField = {}) => {
     return MockApiClient.addMockResponse({
       url: '/organizations/org-slug/integrations/1/?ignored=Sprint',
@@ -126,10 +120,14 @@ describe('ProjectAlerts -> TicketRuleModal', function () {
       await submitSuccess();
     });
 
-    it('should raise validation errors when "Apply Changes" is clicked with invalid data', async function () {
+    it('submit button shall be disabled if form is incomplete', async function () {
       // This doesn't test anything TicketRules specific but I'm leaving it here as an example.
       renderComponent();
-      await submitErrors(1);
+      expect(screen.getByRole('button', {name: 'Apply Changes'})).toBeDisabled();
+      await userEvent.hover(screen.getByRole('button', {name: 'Apply Changes'}));
+      expect(
+        await screen.findByText('Required fields must be filled out')
+      ).toBeInTheDocument();
     });
 
     it('should reload fields when an "updatesForm" field changes', async function () {

static/app/views/issueList/savedIssueSearches.spec.tsx

@@ -43,6 +43,7 @@ describe('SavedIssueSearches', function () {
     name: 'Last 4 Hours',
     query: 'age:-4h',
     visibility: SavedSearchVisibility.ORGANIZATION,
+    sort: 'date',
   });
 
   const pinnedSearch = SearchFixture({