fix(ui): Preserve labels on Sentry App alert rule actions select fields (#37209)

Currently, we only save the value of select fields when users set up their alert rules. With this PR, we will also be saving the label so that if they want to verify the settings later on, we have the associated label to display, and not just the value.

static/app/components/forms/selectAsyncField.tsx

@@ -10,7 +10,13 @@ import {GeneralSelectValue} from 'sentry/components/forms/selectControl';
 
 export interface SelectAsyncFieldProps
   extends Omit<InputFieldProps, 'highlighted' | 'visible' | 'required' | 'value'>,
-    SelectAsyncControlProps {}
+    SelectAsyncControlProps {
+  /**
+   * Similar to onChange, except it provides the entire option object (including label) when a
+   * change is made to the field. Occurs after onChange.
+   */
+  onChangeOption?: (option: GeneralSelectValue, event: any) => void;
+}
 
 type SelectAsyncFieldState = {
   results: Result[];
@@ -29,6 +35,7 @@ class SelectAsyncField extends Component<SelectAsyncFieldProps, SelectAsyncField
   handleChange = (
     onBlur: SelectAsyncFieldProps['onBlur'],
     onChange: SelectAsyncFieldProps['onChange'],
+    onChangeOption: SelectAsyncFieldProps['onChangeOption'],
     optionObj: GeneralSelectValue,
     event: React.MouseEvent
   ) => {
@@ -43,6 +50,7 @@ class SelectAsyncField extends Component<SelectAsyncFieldProps, SelectAsyncField
     }
     this.setState({latestSelection: optionObj});
     onChange?.(value, event);
+    onChangeOption?.(optionObj, event);
     onBlur?.(value, event);
   };
 
@@ -66,24 +74,19 @@ class SelectAsyncField extends Component<SelectAsyncFieldProps, SelectAsyncField
   }
 
   render() {
-    const {...otherProps} = this.props;
+    const {onChangeOption, ...otherProps} = this.props;
     return (
       <InputField
         {...otherProps}
-        field={({onChange, onBlur, required: _required, onResults, value, ...props}) => (
+        field={({onBlur, onChange, required: _required, onResults, value, ...props}) => (
           <SelectAsyncControl
             {...props}
-            onChange={this.handleChange.bind(this, onBlur, onChange)}
+            onChange={this.handleChange.bind(this, onBlur, onChange, onChangeOption)}
             onResults={data => {
               const results = onResults(data);
               const resultSelection = results.find(result => result.value === value);
               this.setState(
-                resultSelection
-                  ? {
-                      results,
-                      latestSelection: resultSelection,
-                    }
-                  : {results}
+                resultSelection ? {results, latestSelection: resultSelection} : {results}
               );
               return results;
             }}

static/app/views/organizationIntegrations/sentryAppExternalForm.tsx

@@ -7,6 +7,7 @@ import {Client} from 'sentry/api';
 import FieldFromConfig from 'sentry/components/forms/fieldFromConfig';
 import Form from 'sentry/components/forms/form';
 import FormModel from 'sentry/components/forms/model';
+import {GeneralSelectValue} from 'sentry/components/forms/selectControl';
 import {Field, FieldValue} from 'sentry/components/forms/type';
 import {t} from 'sentry/locale';
 import {replaceAtArrayIndex} from 'sentry/utils/replaceAtArrayIndex';
@@ -32,9 +33,16 @@ export type SchemaFormConfig = {
   required_fields?: FieldFromSchema[];
 };
 
+type SentryAppSetting = {
+  name: string;
+  value: any;
+  label?: string;
+};
+
 // only need required_fields and optional_fields
 type State = Omit<SchemaFormConfig, 'uri' | 'description'> & {
   optionsByField: Map<string, Array<{label: string; value: any}>>;
+  selectedOptions: {[name: string]: GeneralSelectValue};
 };
 
 type Props = {
@@ -60,7 +68,10 @@ type Props = {
   /**
    * Object containing reset values for fields if previously entered, in case this form is unmounted
    */
-  resetValues?: {[key: string]: any; settings?: {name: string; value: any}[]};
+  resetValues?: {
+    [key: string]: any;
+    settings?: SentryAppSetting[];
+  };
 };
 
 /**
@@ -72,7 +83,7 @@ type Props = {
  *  See (#28465) for more details.
  */
 export class SentryAppExternalForm extends Component<Props, State> {
-  state: State = {optionsByField: new Map()};
+  state: State = {optionsByField: new Map(), selectedOptions: {}};
 
   componentDidMount() {
     this.resetStateFromProps();
@@ -135,6 +146,28 @@ export class SentryAppExternalForm extends Component<Props, State> {
     }
   };
 
+  getDefaultOptions = (field: FieldFromSchema) => {
+    const savedOption = ((this.props.resetValues || {}).settings || []).find(
+      value => value.name === field.name
+    );
+    const currentOptions = (field.choices || []).map(([value, label]) => ({
+      value,
+      label,
+    }));
+
+    const shouldAddSavedOption =
+      // We only render saved options if they have preserved the label, otherwise it appears unselcted.
+      // The next time the user saves, the label should be preserved.
+      savedOption?.value &&
+      savedOption?.label &&
+      // The option isn't in the current options already
+      !currentOptions.some(option => option.value === savedOption?.value);
+
+    return shouldAddSavedOption
+      ? [{value: savedOption.value, label: savedOption.label}, ...currentOptions]
+      : currentOptions;
+  };
+
   getDefaultFieldValue = (field: FieldFromSchema) => {
     // Interpret the default if a getFieldDefault function is provided.
     const {resetValues, getFieldDefault} = this.props;
@@ -191,9 +224,7 @@ export class SentryAppExternalForm extends Component<Props, State> {
 
     const {choices} = await this.props.api.requestPromise(
       `/sentry-app-installations/${sentryAppInstallationUuid}/external-requests/`,
-      {
-        query,
-      }
+      {query}
     );
     return choices || [];
   };
@@ -264,6 +295,15 @@ export class SentryAppExternalForm extends Component<Props, State> {
     });
   };
 
+  createPreserveOptionFunction = (name: string) => (option, _event) => {
+    this.setState({
+      selectedOptions: {
+        ...this.state.selectedOptions,
+        [name]: option,
+      },
+    });
+  };
+
   renderField = (field: FieldFromSchema, required: boolean) => {
     // This function converts the field we get from the backend into
     // the field we need to pass down
@@ -279,23 +319,17 @@ export class SentryAppExternalForm extends Component<Props, State> {
     }
     if (['select', 'select_async'].includes(fieldToPass.type || '')) {
       // find the options from state to pass down
-      const defaultOptions = (field.choices || []).map(([value, label]) => ({
-        value,
-        label,
-      }));
+      const defaultOptions = this.getDefaultOptions(field);
       const options = this.state.optionsByField.get(field.name) || defaultOptions;
-      const allowClear = !required;
-      const defaultValue = this.getDefaultFieldValue(field);
-      // filter by what the user is typing
-      const filterOption = createFilter({});
 
       fieldToPass = {
         ...fieldToPass,
         options,
-        defaultValue,
         defaultOptions,
-        filterOption,
-        allowClear,
+        defaultValue: this.getDefaultFieldValue(field),
+        // filter by what the user is typing
+        filterOption: createFilter({}),
+        allowClear: !required,
         placeholder: 'Type to search',
       } as Field;
       if (field.depends_on) {
@@ -325,6 +359,7 @@ export class SentryAppExternalForm extends Component<Props, State> {
           onCloseResetsInput: false,
           onBlurResetsInput: false,
           autoload: false,
+          onChangeOption: this.createPreserveOptionFunction(field.name),
         }
       : {};
 
@@ -343,7 +378,16 @@ export class SentryAppExternalForm extends Component<Props, State> {
     if (this.model.validateForm()) {
       onSubmitSuccess({
         // The form data must be nested in 'settings' to ensure they don't overlap with any other field names.
-        settings: Object.entries(formData).map(([name, value]) => ({name, value})),
+        settings: Object.entries(formData).map(([name, value]) => {
+          const savedSetting: SentryAppSetting = {name, value};
+          const stateOption = this.state.selectedOptions[name];
+          // If the field is a SelectAsync, we need to preserve the label since the next time it's rendered,
+          // we can't be sure the options will contain this selection
+          if (stateOption?.value === value) {
+            savedSetting.label = `${stateOption?.label}`;
+          }
+          return savedSetting;
+        }),
         sentryAppInstallationUuid,
         // Used on the backend to explicitly associate with a different rule than those without a custom form.
         hasSchemaFormConfig: true,

tests/js/spec/views/alerts/issueRules/sentryAppRuleModal.spec.jsx

@@ -1,4 +1,4 @@
-import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
+import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
 
 import SentryAppRuleModal from 'sentry/views/alerts/rules/issue/sentryAppRuleModal';
 
@@ -16,11 +16,6 @@ describe('SentryAppRuleModal', function () {
     sentryAppInstallation = TestStubs.SentryAppInstallation({sentryApp});
   });
 
-  function openSelectMenu(text) {
-    const placeholder = screen.getByText(text);
-    userEvent.type(placeholder, '{keyDown}');
-  }
-
   const _submit = () => {
     userEvent.click(screen.getByText('Save Changes'));
     return screen.queryAllByText('Field is required');
@@ -66,6 +61,33 @@ describe('SentryAppRuleModal', function () {
         label: 'Extra Details',
         name: 'extra',
       },
+      {
+        type: 'select',
+        label: 'Assignee',
+        name: 'assignee',
+        uri: '/link/assignee/',
+        skip_load_on_open: 'true',
+      },
+      {
+        type: 'select',
+        label: 'Workspace',
+        name: 'workspace',
+        uri: '/link/workspace/',
+      },
+    ],
+  };
+
+  const resetValues = {
+    settings: [
+      {
+        name: 'extra',
+        value: 'saved details from last edit',
+      },
+      {
+        name: 'assignee',
+        value: 'edna-mode',
+        label: 'Edna Mode',
+      },
     ],
   };
 
@@ -78,6 +100,7 @@ describe('SentryAppRuleModal', function () {
         config={defaultConfig}
         action="create"
         onSubmitSuccess={() => {}}
+        resetValues={resetValues}
         {...props}
       />
     );
@@ -99,14 +122,44 @@ describe('SentryAppRuleModal', function () {
       submitErrors(3);
     });
 
-    it('should submit when "Save Changes" is clicked with valid data', function () {
+    it('should submit when "Save Changes" is clicked with valid data', async function () {
       createWrapper();
+
       const titleInput = screen.getByTestId('title');
+      userEvent.type(titleInput, 'some title');
+
       const descriptionInput = screen.getByTestId('description');
-      userEvent.type(titleInput, 'v');
-      userEvent.type(descriptionInput, 'v');
-      openSelectMenu('Type to search');
+      userEvent.type(descriptionInput, 'some description');
+
+      const channelInput = screen.getAllByText('Type to search')[0];
+      userEvent.type(channelInput, '{keyDown}');
       userEvent.click(screen.getByText('valor'));
+
+      // Ensure text fields are persisted on edit
+      const savedExtraDetailsInput = screen.getByDisplayValue(
+        resetValues.settings[0].value
+      );
+      expect(savedExtraDetailsInput).toBeInTheDocument();
+      // Ensure select fields are persisted with labels on edit
+      const savedAssigneeInput = screen.getByText(resetValues.settings[1].label);
+      expect(savedAssigneeInput).toBeInTheDocument();
+
+      // Ensure async select fields filter correctly
+      const workspaceChoices = [
+        ['WS0', 'Primary Workspace'],
+        ['WS1', 'Secondary Workspace'],
+      ];
+      const workspaceResponse = MockApiClient.addMockResponse({
+        url: `/sentry-app-installations/${sentryAppInstallation.uuid}/external-requests/`,
+        body: {choices: workspaceChoices},
+      });
+      const workspaceInput = screen.getByText('Type to search');
+      // Search by value
+      userEvent.type(workspaceInput, workspaceChoices[1][0]);
+      await waitFor(() => expect(workspaceResponse).toHaveBeenCalled());
+      // Select by label
+      userEvent.click(screen.getByText(workspaceChoices[1][1]));
+
       submitSuccess();
     });
   });