From 4c38b11df1d7f4baf0568193ea8cf2434c0a01ba Mon Sep 17 00:00:00 2001 From: TkDodo Date: Wed, 1 Apr 2026 14:03:24 +0200 Subject: [PATCH 01/16] ref: first attempt to re-write externalIssuesForm --- .../backendJsonFormAdapter/index.tsx | 68 +--- .../backendJsonFormAdapter/submitForm.tsx | 360 ++++++++++++++++++ .../backendJsonFormAdapter/types.ts | 30 +- .../backendJsonFormAdapter/utils.ts | 76 ++++ .../externalIssues/externalForm.tsx | 80 ---- .../externalIssues/externalIssueForm.spec.tsx | 196 +++++++--- .../externalIssues/externalIssueForm.tsx | 188 ++++----- .../externalIssues/useAsyncOptionsCache.tsx | 32 -- .../app/components/externalIssues/utils.tsx | 226 ----------- 9 files changed, 691 insertions(+), 565 deletions(-) create mode 100644 static/app/components/backendJsonFormAdapter/submitForm.tsx create mode 100644 static/app/components/backendJsonFormAdapter/utils.ts delete mode 100644 static/app/components/externalIssues/externalForm.tsx delete mode 100644 static/app/components/externalIssues/useAsyncOptionsCache.tsx diff --git a/static/app/components/backendJsonFormAdapter/index.tsx b/static/app/components/backendJsonFormAdapter/index.tsx index b65e55bceb2be6..b625d3ea971c27 100644 --- a/static/app/components/backendJsonFormAdapter/index.tsx +++ b/static/app/components/backendJsonFormAdapter/index.tsx @@ -11,71 +11,7 @@ import {ChoiceMapperDropdown, ChoiceMapperTable} from './choiceMapperAdapter'; import {ProjectMapperAddRow, ProjectMapperTable} from './projectMapperAdapter'; import {TableBody, TableHeaderRow} from './tableAdapter'; import type {FieldValue, JsonFormAdapterFieldConfig} from './types'; - -function getZodType(fieldType: JsonFormAdapterFieldConfig['type']) { - switch (fieldType) { - case 'boolean': - return z.boolean(); - case 'string': - case 'text': - case 'secret': - case 'textarea': - return z.string(); - case 'number': - return z.number(); - case 'email': - return z.email(); - case 'url': - return z.url(); - case 'choice_mapper': - return z.object({}); - case 'project_mapper': - case 'table': - return z.array(z.any()); - case 'select': - case 'choice': - return z.any(); - default: - unreachable(fieldType); - return z.any(); - } -} - -function transformChoices( - choices?: Array<[value: string, label: string]> -): Array<{label: string; value: string}> { - if (!choices) { - return []; - } - return choices.map(([value, label]) => ({value, label})); -} - -function getDefaultForType(fieldType: JsonFormAdapterFieldConfig['type']): unknown { - switch (fieldType) { - case 'boolean': - return false; - case 'string': - case 'text': - case 'url': - case 'email': - case 'secret': - case 'textarea': - return ''; - case 'number': - return 0; - case 'choice_mapper': - return {}; - case 'project_mapper': - case 'table': - return []; - case 'select': - case 'choice': - return null; - default: - unreachable(fieldType); - return ''; - } -} +import {getDefaultForType, getZodType, transformChoices} from './utils'; interface BackendJsonFormAdapterProps< TField extends JsonFormAdapterFieldConfig, @@ -304,6 +240,8 @@ export function BackendJsonFormAdapter< /> ); + case 'blank': + return null; default: unreachable(field); return null; diff --git a/static/app/components/backendJsonFormAdapter/submitForm.tsx b/static/app/components/backendJsonFormAdapter/submitForm.tsx new file mode 100644 index 00000000000000..00629eb80f06ac --- /dev/null +++ b/static/app/components/backendJsonFormAdapter/submitForm.tsx @@ -0,0 +1,360 @@ +import {useEffect, useMemo, useRef} from 'react'; + +import {defaultFormOptions, useScrapsForm} from '@sentry/scraps/form'; +import {Stack} from '@sentry/scraps/layout'; + +import {Client} from 'sentry/api'; +import {LoadingIndicator} from 'sentry/components/loadingIndicator'; +import type {SelectValue} from 'sentry/types/core'; + +import type {JsonFormAdapterFieldConfig} from './types'; +import {getDefaultForType, transformChoices} from './utils'; + +/** + * API client without base URL prefix, needed for async select fields + * that use URLs like `/extensions/jira/search/...` or `/search`. + */ +const API_CLIENT = new Client({baseUrl: '', headers: {}}); + +interface BackendJsonSubmitFormProps { + /** + * Field configs from the backend API response. + */ + fields: JsonFormAdapterFieldConfig[]; + /** + * Called when the form is submitted. Should return a promise that + * resolves on success or rejects/throws on error. + */ + onSubmit: (values: Record) => Promise; + /** + * Current values of dynamic fields, passed as query params to async select endpoints. + */ + dynamicFieldValues?: Record; + /** + * Render prop for the submit button area. Receives the disabled state and the + * SubmitButton component. Use this to place the button in a custom location + * (e.g., a modal footer). If not provided, the submit button renders inline. + */ + footer?: (props: {SubmitButton: any; disabled: boolean}) => React.ReactNode; + /** + * Whether the form is in a loading state (e.g., dynamic field refetch in progress). + */ + isLoading?: boolean; + /** + * Called when a field with `updatesForm: true` changes value. + */ + onFieldChange?: (fieldName: string, value: unknown) => void; + /** + * Whether the submit button should be disabled (e.g., form has errors). + */ + submitDisabled?: boolean; + /** + * Label for the submit button. + */ + submitLabel?: string; +} + +function computeDefaultValues( + fields: JsonFormAdapterFieldConfig[] +): Record { + const defaults: Record = {}; + for (const field of fields) { + if (field.name && field.type !== 'blank') { + defaults[field.name] = field.default ?? getDefaultForType(field.type); + } + } + return defaults; +} + +function buildAsyncSelectQuery( + fieldName: string, + query: string, + dynamicFieldValues?: Record +): Record { + return { + ...dynamicFieldValues, + field: fieldName, + query, + }; +} + +/** + * A multi-field form that renders backend-driven field configs with a submit button. + * Unlike `BackendJsonFormAdapter` (which is per-field auto-save), this component + * renders all fields in a single form and submits them together. + * + * Supports: + * - Static select fields (from `field.choices`) + * - Async select fields (from `field.url` with debounced search) + * - Dynamic field refetching (via `onFieldChange` for `updatesForm` fields) + * - Text, textarea, number, boolean, and other basic field types + */ +export function BackendJsonSubmitForm({ + fields, + onSubmit, + submitLabel, + submitDisabled, + isLoading, + dynamicFieldValues, + onFieldChange, + footer, +}: BackendJsonSubmitFormProps) { + const defaultValues = useMemo(() => computeDefaultValues(fields), [fields]); + const prevFieldsRef = useRef(fields); + + const form = useScrapsForm({ + ...defaultFormOptions, + defaultValues, + onSubmit: async ({value}) => { + await onSubmit(value); + }, + }); + + // When fields change (e.g., dynamic refetch), set values for new fields + // while preserving user-entered values for existing fields + useEffect(() => { + const prevNames = new Set( + prevFieldsRef.current.filter(f => f.type !== 'blank').map(f => f.name) + ); + const currentNames = new Set(fields.filter(f => f.type !== 'blank').map(f => f.name)); + + // Only act if the set of field names has changed + if ( + prevNames.size !== currentNames.size || + [...prevNames].some(n => !currentNames.has(n)) + ) { + for (const field of fields) { + if (field.name && field.type !== 'blank' && !prevNames.has(field.name)) { + // New field — set its default value + form.setFieldValue( + field.name as never, + (field.default ?? getDefaultForType(field.type)) as never + ); + } + } + } + prevFieldsRef.current = fields; + }, [fields, form]); + + const hasErrors = fields.some( + field => field.name === 'error' && field.type === 'blank' + ); + + const requiredFields = useMemo( + () => fields.filter(f => f.required && f.type !== 'blank').map(f => f.name), + [fields] + ); + + const renderSubmitButton = (disabled: boolean) => { + if (footer) { + return footer({ + SubmitButton: form.SubmitButton, + disabled, + }); + } + return {submitLabel}; + }; + + return ( + + {isLoading && } + {!isLoading && ( + + {fields + .filter(field => field.hasOwnProperty('name') && field.type !== 'blank') + .map(field => ( + + ))} + + )} + state.values}> + {(values: any) => { + const hasUnfilledRequired = requiredFields.some(name => { + const val = values[name]; + return val === null || val === undefined || val === ''; + }); + return renderSubmitButton( + hasErrors || !!submitDisabled || !!isLoading || hasUnfilledRequired + ); + }} + + + ); +} + +/** + * Renders a single field within the submit form. + */ +function SubmitFormField({ + form, + field, + dynamicFieldValues, + onFieldChange, +}: { + field: JsonFormAdapterFieldConfig; + form: any; + dynamicFieldValues?: Record; + onFieldChange?: (fieldName: string, value: unknown) => void; +}) { + return ( + + {(fieldApi: any) => { + const handleChange = (value: unknown) => { + fieldApi.handleChange(value as never); + if (field.updatesForm && onFieldChange) { + onFieldChange(field.name, value); + } + }; + + switch (field.type) { + case 'boolean': + return ( + + + + ); + case 'textarea': + return ( + + + + ); + case 'number': + return ( + + + + ); + case 'select': + case 'choice': + if (field.url) { + // Async select: fetch options from URL as user types. + // Show static choices as initial options before any search. + const staticOptions = transformChoices(field.choices); + return ( + + ({ + queryKey: [ + 'backend-json-async-select', + field.name, + field.url, + debouncedInput, + dynamicFieldValues, + ], + queryFn: async (): Promise>> => { + if (!debouncedInput) { + return staticOptions; + } + return API_CLIENT.requestPromise(field.url!, { + query: buildAsyncSelectQuery( + field.name, + debouncedInput, + dynamicFieldValues + ), + }); + }, + // Always enabled so static choices show immediately + enabled: true, + })} + /> + + ); + } + return ( + + + + ); + case 'secret': + return ( + + + + ); + case 'string': + case 'text': + case 'url': + case 'email': + return ( + + + + ); + default: + return null; + } + }} + + ); +} diff --git a/static/app/components/backendJsonFormAdapter/types.ts b/static/app/components/backendJsonFormAdapter/types.ts index 82519926681146..d5fc670be61e82 100644 --- a/static/app/components/backendJsonFormAdapter/types.ts +++ b/static/app/components/backendJsonFormAdapter/types.ts @@ -14,6 +14,12 @@ interface JsonFormAdapterBase { help?: string; placeholder?: string; required?: boolean; + /** + * When true, changing this field triggers a re-fetch of the form config + * from the backend with the new value as a query parameter. + * Used by BackendJsonSubmitForm to support dynamic form fields. + */ + updatesForm?: boolean; } interface JsonFormAdapterBoolean extends JsonFormAdapterBase { @@ -33,6 +39,15 @@ interface JsonFormAdapterSecret extends JsonFormAdapterBase { interface JsonFormAdapterSelect extends JsonFormAdapterBase { type: 'select' | 'choice'; choices?: Array<[value: string, label: string]>; + /** + * When true, allows selecting multiple values. + */ + multiple?: boolean; + /** + * URL for async select fields. When set, options are fetched from this + * endpoint as the user types instead of using static `choices`. + */ + url?: string; } interface JsonFormAdapterNumber extends JsonFormAdapterBase { @@ -95,6 +110,14 @@ interface JsonFormAdapterProjectMapper extends JsonFormAdapterBase { }>; } +/** + * A blank field is used to signal errors in the form config. + * It renders nothing but can be detected to disable form submission. + */ +interface JsonFormAdapterBlank extends JsonFormAdapterBase { + type: 'blank'; +} + export type JsonFormAdapterFieldConfig = | JsonFormAdapterBoolean | JsonFormAdapterString @@ -103,7 +126,8 @@ export type JsonFormAdapterFieldConfig = | JsonFormAdapterNumber | JsonFormAdapterChoiceMapper | JsonFormAdapterTable - | JsonFormAdapterProjectMapper; + | JsonFormAdapterProjectMapper + | JsonFormAdapterBlank; /** * Maps a field config type to the shape of its value. @@ -123,4 +147,6 @@ export type FieldValue = ? Array> : T extends JsonFormAdapterProjectMapper ? Array<[number, string]> - : unknown; + : T extends JsonFormAdapterBlank + ? never + : unknown; diff --git a/static/app/components/backendJsonFormAdapter/utils.ts b/static/app/components/backendJsonFormAdapter/utils.ts new file mode 100644 index 00000000000000..171a9ed119a614 --- /dev/null +++ b/static/app/components/backendJsonFormAdapter/utils.ts @@ -0,0 +1,76 @@ +import {z} from 'zod'; + +import {unreachable} from 'sentry/utils/unreachable'; + +import type {JsonFormAdapterFieldConfig} from './types'; + +export function getZodType(fieldType: JsonFormAdapterFieldConfig['type']) { + switch (fieldType) { + case 'boolean': + return z.boolean(); + case 'string': + case 'text': + case 'secret': + case 'textarea': + return z.string(); + case 'number': + return z.number(); + case 'email': + return z.email(); + case 'url': + return z.url(); + case 'choice_mapper': + return z.object({}); + case 'project_mapper': + case 'table': + return z.array(z.any()); + case 'select': + case 'choice': + return z.any(); + case 'blank': + return z.any(); + default: + unreachable(fieldType); + return z.any(); + } +} + +export function transformChoices( + choices?: Array<[value: string, label: string]> +): Array<{label: string; value: string}> { + if (!choices) { + return []; + } + return choices.map(([value, label]) => ({value, label})); +} + +export function getDefaultForType( + fieldType: JsonFormAdapterFieldConfig['type'] +): unknown { + switch (fieldType) { + case 'boolean': + return false; + case 'string': + case 'text': + case 'url': + case 'email': + case 'secret': + case 'textarea': + return ''; + case 'number': + return 0; + case 'choice_mapper': + return {}; + case 'project_mapper': + case 'table': + return []; + case 'select': + case 'choice': + return null; + case 'blank': + return null; + default: + unreachable(fieldType); + return ''; + } +} diff --git a/static/app/components/externalIssues/externalForm.tsx b/static/app/components/externalIssues/externalForm.tsx deleted file mode 100644 index f61c899512e852..00000000000000 --- a/static/app/components/externalIssues/externalForm.tsx +++ /dev/null @@ -1,80 +0,0 @@ -import {Fragment} from 'react'; - -import type {ModalRenderProps} from 'sentry/actionCreators/modal'; -import type {ExternalIssueFormErrors} from 'sentry/components/externalIssues/utils'; -import {FieldFromConfig} from 'sentry/components/forms/fieldFromConfig'; -import type {FormProps} from 'sentry/components/forms/form'; -import {Form} from 'sentry/components/forms/form'; -import {LoadingIndicator} from 'sentry/components/loadingIndicator'; -import {t} from 'sentry/locale'; -import type {IssueConfigField} from 'sentry/types/integrations'; -import type {FormField} from 'sentry/views/alerts/rules/issue/ruleNode'; - -interface ExternalFormProps { - Body: ModalRenderProps['Body']; - Header: ModalRenderProps['Header']; - bodyText: React.ReactNode; - formProps: FormProps; - // XXX: Ideally this would be Partial but different field types - // have different props. For this conversion, I'm assuming the props are correct. - getFieldProps: (field: IssueConfigField) => Record; - isLoading: boolean; - navTabs: React.ReactNode; - title: React.ReactNode; - errors?: ExternalIssueFormErrors; - formFields?: IssueConfigField[]; -} - -export function ExternalForm({ - Header, - Body, - isLoading, - formProps, - title, - navTabs, - bodyText, - formFields = [], - errors = {}, - getFieldProps, -}: ExternalFormProps) { - return ( - -
-

{title}

-
- {navTabs} - - {isLoading ? ( - - ) : ( - - {bodyText} -
- {(formFields || []) - .filter((field: FormField) => field.hasOwnProperty('name')) - .map(fields => ({ - ...fields, - noOptionsMessage: () => t('No options. Type to search.'), - })) - .map((field, i) => { - return ( - - - {errors[field.name] && errors[field.name]} - - ); - })} -
-
- )} - -
- ); -} diff --git a/static/app/components/externalIssues/externalIssueForm.spec.tsx b/static/app/components/externalIssues/externalIssueForm.spec.tsx index ff66f46c6e278a..79b646a81dcf7f 100644 --- a/static/app/components/externalIssues/externalIssueForm.spec.tsx +++ b/static/app/components/externalIssues/externalIssueForm.spec.tsx @@ -2,7 +2,14 @@ import {GitHubIntegrationFixture} from 'sentry-fixture/githubIntegration'; import {GroupFixture} from 'sentry-fixture/group'; import {OrganizationFixture} from 'sentry-fixture/organization'; -import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; +import { + render, + screen, + userEvent, + waitFor, + waitForElementToBeRemoved, +} from 'sentry-test/reactTestingLibrary'; +import {selectEvent} from 'sentry-test/selectEvent'; import {ExternalIssueForm} from 'sentry/components/externalIssues/externalIssueForm'; import { @@ -12,25 +19,6 @@ import { ModalFooter, } from 'sentry/components/globalModal/components'; -jest.mock('lodash/debounce', () => { - const debounceMap = new Map(); - const mockDebounce = - (fn: (...args: any[]) => void, timeout: number) => - (...args: any[]) => { - if (debounceMap.has(fn)) { - clearTimeout(debounceMap.get(fn)); - } - debounceMap.set( - fn, - setTimeout(() => { - fn.apply(fn, args); - debounceMap.delete(fn); - }, timeout) - ); - }; - return mockDebounce; -}); - describe('ExternalIssueForm', () => { const group = GroupFixture(); const integration = GitHubIntegrationFixture({externalIssues: []}); @@ -70,13 +58,12 @@ describe('ExternalIssueForm', () => { />, {organization} ); - expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument(); + await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); await userEvent.click(screen.getByText(action)); return wrapper; }; describe('create', () => { - // TODO: expand tests beforeEach(() => { formConfig = { createIssueConfig: [], @@ -89,6 +76,109 @@ describe('ExternalIssueForm', () => { it('renders', async () => { await renderComponent(); }); + + it('should submit the form and close the modal on success', async () => { + formConfig = { + createIssueConfig: [ + { + label: 'Title', + required: true, + type: 'string', + name: 'title', + default: 'Default Title', + }, + { + label: 'Description', + required: false, + type: 'textarea', + name: 'description', + default: '', + }, + ], + }; + MockApiClient.addMockResponse({ + url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/`, + body: formConfig, + }); + + const submitRequest = MockApiClient.addMockResponse({ + url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/`, + method: 'POST', + body: {id: '123', key: 'TEST-1'}, + }); + + await renderComponent(); + + await userEvent.click(screen.getByRole('button', {name: 'Create Issue'})); + await waitFor(() => expect(submitRequest).toHaveBeenCalled()); + expect(onChange).toHaveBeenCalled(); + expect(closeModal).toHaveBeenCalled(); + }); + + it('should switch to link action and load link config', async () => { + formConfig = { + createIssueConfig: [ + { + label: 'Title', + required: true, + type: 'string', + name: 'title', + default: 'Default Title', + }, + ], + }; + MockApiClient.addMockResponse({ + url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/`, + body: formConfig, + match: [MockApiClient.matchQuery({action: 'create'})], + }); + + const linkConfig = MockApiClient.addMockResponse({ + url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/`, + body: { + linkIssueConfig: [ + { + label: 'Issue', + required: true, + type: 'select', + name: 'externalIssue', + url: '/search', + choices: [], + }, + ], + }, + match: [MockApiClient.matchQuery({action: 'link'})], + }); + + render( + , + {organization} + ); + + // Wait for initial load + expect(await screen.findByRole('textbox', {name: 'Title'})).toBeInTheDocument(); + + // Click the Link tab + await userEvent.click(screen.getByText('Link')); + expect(linkConfig).toHaveBeenCalled(); + + // Should show link config fields, not create config fields + expect(await screen.findByRole('textbox', {name: 'Issue'})).toBeInTheDocument(); + expect(screen.queryByRole('textbox', {name: 'Title'})).not.toBeInTheDocument(); + + // Link action should show "Link Issue" button + expect(screen.getByRole('button', {name: 'Link Issue'})).toBeInTheDocument(); + }); + it('if we have an error fields, we should disable the create button', async () => { formConfig = { createIssueConfig: [ @@ -196,41 +286,37 @@ describe('ExternalIssueForm', () => { />, {organization} ); - expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument(); + await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); expect(initialQuery).toHaveBeenCalled(); // Initial query may only have a few fields - const projectSelect = screen.getByRole('textbox', {name: 'Project'}); expect(screen.queryByRole('textbox', {name: 'Summary'})).not.toBeInTheDocument(); expect(screen.queryByRole('textbox', {name: 'Reporter'})).not.toBeInTheDocument(); // If the field has `updatesForm`, refetch the config. // If new fields are in the response, they should be visible. - await userEvent.click(projectSelect); - await userEvent.click(screen.getByText('Project 1')); + await selectEvent.select( + screen.getByRole('textbox', {name: 'Project'}), + 'Project 1' + ); expect(projectOneQuery).toHaveBeenCalled(); - // Project 1 should be selected, Project 2 should not - expect(screen.getByText('Project 1')).toBeInTheDocument(); - expect(screen.queryByText('Project 2')).not.toBeInTheDocument(); expect(screen.getByRole('textbox', {name: 'Summary'})).toBeInTheDocument(); expect(screen.getByRole('textbox', {name: 'Reporter'})).toBeInTheDocument(); // We should also respect new required fields const submitButton = screen.getByRole('button', {name: 'Create Issue'}); expect(submitButton).toBeDisabled(); - await userEvent.click(screen.getByRole('textbox', {name: 'Reporter'})); - await userEvent.click(screen.getByText('User 1')); - expect(submitButton).toBeEnabled(); + await selectEvent.select(screen.getByRole('textbox', {name: 'Reporter'}), 'User 1'); + await waitFor(() => expect(submitButton).toBeEnabled()); // Swapping projects should refetch, and remove stale fields - await userEvent.click(projectSelect); - await userEvent.click(screen.getByText('Project 2')); + await selectEvent.select( + screen.getByRole('textbox', {name: 'Project'}), + 'Project 2' + ); expect(projectTwoQuery).toHaveBeenCalled(); - // Project 2 should be selected, Project 1 should not - expect(screen.getByText('Project 2')).toBeInTheDocument(); - expect(screen.queryByText('Project 1')).not.toBeInTheDocument(); expect(screen.queryByRole('textbox', {name: 'Summary'})).not.toBeInTheDocument(); expect(screen.queryByRole('textbox', {name: 'Reporter'})).not.toBeInTheDocument(); expect(submitButton).toBeEnabled(); @@ -309,25 +395,32 @@ describe('ExternalIssueForm', () => { expect(getFormConfigRequest).toHaveBeenCalled(); }); - it('shows placeholder for async select fields', async () => { + it('shows async select fields', async () => { await renderComponent('Link'); - // The Issue field has a url property (async select), so it should display our placeholder + // The Issue field has a url property (async select), so it should be rendered expect(screen.getByRole('textbox', {name: 'Issue'})).toBeInTheDocument(); - expect(screen.getByText('Type to search')).toBeInTheDocument(); }); describe('options loaded', () => { - beforeEach(() => { + it('fetches options when user types in async select', async () => { + // Catch-all mock for /search to prevent unmocked request errors MockApiClient.addMockResponse({ - url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/?action=link`, - body: formConfig, + url: '/search', + body: [], }); - }); - it('fast typing is debounced and uses trailing call when fetching data', async () => { + // The SelectAsyncField uses useDebouncedValue (250ms) internally + // Dynamic field values (repo default) are included in the search URL const searchResponse = MockApiClient.addMockResponse({ - url: '/search?field=externalIssue&query=faster&repo=scefali%2Ftest', + url: '/search', + match: [ + MockApiClient.matchQuery({ + repo: 'scefali/test', + field: 'externalIssue', + query: 'faster', + }), + ], body: [ { label: '#1337 ref(js): Convert Form to a FC', @@ -341,13 +434,12 @@ describe('ExternalIssueForm', () => { }); await renderComponent('Link'); - jest.useFakeTimers(); const textbox = screen.getByRole('textbox', {name: 'Issue'}); - await userEvent.click(textbox, {delay: null}); - await userEvent.type(textbox, 'faster', {delay: null}); - expect(searchResponse).not.toHaveBeenCalled(); - jest.advanceTimersByTime(300); - expect(searchResponse).toHaveBeenCalledTimes(1); + await userEvent.click(textbox); + await userEvent.type(textbox, 'faster'); + + // Wait for debounce and API call + await waitFor(() => expect(searchResponse).toHaveBeenCalled()); expect(await screen.findByText('#2345 perf: Make it faster')).toBeInTheDocument(); }); }); diff --git a/static/app/components/externalIssues/externalIssueForm.tsx b/static/app/components/externalIssues/externalIssueForm.tsx index 3739164a6d0ef8..0b1c4ae9728c9f 100644 --- a/static/app/components/externalIssues/externalIssueForm.tsx +++ b/static/app/components/externalIssues/externalIssueForm.tsx @@ -8,19 +8,11 @@ import {TabList, Tabs} from '@sentry/scraps/tabs'; import {addSuccessMessage} from 'sentry/actionCreators/indicator'; import {openModal, type ModalRenderProps} from 'sentry/actionCreators/modal'; import type {RequestOptions, ResponseMeta} from 'sentry/api'; -import {ExternalForm} from 'sentry/components/externalIssues/externalForm'; -import {useAsyncOptionsCache} from 'sentry/components/externalIssues/useAsyncOptionsCache'; +import {BackendJsonSubmitForm} from 'sentry/components/backendJsonFormAdapter/submitForm'; +import type {JsonFormAdapterFieldConfig} from 'sentry/components/backendJsonFormAdapter/types'; import {useDynamicFields} from 'sentry/components/externalIssues/useDynamicFields'; import type {ExternalIssueAction} from 'sentry/components/externalIssues/utils'; -import { - getConfigName, - getFieldProps, - getOptions, - hasErrorInFields, - loadAsyncThenFetchAllFields, -} from 'sentry/components/externalIssues/utils'; -import type {FieldValue} from 'sentry/components/forms/model'; -import {FormModel} from 'sentry/components/forms/model'; +import {getConfigName} from 'sentry/components/externalIssues/utils'; import {LoadingError} from 'sentry/components/loadingError'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {t, tct} from 'sentry/locale'; @@ -30,7 +22,6 @@ import type { Integration, IntegrationExternalIssue, IntegrationIssueConfig, - IssueConfigField, } from 'sentry/types/integrations'; import type {Organization} from 'sentry/types/organization'; import {trackAnalytics} from 'sentry/utils/analytics'; @@ -121,9 +112,9 @@ export function ExternalIssueForm({ closeModal, Header, Body, + Footer, }: ExternalIssueFormProps) { const api = useApi({persistInFlight: true}); - const [model] = useState(() => new FormModel()); const organization = useOrganization(); const {url: endpointString} = parseQueryKey( makeIntegrationIssueConfigQueryKey({ @@ -139,7 +130,6 @@ export function ExternalIssueForm({ const [loadSpan, setLoadSpan] = useState(null); const [submitSpan, setSubmitSpan] = useState(null); const [action, setAction] = useState('create'); - const {cache, updateCache} = useAsyncOptionsCache(); const [isDynamicallyRefetching, setIsDynamicallyRefetching] = useState(false); const { @@ -175,7 +165,7 @@ export function ExternalIssueForm({ * `useApiQuery`, and instead manually call the api, and update the cache ourselves. */ const refetchWithDynamicFields = useCallback( - (dynamicValues: Record) => { + (dynamicValues: Record) => { setIsDynamicallyRefetching(true); const requestOptions: RequestOptions = { method: 'GET', @@ -271,83 +261,66 @@ export function ExternalIssueForm({ [refetch] ); - const handlePreSubmit = useCallback(() => { - setSubmitSpan(startSpan('submit')); - }, [startSpan]); - - const handleSubmitError = useCallback(() => { - submitSpan?.end(); - }, [submitSpan]); - - const handleSubmitSuccess = useCallback( - (_data: IntegrationExternalIssue) => { - trackAnalytics('issue_details.external_issue_created', { - organization, - ...getAnalyticsDataForGroup(group), - external_issue_provider: integration.provider.key, - external_issue_type: 'first_party', - }); - onChange(() => addSuccessMessage(MESSAGES_BY_ACTION[action])); - closeModal(); - submitSpan?.end(); + const handleSubmit = useCallback( + async (values: Record) => { + setSubmitSpan(startSpan('submit')); + try { + const data: IntegrationExternalIssue = await api.requestPromise(endpointString, { + method: action === 'create' ? 'POST' : 'PUT', + data: values, + }); + trackAnalytics('issue_details.external_issue_created', { + organization, + ...getAnalyticsDataForGroup(group), + external_issue_provider: integration.provider.key, + external_issue_type: 'first_party', + }); + onChange(() => addSuccessMessage(MESSAGES_BY_ACTION[action])); + closeModal(); + submitSpan?.end(); + return data; + } catch (err) { + submitSpan?.end(); + throw err; + } }, - [organization, group, integration, submitSpan, action, closeModal, onChange] + [ + api, + endpointString, + action, + organization, + group, + integration, + submitSpan, + startSpan, + closeModal, + onChange, + ] ); const onFieldChange = useCallback( - (fieldName: string, value: FieldValue) => { + (fieldName: string, value: unknown) => { if (dynamicFieldValues.hasOwnProperty(fieldName)) { setDynamicFieldValue(fieldName, value); - refetchWithDynamicFields({...dynamicFieldValues, [fieldName]: value}); + refetchWithDynamicFields({ + ...dynamicFieldValues, + [fieldName]: value, + }); } }, [dynamicFieldValues, refetchWithDynamicFields, setDynamicFieldValue] ); - // Even if we pass onFieldChange as a prop, the model only uses the first instance. - // In order to use the correct dynamicFieldValues, we need to set it whenever this function is changed. - useEffect(() => { - model.setFormOptions({onFieldChange}); - }, [model, onFieldChange]); - - const getExternalIssueFieldProps = useCallback( - (field: IssueConfigField) => { - return getFieldProps({ - field, - loadOptions: (input: string) => - getOptions({ - field, - input, - dynamicFieldValues, - model, - successCallback: updateCache, - }), - }); - }, - [updateCache, dynamicFieldValues, model] - ); - - const formFields = useMemo(() => { + const formFields = useMemo((): JsonFormAdapterFieldConfig[] => { if (!integrationDetails) { return []; } - return loadAsyncThenFetchAllFields({ - configName: getConfigName(action), - integrationDetails, - fetchedFieldOptionsCache: cache, - }); - }, [integrationDetails, action, cache]); - - const initialData = formFields.reduce>( - (accumulator, field: IssueConfigField) => { - accumulator[field.name] = field.default; - return accumulator; - }, - {} - ); + const config = integrationDetails[getConfigName(action)]; + return (config ?? []) as JsonFormAdapterFieldConfig[]; + }, [integrationDetails, action]); const hasFormErrors = useMemo(() => { - return hasErrorInFields({fields: formFields}); + return formFields.some(field => field.name === 'error' && field.type === 'blank'); }, [formFields]); if (isPending) { @@ -382,38 +355,37 @@ export function ExternalIssueForm({ } return ( - - - - {t('Create')} - {t('Link')} - - - - } - bodyText={null} - getFieldProps={getExternalIssueFieldProps} - /> + +
+

{title}

+
+ + + + {t('Create')} + {t('Link')} + + + + + ( +
+ + {SUBMIT_LABEL_BY_ACTION[action]} + +
+ )} + /> + +
); } diff --git a/static/app/components/externalIssues/useAsyncOptionsCache.tsx b/static/app/components/externalIssues/useAsyncOptionsCache.tsx deleted file mode 100644 index 991e890c147b41..00000000000000 --- a/static/app/components/externalIssues/useAsyncOptionsCache.tsx +++ /dev/null @@ -1,32 +0,0 @@ -import {useCallback, useState} from 'react'; - -import type {Choices, SelectValue} from 'sentry/types/core'; -import type {IssueConfigField} from 'sentry/types/integrations'; - -/** - * Manages state cache of options fetched for async fields. - * This is used to avoid fetching the same options multiple times. - */ -export function useAsyncOptionsCache(initialCache?: Record) { - const [asyncOptions, setAsyncOptions] = useState>( - initialCache || {} - ); - - const updateCache = useCallback( - ({ - field, - result, - }: { - field: IssueConfigField; - result: Array>; - }) => { - setAsyncOptions({ - ...asyncOptions, - [field.name]: result.map(obj => [obj.value, obj.label]) as Choices, - }); - }, - [asyncOptions] - ); - - return {cache: asyncOptions, updateCache}; -} diff --git a/static/app/components/externalIssues/utils.tsx b/static/app/components/externalIssues/utils.tsx index f3e7bf9ed239a0..40338fac3e2098 100644 --- a/static/app/components/externalIssues/utils.tsx +++ b/static/app/components/externalIssues/utils.tsx @@ -1,129 +1,7 @@ -import {Fragment} from 'react'; -import debounce from 'lodash/debounce'; -import * as qs from 'query-string'; - -import {Client} from 'sentry/api'; -import type {FormModel} from 'sentry/components/forms/model'; import type {FieldValue} from 'sentry/components/forms/types'; -import {QuestionTooltip} from 'sentry/components/questionTooltip'; -import {t, tct} from 'sentry/locale'; -import type {Choices, SelectValue} from 'sentry/types/core'; import type {IntegrationIssueConfig, IssueConfigField} from 'sentry/types/integrations'; export type ExternalIssueAction = 'create' | 'link'; -export type ExternalIssueFormErrors = Record; - -// This exists because /extensions/type/search API is not prefixed with -// /api/0/, but the default API client on the abstract issue form is... -const API_CLIENT = new Client({baseUrl: '', headers: {}}); -const DEBOUNCE_MS = 200; - -async function getOptionLoad({ - field, - input, - callback, - dynamicFieldValues, -}: { - callback: (err: Error | null, result?: any) => void; - dynamicFieldValues: Record; - field: IssueConfigField; - input: string; -}) { - const query = qs.stringify({ - ...dynamicFieldValues, - field: field.name, - query: input, - }); - - const url = field.url || ''; - const separator = url.includes('?') ? '&' : '?'; - // We can't use the API client here since the URL is not scoped under the - // API endpoints (which the client prefixes) - - try { - const response = await API_CLIENT.requestPromise(url + separator + query); - callback(null, response); - } catch (err: any) { - callback(err); - } -} - -const debouncedOptionLoad = debounce( - ({ - field, - input, - callback, - dynamicFieldValues, - }: { - callback: (err: Error | null, result?: any) => void; - dynamicFieldValues: Record; - field: IssueConfigField; - input: string; - }) => getOptionLoad({field, input, callback, dynamicFieldValues}), - DEBOUNCE_MS, - {trailing: true} -); - -function getDefaultOptions({ - field, -}: { - field: IssueConfigField; -}): Array> { - const choices = - (field.choices as Array<[number | string, number | string | React.ReactElement]>) || - []; - return choices.map(([value, label]) => ({value, label})); -} - -/** - * Ensures current result from Async select fields is never discarded. Without this method, - * searching in an async select field without selecting one of the returned choices will - * result in a value saved to the form, and no associated label; appearing empty. - * @param field The field being examined - * @param result The result from its asynchronous query - * @param model The form model - * @returns The result with a tooltip attached to the current option - */ -function ensureCurrentOption({ - field, - result, - model, -}: { - field: IssueConfigField; - model: FormModel; - result: Array>; -}): Array> { - const currentOption = getDefaultOptions({field}).find( - option => option.value === model.getValue(field.name) - ); - if (!currentOption) { - return result; - } - if (typeof currentOption.label === 'string') { - currentOption.label = ( - - {' '} - {currentOption.label} - - ); - } - const currentOptionResultIndex = result.findIndex( - obj => obj.value === currentOption?.value - ); - // Has a selected option, and it is in API results - if (currentOptionResultIndex >= 0) { - const newResult = result; - newResult[currentOptionResultIndex] = currentOption; - return newResult; - } - // Has a selected option, and it is not in API results - return [...result, currentOption]; -} export function getConfigName( action: ExternalIssueAction @@ -138,51 +16,6 @@ export function getConfigName( } } -/** - * Get the list of options for a field via debounced API call. For example, - * the list of users that match the input string. The Promise rejects if there - * are any errors. - */ -export function getOptions({ - field, - input, - model, - successCallback, - dynamicFieldValues = {}, -}: { - field: IssueConfigField; - input: string; - model: FormModel; - dynamicFieldValues?: Record; - successCallback?: ({ - field, - result, - }: { - field: IssueConfigField; - result: Array>; - }) => void; -}) { - return new Promise>>((resolve, reject) => { - if (!input) { - return resolve(getDefaultOptions({field})); - } - return debouncedOptionLoad({ - field, - input, - callback: (err, result) => { - if (err) { - reject(err); - } else { - result = ensureCurrentOption({field, result, model}); - successCallback?.({field, result}); - resolve(result); - } - }, - dynamicFieldValues, - }); - }); -} - /** * Convert IntegrationIssueConfig to an object that maps field names to the * values of fields where `updatesForm` is true. @@ -202,62 +35,3 @@ export function getDynamicFields({ .map((field: IssueConfigField) => [field.name, field.default || null]) ); } - -/** - * If this field is an async select (field.url is not null), add async props. - * XXX: We pass through loadOptions since it's highly opinionated in the abstract class. - */ -export function getFieldProps({ - field, - loadOptions, -}: { - field: IssueConfigField; - loadOptions: (input: string) => Promise>>; -}) { - if (!field.url) { - return {}; - } - return { - async: true, - autoload: true, - cache: false, - loadOptions, - defaultOptions: getDefaultOptions({field}), - onBlurResetsInput: false, - onCloseResetsInput: false, - onSelectResetsInput: false, - placeholder: t('Type to search'), - }; -} - -/** - * Populate all async fields with their choices, then return the full list of fields. - * We pull from the fetchedFieldOptionsCache which contains the most recent choices - * for each async field. - */ -export function loadAsyncThenFetchAllFields({ - configName, - integrationDetails, - fetchedFieldOptionsCache, -}: { - configName: 'createIssueConfig' | 'linkIssueConfig'; - fetchedFieldOptionsCache: Record; - integrationDetails: IntegrationIssueConfig | null; -}): IssueConfigField[] { - const configsFromAPI = integrationDetails?.[configName]; - return (configsFromAPI || []).map(field => { - const fieldCopy = {...field}; - // Overwrite choices from cache. - if (fetchedFieldOptionsCache?.hasOwnProperty(field.name)) { - fieldCopy.choices = fetchedFieldOptionsCache[field.name]; - } - return fieldCopy; - }); -} - -/** - * check if we have any form fields with name error and type blank - */ -export function hasErrorInFields({fields}: {fields: IssueConfigField[]}) { - return fields.some(field => field.name === 'error' && field.type === 'blank'); -} From 5338d64209ca906767c0be9e7cfa4d567d190582 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Wed, 1 Apr 2026 14:26:03 +0200 Subject: [PATCH 02/16] ref: use a key instead of a useEffect --- .../backendJsonFormAdapter/submitForm.tsx | 47 ++++------ .../externalIssues/externalIssueForm.spec.tsx | 86 +++++++++++++++++++ .../externalIssues/externalIssueForm.tsx | 14 +++ 3 files changed, 116 insertions(+), 31 deletions(-) diff --git a/static/app/components/backendJsonFormAdapter/submitForm.tsx b/static/app/components/backendJsonFormAdapter/submitForm.tsx index 00629eb80f06ac..8ac59c6de58a5a 100644 --- a/static/app/components/backendJsonFormAdapter/submitForm.tsx +++ b/static/app/components/backendJsonFormAdapter/submitForm.tsx @@ -1,4 +1,4 @@ -import {useEffect, useMemo, useRef} from 'react'; +import {useMemo} from 'react'; import {defaultFormOptions, useScrapsForm} from '@sentry/scraps/form'; import {Stack} from '@sentry/scraps/layout'; @@ -36,6 +36,12 @@ interface BackendJsonSubmitFormProps { * (e.g., a modal footer). If not provided, the submit button renders inline. */ footer?: (props: {SubmitButton: any; disabled: boolean}) => React.ReactNode; + /** + * Override default values for specific fields. Takes precedence over + * `field.default`. Useful for preserving dynamic field selections + * across form remounts. + */ + initialValues?: Record; /** * Whether the form is in a loading state (e.g., dynamic field refetch in progress). */ @@ -55,12 +61,14 @@ interface BackendJsonSubmitFormProps { } function computeDefaultValues( - fields: JsonFormAdapterFieldConfig[] + fields: JsonFormAdapterFieldConfig[], + initialValues?: Record ): Record { const defaults: Record = {}; for (const field of fields) { if (field.name && field.type !== 'blank') { - defaults[field.name] = field.default ?? getDefaultForType(field.type); + defaults[field.name] = + initialValues?.[field.name] ?? field.default ?? getDefaultForType(field.type); } } return defaults; @@ -94,13 +102,16 @@ export function BackendJsonSubmitForm({ onSubmit, submitLabel, submitDisabled, + initialValues, isLoading, dynamicFieldValues, onFieldChange, footer, }: BackendJsonSubmitFormProps) { - const defaultValues = useMemo(() => computeDefaultValues(fields), [fields]); - const prevFieldsRef = useRef(fields); + const defaultValues = useMemo( + () => computeDefaultValues(fields, initialValues), + [fields, initialValues] + ); const form = useScrapsForm({ ...defaultFormOptions, @@ -110,32 +121,6 @@ export function BackendJsonSubmitForm({ }, }); - // When fields change (e.g., dynamic refetch), set values for new fields - // while preserving user-entered values for existing fields - useEffect(() => { - const prevNames = new Set( - prevFieldsRef.current.filter(f => f.type !== 'blank').map(f => f.name) - ); - const currentNames = new Set(fields.filter(f => f.type !== 'blank').map(f => f.name)); - - // Only act if the set of field names has changed - if ( - prevNames.size !== currentNames.size || - [...prevNames].some(n => !currentNames.has(n)) - ) { - for (const field of fields) { - if (field.name && field.type !== 'blank' && !prevNames.has(field.name)) { - // New field — set its default value - form.setFieldValue( - field.name as never, - (field.default ?? getDefaultForType(field.type)) as never - ); - } - } - } - prevFieldsRef.current = fields; - }, [fields, form]); - const hasErrors = fields.some( field => field.name === 'error' && field.type === 'blank' ); diff --git a/static/app/components/externalIssues/externalIssueForm.spec.tsx b/static/app/components/externalIssues/externalIssueForm.spec.tsx index 79b646a81dcf7f..7121418da3fe58 100644 --- a/static/app/components/externalIssues/externalIssueForm.spec.tsx +++ b/static/app/components/externalIssues/externalIssueForm.spec.tsx @@ -321,6 +321,92 @@ describe('ExternalIssueForm', () => { expect(screen.queryByRole('textbox', {name: 'Reporter'})).not.toBeInTheDocument(); expect(submitButton).toBeEnabled(); }); + + it('should reset field values when dynamic refetch returns new config', async () => { + MockApiClient.addMockResponse({ + url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/`, + match: [MockApiClient.matchQuery({action: 'create'})], + body: { + createIssueConfig: [ + { + label: 'Project', + required: true, + choices: [ + ['#proj-1', 'Project 1'], + ['#proj-2', 'Project 2'], + ], + type: 'select', + name: 'project', + updatesForm: true, + }, + { + label: 'Summary', + required: false, + type: 'text', + name: 'summary', + }, + ], + }, + }); + // After selecting Project 1, the refetch returns Summary with a new default + MockApiClient.addMockResponse({ + url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/`, + match: [MockApiClient.matchQuery({action: 'create', project: '#proj-1'})], + body: { + createIssueConfig: [ + { + label: 'Project', + required: true, + choices: [ + ['#proj-1', 'Project 1'], + ['#proj-2', 'Project 2'], + ], + type: 'select', + name: 'project', + updatesForm: true, + }, + { + label: 'Summary', + required: false, + type: 'text', + name: 'summary', + default: 'New default from server', + }, + ], + }, + }); + + render( + , + {organization} + ); + await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); + + // Type something into Summary + const summaryInput = screen.getByRole('textbox', {name: 'Summary'}); + await userEvent.type(summaryInput, 'User typed text'); + expect(summaryInput).toHaveValue('User typed text'); + + // Select a project (triggers dynamic refetch) + await selectEvent.select( + screen.getByRole('textbox', {name: 'Project'}), + 'Project 1' + ); + + // Summary should be reset to the new default from the server, not preserved + expect(screen.getByRole('textbox', {name: 'Summary'})).toHaveValue( + 'New default from server' + ); + }); }); describe('link', () => { let externalIssueField!: any; diff --git a/static/app/components/externalIssues/externalIssueForm.tsx b/static/app/components/externalIssues/externalIssueForm.tsx index 0b1c4ae9728c9f..045729260458d3 100644 --- a/static/app/components/externalIssues/externalIssueForm.tsx +++ b/static/app/components/externalIssues/externalIssueForm.tsx @@ -298,9 +298,14 @@ export function ExternalIssueForm({ ] ); + // Track the field that triggered the last dynamic refetch so we can + // preserve its value when the form remounts with new config. + const [lastChangedField, setLastChangedField] = useState>({}); + const onFieldChange = useCallback( (fieldName: string, value: unknown) => { if (dynamicFieldValues.hasOwnProperty(fieldName)) { + setLastChangedField({[fieldName]: value}); setDynamicFieldValue(fieldName, value); refetchWithDynamicFields({ ...dynamicFieldValues, @@ -323,6 +328,13 @@ export function ExternalIssueForm({ return formFields.some(field => field.name === 'error' && field.type === 'blank'); }, [formFields]); + // Key changes when field config changes, forcing the form to remount with fresh defaults. + // Includes field names and defaults so the form resets even when only defaults change. + const formKey = useMemo( + () => formFields.map(f => `${f.name}:${JSON.stringify(f.default)}`).join(','), + [formFields] + ); + if (isPending) { return ( @@ -369,7 +381,9 @@ export function ExternalIssueForm({ Date: Wed, 1 Apr 2026 14:52:24 +0200 Subject: [PATCH 03/16] fix: field validation errors and schema --- .../backendJsonFormAdapter/submitForm.tsx | 70 +++++++++------ .../externalIssues/externalIssueForm.spec.tsx | 85 +++++++++++++++++-- 2 files changed, 123 insertions(+), 32 deletions(-) diff --git a/static/app/components/backendJsonFormAdapter/submitForm.tsx b/static/app/components/backendJsonFormAdapter/submitForm.tsx index 8ac59c6de58a5a..d27a2a5c015a12 100644 --- a/static/app/components/backendJsonFormAdapter/submitForm.tsx +++ b/static/app/components/backendJsonFormAdapter/submitForm.tsx @@ -1,11 +1,15 @@ import {useMemo} from 'react'; +import {z} from 'zod'; import {defaultFormOptions, useScrapsForm} from '@sentry/scraps/form'; import {Stack} from '@sentry/scraps/layout'; +import {addErrorMessage} from 'sentry/actionCreators/indicator'; import {Client} from 'sentry/api'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; +import {t} from 'sentry/locale'; import type {SelectValue} from 'sentry/types/core'; +import {RequestError} from 'sentry/utils/requestError/requestError'; import type {JsonFormAdapterFieldConfig} from './types'; import {getDefaultForType, transformChoices} from './utils'; @@ -60,6 +64,26 @@ interface BackendJsonSubmitFormProps { submitLabel?: string; } +/** + * Build a Zod schema that validates required fields are non-empty. + */ +function buildValidationSchema(fields: JsonFormAdapterFieldConfig[]) { + const shape: Record = {}; + for (const field of fields) { + if (field.type === 'blank') { + continue; + } + if (field.required) { + shape[field.name] = z + .any() + .refine(val => val !== null && val !== undefined && val !== '', { + message: t('This field is required'), + }); + } + } + return z.object(shape).passthrough(); +} + function computeDefaultValues( fields: JsonFormAdapterFieldConfig[], initialValues?: Record @@ -113,11 +137,24 @@ export function BackendJsonSubmitForm({ [fields, initialValues] ); + const validationSchema = useMemo(() => buildValidationSchema(fields), [fields]); + const form = useScrapsForm({ ...defaultFormOptions, defaultValues, + validators: { + onSubmit: validationSchema, + }, onSubmit: async ({value}) => { - await onSubmit(value); + try { + await onSubmit(value); + } catch (err) { + if (err instanceof RequestError) { + const detail = err.responseJSON?.detail; + const message = typeof detail === 'string' ? detail : detail?.message; + addErrorMessage(message ?? t('An error occurred while submitting')); + } + } }, }); @@ -125,20 +162,13 @@ export function BackendJsonSubmitForm({ field => field.name === 'error' && field.type === 'blank' ); - const requiredFields = useMemo( - () => fields.filter(f => f.required && f.type !== 'blank').map(f => f.name), - [fields] - ); + const buttonDisabled = hasErrors || !!submitDisabled || !!isLoading; - const renderSubmitButton = (disabled: boolean) => { - if (footer) { - return footer({ - SubmitButton: form.SubmitButton, - disabled, - }); - } - return {submitLabel}; - }; + const submitButton = footer ? ( + footer({SubmitButton: form.SubmitButton, disabled: buttonDisabled}) + ) : ( + {submitLabel} + ); return ( @@ -158,17 +188,7 @@ export function BackendJsonSubmitForm({ ))} )} - state.values}> - {(values: any) => { - const hasUnfilledRequired = requiredFields.some(name => { - const val = values[name]; - return val === null || val === undefined || val === ''; - }); - return renderSubmitButton( - hasErrors || !!submitDisabled || !!isLoading || hasUnfilledRequired - ); - }} - + {submitButton} ); } diff --git a/static/app/components/externalIssues/externalIssueForm.spec.tsx b/static/app/components/externalIssues/externalIssueForm.spec.tsx index 7121418da3fe58..9390667938d1ab 100644 --- a/static/app/components/externalIssues/externalIssueForm.spec.tsx +++ b/static/app/components/externalIssues/externalIssueForm.spec.tsx @@ -11,7 +11,10 @@ import { } from 'sentry-test/reactTestingLibrary'; import {selectEvent} from 'sentry-test/selectEvent'; +import {addErrorMessage} from 'sentry/actionCreators/indicator'; import {ExternalIssueForm} from 'sentry/components/externalIssues/externalIssueForm'; + +jest.mock('sentry/actionCreators/indicator'); import { makeClosableHeader, makeCloseButton, @@ -198,6 +201,81 @@ describe('ExternalIssueForm', () => { expect(submitButton).toBeDisabled(); }); + it('should not submit when required fields are empty', async () => { + formConfig = { + createIssueConfig: [ + { + label: 'Title', + required: true, + type: 'string', + name: 'title', + default: '', + }, + { + label: 'Repo', + required: true, + type: 'select', + name: 'repo', + choices: [ + ['repo-1', 'Repo 1'], + ['repo-2', 'Repo 2'], + ], + }, + ], + }; + MockApiClient.addMockResponse({ + url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/`, + body: formConfig, + }); + + const submitRequest = MockApiClient.addMockResponse({ + url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/`, + method: 'POST', + body: {}, + }); + + await renderComponent(); + + // Click submit without filling required fields + await userEvent.click(screen.getByRole('button', {name: 'Create Issue'})); + + // Should NOT have made an API request + expect(submitRequest).not.toHaveBeenCalled(); + expect(closeModal).not.toHaveBeenCalled(); + }); + + it('should show an error toast when the submit request fails', async () => { + formConfig = { + createIssueConfig: [ + { + label: 'Title', + required: true, + type: 'string', + name: 'title', + default: 'Default Title', + }, + ], + }; + MockApiClient.addMockResponse({ + url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/`, + body: formConfig, + }); + + MockApiClient.addMockResponse({ + url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/`, + method: 'POST', + statusCode: 400, + body: {detail: 'Something went wrong'}, + }); + + await renderComponent(); + + await userEvent.click(screen.getByRole('button', {name: 'Create Issue'})); + + await waitFor(() => expect(addErrorMessage).toHaveBeenCalled()); + expect(closeModal).not.toHaveBeenCalled(); + }); + it('should refetch the form when a dynamic field is changed', async () => { const initialQuery = MockApiClient.addMockResponse({ url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/`, @@ -304,12 +382,6 @@ describe('ExternalIssueForm', () => { expect(screen.getByRole('textbox', {name: 'Summary'})).toBeInTheDocument(); expect(screen.getByRole('textbox', {name: 'Reporter'})).toBeInTheDocument(); - // We should also respect new required fields - const submitButton = screen.getByRole('button', {name: 'Create Issue'}); - expect(submitButton).toBeDisabled(); - await selectEvent.select(screen.getByRole('textbox', {name: 'Reporter'}), 'User 1'); - await waitFor(() => expect(submitButton).toBeEnabled()); - // Swapping projects should refetch, and remove stale fields await selectEvent.select( screen.getByRole('textbox', {name: 'Project'}), @@ -319,7 +391,6 @@ describe('ExternalIssueForm', () => { expect(screen.queryByRole('textbox', {name: 'Summary'})).not.toBeInTheDocument(); expect(screen.queryByRole('textbox', {name: 'Reporter'})).not.toBeInTheDocument(); - expect(submitButton).toBeEnabled(); }); it('should reset field values when dynamic refetch returns new config', async () => { From ce5bd084c5938cd203785aaa8629849aa46b299d Mon Sep 17 00:00:00 2001 From: TkDodo Date: Wed, 1 Apr 2026 15:42:37 +0200 Subject: [PATCH 04/16] ref: update default value retrieval for form fields and enhance multi-select handling --- .../backendJsonFormAdapter/index.tsx | 4 +- .../backendJsonFormAdapter/submitForm.tsx | 85 +++++++++++++------ .../backendJsonFormAdapter/utils.ts | 10 +-- .../externalIssues/externalIssueForm.spec.tsx | 33 +++++++ 4 files changed, 99 insertions(+), 33 deletions(-) diff --git a/static/app/components/backendJsonFormAdapter/index.tsx b/static/app/components/backendJsonFormAdapter/index.tsx index b625d3ea971c27..6a75f1f92adb00 100644 --- a/static/app/components/backendJsonFormAdapter/index.tsx +++ b/static/app/components/backendJsonFormAdapter/index.tsx @@ -11,7 +11,7 @@ import {ChoiceMapperDropdown, ChoiceMapperTable} from './choiceMapperAdapter'; import {ProjectMapperAddRow, ProjectMapperTable} from './projectMapperAdapter'; import {TableBody, TableHeaderRow} from './tableAdapter'; import type {FieldValue, JsonFormAdapterFieldConfig} from './types'; -import {getDefaultForType, getZodType, transformChoices} from './utils'; +import {getDefaultForField, getZodType, transformChoices} from './utils'; interface BackendJsonFormAdapterProps< TField extends JsonFormAdapterFieldConfig, @@ -40,7 +40,7 @@ export function BackendJsonFormAdapter< [fieldName, field.type] ); - const value = initialValue ?? field.default ?? getDefaultForType(field.type); + const value = initialValue ?? field.default ?? getDefaultForField(field); if (field.type === 'table') { return ( diff --git a/static/app/components/backendJsonFormAdapter/submitForm.tsx b/static/app/components/backendJsonFormAdapter/submitForm.tsx index d27a2a5c015a12..188a9c5e8bcb6c 100644 --- a/static/app/components/backendJsonFormAdapter/submitForm.tsx +++ b/static/app/components/backendJsonFormAdapter/submitForm.tsx @@ -12,7 +12,7 @@ import type {SelectValue} from 'sentry/types/core'; import {RequestError} from 'sentry/utils/requestError/requestError'; import type {JsonFormAdapterFieldConfig} from './types'; -import {getDefaultForType, transformChoices} from './utils'; +import {getDefaultForField, transformChoices} from './utils'; /** * API client without base URL prefix, needed for async select fields @@ -92,7 +92,7 @@ function computeDefaultValues( for (const field of fields) { if (field.name && field.type !== 'blank') { defaults[field.name] = - initialValues?.[field.name] ?? field.default ?? getDefaultForType(field.type); + initialValues?.[field.name] ?? field.default ?? getDefaultForField(field); } } return defaults; @@ -269,6 +269,46 @@ function SubmitFormField({ // Async select: fetch options from URL as user types. // Show static choices as initial options before any search. const staticOptions = transformChoices(field.choices); + const asyncQueryOptions = (debouncedInput: string) => ({ + queryKey: [ + 'backend-json-async-select', + field.name, + field.url, + debouncedInput, + dynamicFieldValues, + ], + queryFn: async (): Promise>> => { + if (!debouncedInput) { + return staticOptions; + } + return API_CLIENT.requestPromise(field.url!, { + query: buildAsyncSelectQuery( + field.name, + debouncedInput, + dynamicFieldValues + ), + }); + }, + // Always enabled so static choices show immediately + enabled: true, + }); + if (field.multiple) { + return ( + + + + ); + } return ( ({ - queryKey: [ - 'backend-json-async-select', - field.name, - field.url, - debouncedInput, - dynamicFieldValues, - ], - queryFn: async (): Promise>> => { - if (!debouncedInput) { - return staticOptions; - } - return API_CLIENT.requestPromise(field.url!, { - query: buildAsyncSelectQuery( - field.name, - debouncedInput, - dynamicFieldValues - ), - }); - }, - // Always enabled so static choices show immediately - enabled: true, - })} + queryOptions={asyncQueryOptions} + /> + + ); + } + if (field.multiple) { + return ( + + ); diff --git a/static/app/components/backendJsonFormAdapter/utils.ts b/static/app/components/backendJsonFormAdapter/utils.ts index 171a9ed119a614..d34a6e7629e1ac 100644 --- a/static/app/components/backendJsonFormAdapter/utils.ts +++ b/static/app/components/backendJsonFormAdapter/utils.ts @@ -44,10 +44,8 @@ export function transformChoices( return choices.map(([value, label]) => ({value, label})); } -export function getDefaultForType( - fieldType: JsonFormAdapterFieldConfig['type'] -): unknown { - switch (fieldType) { +export function getDefaultForField(field: JsonFormAdapterFieldConfig): unknown { + switch (field.type) { case 'boolean': return false; case 'string': @@ -66,11 +64,11 @@ export function getDefaultForType( return []; case 'select': case 'choice': - return null; + return field.multiple ? [] : null; case 'blank': return null; default: - unreachable(fieldType); + unreachable(field); return ''; } } diff --git a/static/app/components/externalIssues/externalIssueForm.spec.tsx b/static/app/components/externalIssues/externalIssueForm.spec.tsx index 9390667938d1ab..88c29ce9606189 100644 --- a/static/app/components/externalIssues/externalIssueForm.spec.tsx +++ b/static/app/components/externalIssues/externalIssueForm.spec.tsx @@ -80,6 +80,39 @@ describe('ExternalIssueForm', () => { await renderComponent(); }); + it('should render multi-select fields and allow selecting multiple values', async () => { + formConfig = { + createIssueConfig: [ + { + label: 'Labels', + required: false, + type: 'select', + name: 'labels', + multiple: true, + choices: [ + ['bug', 'bug'], + ['feature', 'feature'], + ['docs', 'docs'], + ], + }, + ], + }; + MockApiClient.addMockResponse({ + url: `/organizations/org-slug/issues/${group.id}/integrations/${integration.id}/`, + body: formConfig, + }); + + await renderComponent(); + + const labelsSelect = screen.getByRole('textbox', {name: 'Labels'}); + await selectEvent.select(labelsSelect, 'bug'); + await selectEvent.select(labelsSelect, 'feature'); + + // Both values should be visible as selected tags + expect(screen.getAllByText('bug').length).toBeGreaterThanOrEqual(1); + expect(screen.getAllByText('feature').length).toBeGreaterThanOrEqual(1); + }); + it('should submit the form and close the modal on success', async () => { formConfig = { createIssueConfig: [ From 5d2a11096292cee1c41050ee644cde2f4381a518 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Thu, 2 Apr 2026 10:22:29 +0200 Subject: [PATCH 05/16] ref: success notifications --- static/app/components/externalIssues/externalIssueForm.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/static/app/components/externalIssues/externalIssueForm.tsx b/static/app/components/externalIssues/externalIssueForm.tsx index 045729260458d3..6981725c686a28 100644 --- a/static/app/components/externalIssues/externalIssueForm.tsx +++ b/static/app/components/externalIssues/externalIssueForm.tsx @@ -76,7 +76,7 @@ const SUBMIT_LABEL_BY_ACTION = { interface ExternalIssueFormProps extends ModalRenderProps { group: Group; integration: Integration; - onChange: (onSuccess?: () => void, onError?: () => void) => void; + onChange: () => void; } function makeIntegrationIssueConfigQueryKey({ @@ -275,7 +275,8 @@ export function ExternalIssueForm({ external_issue_provider: integration.provider.key, external_issue_type: 'first_party', }); - onChange(() => addSuccessMessage(MESSAGES_BY_ACTION[action])); + addSuccessMessage(MESSAGES_BY_ACTION[action]); + onChange(); closeModal(); submitSpan?.end(); return data; From 80346a16f73d8365262b5d7b160b20486d6a6683 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Thu, 2 Apr 2026 10:43:15 +0200 Subject: [PATCH 06/16] heading --- static/app/components/externalIssues/externalIssueForm.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/static/app/components/externalIssues/externalIssueForm.tsx b/static/app/components/externalIssues/externalIssueForm.tsx index 6981725c686a28..5ad1d577377049 100644 --- a/static/app/components/externalIssues/externalIssueForm.tsx +++ b/static/app/components/externalIssues/externalIssueForm.tsx @@ -4,6 +4,7 @@ import type {Span} from '@sentry/core'; import * as Sentry from '@sentry/react'; import {TabList, Tabs} from '@sentry/scraps/tabs'; +import {Heading} from '@sentry/scraps/text'; import {addSuccessMessage} from 'sentry/actionCreators/indicator'; import {openModal, type ModalRenderProps} from 'sentry/actionCreators/modal'; @@ -340,7 +341,7 @@ export function ExternalIssueForm({ return (
-

{title}

+ {title}
@@ -358,7 +359,7 @@ export function ExternalIssueForm({ return (
-

{title}

+ {title}
@@ -370,7 +371,7 @@ export function ExternalIssueForm({ return (
-

{title}

+ {title}
From 08264bd45fb059d4918a689fe386e67426f5bbe4 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Thu, 2 Apr 2026 10:52:00 +0200 Subject: [PATCH 07/16] ref: inline field for better type inference --- .../backendJsonFormAdapter/submitForm.tsx | 405 +++++++++--------- 1 file changed, 192 insertions(+), 213 deletions(-) diff --git a/static/app/components/backendJsonFormAdapter/submitForm.tsx b/static/app/components/backendJsonFormAdapter/submitForm.tsx index 188a9c5e8bcb6c..72ede5093aea74 100644 --- a/static/app/components/backendJsonFormAdapter/submitForm.tsx +++ b/static/app/components/backendJsonFormAdapter/submitForm.tsx @@ -1,4 +1,5 @@ import {useMemo} from 'react'; +import {queryOptions} from '@tanstack/react-query'; import {z} from 'zod'; import {defaultFormOptions, useScrapsForm} from '@sentry/scraps/form'; @@ -178,13 +179,197 @@ export function BackendJsonSubmitForm({ {fields .filter(field => field.hasOwnProperty('name') && field.type !== 'blank') .map(field => ( - + + {fieldApi => { + const handleChange = (value: unknown) => { + fieldApi.handleChange(value); + if (field.updatesForm && onFieldChange) { + onFieldChange(field.name, value); + } + }; + + switch (field.type) { + case 'boolean': + return ( + + + + ); + case 'textarea': + return ( + + + + ); + case 'number': + return ( + + + + ); + case 'select': + case 'choice': + if (field.url) { + // Async select: fetch options from URL as user types. + // Show static choices as initial options before any search. + const staticOptions = transformChoices(field.choices); + const asyncQueryOptions = (debouncedInput: string) => + queryOptions({ + queryKey: [ + 'backend-json-async-select', + field.name, + field.url, + debouncedInput, + dynamicFieldValues, + ], + queryFn: async (): Promise< + Array> + > => { + if (!debouncedInput) { + return staticOptions; + } + return API_CLIENT.requestPromise(field.url!, { + query: buildAsyncSelectQuery( + field.name, + debouncedInput, + dynamicFieldValues + ), + }); + }, + }); + if (field.multiple) { + return ( + + + + ); + } + return ( + + + + ); + } + if (field.multiple) { + return ( + + + + ); + } + return ( + + + + ); + case 'secret': + return ( + + + + ); + case 'string': + case 'text': + case 'url': + case 'email': + return ( + + + + ); + default: + return null; + } + }} + ))} )} @@ -192,209 +377,3 @@ export function BackendJsonSubmitForm({ ); } - -/** - * Renders a single field within the submit form. - */ -function SubmitFormField({ - form, - field, - dynamicFieldValues, - onFieldChange, -}: { - field: JsonFormAdapterFieldConfig; - form: any; - dynamicFieldValues?: Record; - onFieldChange?: (fieldName: string, value: unknown) => void; -}) { - return ( - - {(fieldApi: any) => { - const handleChange = (value: unknown) => { - fieldApi.handleChange(value as never); - if (field.updatesForm && onFieldChange) { - onFieldChange(field.name, value); - } - }; - - switch (field.type) { - case 'boolean': - return ( - - - - ); - case 'textarea': - return ( - - - - ); - case 'number': - return ( - - - - ); - case 'select': - case 'choice': - if (field.url) { - // Async select: fetch options from URL as user types. - // Show static choices as initial options before any search. - const staticOptions = transformChoices(field.choices); - const asyncQueryOptions = (debouncedInput: string) => ({ - queryKey: [ - 'backend-json-async-select', - field.name, - field.url, - debouncedInput, - dynamicFieldValues, - ], - queryFn: async (): Promise>> => { - if (!debouncedInput) { - return staticOptions; - } - return API_CLIENT.requestPromise(field.url!, { - query: buildAsyncSelectQuery( - field.name, - debouncedInput, - dynamicFieldValues - ), - }); - }, - // Always enabled so static choices show immediately - enabled: true, - }); - if (field.multiple) { - return ( - - - - ); - } - return ( - - - - ); - } - if (field.multiple) { - return ( - - - - ); - } - return ( - - - - ); - case 'secret': - return ( - - - - ); - case 'string': - case 'text': - case 'url': - case 'email': - return ( - - - - ); - default: - return null; - } - }} - - ); -} From ea6973e2ebbab78f2e9f3e7ec300411b3710bf86 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Thu, 2 Apr 2026 12:13:24 +0200 Subject: [PATCH 08/16] ref: migrate ticketRuleModal to scraps form --- .../backendJsonFormAdapter/submitForm.tsx | 2 +- .../externalIssues/ticketRuleModal.spec.tsx | 55 +++-- .../externalIssues/ticketRuleModal.tsx | 206 +++++++----------- 3 files changed, 114 insertions(+), 149 deletions(-) diff --git a/static/app/components/backendJsonFormAdapter/submitForm.tsx b/static/app/components/backendJsonFormAdapter/submitForm.tsx index 72ede5093aea74..619468c6b1faa8 100644 --- a/static/app/components/backendJsonFormAdapter/submitForm.tsx +++ b/static/app/components/backendJsonFormAdapter/submitForm.tsx @@ -30,7 +30,7 @@ interface BackendJsonSubmitFormProps { * Called when the form is submitted. Should return a promise that * resolves on success or rejects/throws on error. */ - onSubmit: (values: Record) => Promise; + onSubmit: (values: Record) => Promise | void; /** * Current values of dynamic fields, passed as query params to async select endpoints. */ diff --git a/static/app/components/externalIssues/ticketRuleModal.spec.tsx b/static/app/components/externalIssues/ticketRuleModal.spec.tsx index c70a79164209ed..602fea34f23397 100644 --- a/static/app/components/externalIssues/ticketRuleModal.spec.tsx +++ b/static/app/components/externalIssues/ticketRuleModal.spec.tsx @@ -1,6 +1,12 @@ import {OrganizationFixture} from 'sentry-fixture/organization'; -import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; +import { + render, + screen, + userEvent, + waitFor, + waitForElementToBeRemoved, +} from 'sentry-test/reactTestingLibrary'; import {selectEvent} from 'sentry-test/selectEvent'; import {addSuccessMessage} from 'sentry/actionCreators/indicator'; @@ -103,7 +109,7 @@ describe('ProjectAlerts -> TicketRuleModal', () => { organization, } ); - expect(await screen.findByTestId('loading-indicator')).not.toBeInTheDocument(); + await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); return wrapper; }; @@ -122,16 +128,6 @@ describe('ProjectAlerts -> TicketRuleModal', () => { await submitSuccess(); }); - it('submit button shall be disabled if form is incomplete', async () => { - await renderTicketRuleModal(); - await userEvent.click(screen.getByRole('textbox', {name: 'Reporter'})); - 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 () => { await renderTicketRuleModal(); await selectEvent.select(screen.getByRole('textbox', {name: 'Reporter'}), 'a'); @@ -321,20 +317,31 @@ describe('ProjectAlerts -> TicketRuleModal', () => { issueTypeLabel ); - // Component makes 1 request per character typed. - let txt = ''; - for (const char of 'Joe') { - txt += char; - MockApiClient.addMockResponse({ - url: `http://example.com?field=assignee&issuetype=10001&project=10000&query=${txt}`, - method: 'GET', - body: [{label: 'Joe', value: 'Joe'}], - }); - } + // Catch-all mock for async search endpoint + MockApiClient.addMockResponse({ + url: 'http://example.com', + method: 'GET', + body: [], + }); + // Specific mock for the full "Joe" search (after debounce) + const searchResponse = MockApiClient.addMockResponse({ + url: 'http://example.com', + match: [ + MockApiClient.matchQuery({ + field: 'assignee', + query: 'Joe', + }), + ], + method: 'GET', + body: [{label: 'Joe', value: 'Joe'}], + }); + expect(dynamicQuery).toHaveBeenCalled(); const menu = screen.getByRole('textbox', {name: 'Assignee'}); - await selectEvent.openMenu(menu); - await userEvent.type(menu, 'Joe{Escape}'); + await userEvent.click(menu); + await userEvent.type(menu, 'Joe'); + + await waitFor(() => expect(searchResponse).toHaveBeenCalled()); await selectEvent.select(menu, 'Joe'); await submitSuccess(); diff --git a/static/app/components/externalIssues/ticketRuleModal.tsx b/static/app/components/externalIssues/ticketRuleModal.tsx index aa715119c1bcfc..c6cac9a387718d 100644 --- a/static/app/components/externalIssues/ticketRuleModal.tsx +++ b/static/app/components/externalIssues/ticketRuleModal.tsx @@ -1,4 +1,4 @@ -import {useCallback, useEffect, useMemo, useState} from 'react'; +import {Fragment, useCallback, useEffect, useMemo, useState} from 'react'; import styled from '@emotion/styled'; import * as Sentry from '@sentry/react'; @@ -7,19 +7,10 @@ import {ExternalLink} from '@sentry/scraps/link'; import {addSuccessMessage} from 'sentry/actionCreators/indicator'; import type {ModalRenderProps} from 'sentry/actionCreators/modal'; import type {RequestOptions, ResponseMeta} from 'sentry/api'; -import {ExternalForm} from 'sentry/components/externalIssues/externalForm'; -import {useAsyncOptionsCache} from 'sentry/components/externalIssues/useAsyncOptionsCache'; +import {BackendJsonSubmitForm} from 'sentry/components/backendJsonFormAdapter/submitForm'; +import type {JsonFormAdapterFieldConfig} from 'sentry/components/backendJsonFormAdapter/types'; import {useDynamicFields} from 'sentry/components/externalIssues/useDynamicFields'; -import type {ExternalIssueFormErrors} from 'sentry/components/externalIssues/utils'; -import { - getConfigName, - getFieldProps, - getOptions, - hasErrorInFields, - loadAsyncThenFetchAllFields, -} from 'sentry/components/externalIssues/utils'; -import type {FormProps} from 'sentry/components/forms/form'; -import {FormModel} from 'sentry/components/forms/model'; +import {getConfigName} from 'sentry/components/externalIssues/utils'; import type {FieldValue} from 'sentry/components/forms/types'; import {LoadingError} from 'sentry/components/loadingError'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; @@ -79,10 +70,10 @@ export function TicketRuleModal({ closeModal, Header, Body, + Footer, }: TicketRuleModalProps) { const action = 'create'; const title = t('Issue Link Settings'); - const [model] = useState(() => new FormModel()); const queryClient = useQueryClient(); const api = useApi({persistInFlight: true}); const organization = useOrganization(); @@ -97,15 +88,6 @@ export function TicketRuleModal({ return Object.values(instance?.dynamic_form_fields || {}); }); - const initialOptionsCache = useMemo(() => { - return Object.fromEntries( - issueConfigFieldsCache - .filter(field => field.url) - .map(field => [field.name, field.choices as Choices]) - ); - }, [issueConfigFieldsCache]); - - const {cache, updateCache} = useAsyncOptionsCache(initialOptionsCache); const [isDynamicallyRefetching, setIsDynamicallyRefetching] = useState(false); const {url: endpointString} = parseQueryKey( @@ -225,7 +207,7 @@ export function TicketRuleModal({ * Clean up the form data before saving it to state. */ const cleanData = useCallback( - (data: Record) => { + (data: Record) => { const formData: { [key: string]: any; integration?: string | number; @@ -244,96 +226,76 @@ export function TicketRuleModal({ [validAndSavableFieldNames, issueConfigFieldsCache, instance] ); - const onFormSubmit = useCallback['onSubmit']>( - (data, _success, _error, e, modelParam) => { - // This is a "fake form", so don't actually POST to an endpoint. - e.preventDefault(); - e.stopPropagation(); - - if (modelParam.validateForm()) { - onSubmitAction(cleanData(data), cache); - addSuccessMessage(t('Changes applied.')); - closeModal(); + const handleSubmit = useCallback( + (values: Record) => { + // Build a cache of field choices for async fields so the parent can persist them + const fieldOptionsCache: Record = {}; + const config = integrationDetails?.[getConfigName(action)] || []; + for (const field of config) { + if (field.url && field.choices) { + fieldOptionsCache[field.name] = field.choices as Choices; + } } + + onSubmitAction(cleanData(values) as Record, fieldOptionsCache); + addSuccessMessage(t('Changes applied.')); + closeModal(); }, - [cleanData, cache, onSubmitAction, closeModal] + [cleanData, onSubmitAction, closeModal, integrationDetails] ); + const [lastChangedField, setLastChangedField] = useState>({}); + const onFieldChange = useCallback( - (fieldName: string, value: FieldValue) => { + (fieldName: string, value: unknown) => { setShowInstanceValues(false); if (dynamicFieldValues.hasOwnProperty(fieldName)) { - setDynamicFieldValue(fieldName, value); - refetchWithDynamicFields({...dynamicFieldValues, [fieldName]: value}); + setLastChangedField({[fieldName]: value}); + setDynamicFieldValue(fieldName, value as FieldValue); + refetchWithDynamicFields({ + ...dynamicFieldValues, + [fieldName]: value as FieldValue, + }); } }, [dynamicFieldValues, refetchWithDynamicFields, setDynamicFieldValue] ); - // Even if we pass onFieldChange as a prop, the model only uses the first instance. - // In order to use the correct dynamicFieldValues, we need to set it whenever this function is changed. - useEffect(() => { - model.setFormOptions({onFieldChange}); - }, [model, onFieldChange]); - - const getTicketRuleFieldProps = useCallback( - (field: IssueConfigField) => { - return getFieldProps({ - field, - loadOptions: (input: string) => - getOptions({ - field, - input, - dynamicFieldValues, - model, - successCallback: updateCache, - }), - }); - }, - [updateCache, dynamicFieldValues, model] - ); - /** * Set the initial data from the Rule, replace `title` and `description` with * disabled inputs, and use the cached dynamic choices. */ - const cleanFields = useMemo(() => { - const fields: IssueConfigField[] = [ + const cleanFields = useMemo((): JsonFormAdapterFieldConfig[] => { + const staticFields: JsonFormAdapterFieldConfig[] = [ { name: 'title', label: 'Title', type: 'string', default: 'This will be the same as the Sentry Issue.', disabled: true, - } as IssueConfigField, + }, { name: 'description', label: 'Description', type: 'string', default: 'This will be generated from the Sentry Issue details.', disabled: true, - } as IssueConfigField, + }, ]; - const cleanedFields = loadAsyncThenFetchAllFields({ - configName: getConfigName(action), - integrationDetails: integrationDetails || null, - fetchedFieldOptionsCache: cache, - }) + const configFields = (integrationDetails?.[getConfigName(action)] || + []) as JsonFormAdapterFieldConfig[]; + + const cleanedFields = configFields // Don't overwrite the default values for title and description. - .filter(field => !fields.map(f => f.name).includes(field.name)) + .filter(field => !staticFields.some(f => f.name === field.name)) .map(field => { // We only need to do the below operation if the form has not been modified. if (!showInstanceValues) { return field; } // Overwrite defaults with previously selected values if they exist. - // Certain fields such as priority (for Jira) have their options change - // because they depend on another field such as Project, so we need to - // check if the last selected value is in the list of available field choices. const prevChoice = instance?.[field.name]; - // Note that field.choices is an array of tuples, where each tuple - // contains a numeric id and string label, eg. ("10000", "EX") or ("1", "Bug") if (!prevChoice) { return field; @@ -341,43 +303,39 @@ export function TicketRuleModal({ let shouldDefaultChoice = true; - if (field.choices) { + if (field.type === 'select' || field.type === 'choice') { + const choices = field.choices || []; shouldDefaultChoice = !!(Array.isArray(prevChoice) - ? prevChoice.every(value => field.choices?.some(tuple => tuple[0] === value)) - : // Single-select fields have a single value, eg: 'a' - field.choices?.some(item => item[0] === prevChoice)); + ? prevChoice.every(value => choices.some(tuple => tuple[0] === value)) + : choices.some(item => item[0] === prevChoice)); } if (shouldDefaultChoice) { - field.default = prevChoice; + return {...field, default: prevChoice}; } return field; }); - return [...fields, ...cleanedFields]; - }, [instance, integrationDetails, cache, showInstanceValues]); + return [...staticFields, ...cleanedFields]; + }, [instance, integrationDetails, showInstanceValues]); const formErrors = useMemo(() => { - const errors: ExternalIssueFormErrors = {}; + const errors: Record = {}; for (const field of cleanFields) { - // If the field is a select and has a default value, make sure that the - // default value exists in the choices. Skip check if the default is not - // set, an empty string, or an empty array. if ( - field.type === 'select' && + (field.type === 'select' || field.type === 'choice') && field.default && !(Array.isArray(field.default) && !field.default.length) ) { const fieldChoices = (field.choices || []) as Choices; const found = fieldChoices.find(([value, _]) => Array.isArray(field.default) - ? field.default.includes(value) + ? (field.default as unknown[]).includes(value) : value === field.default ); if (!found) { errors[field.name] = ( - // eslint-disable-next-line @typescript-eslint/no-base-to-string {`Could not fetch saved option for ${field.label}. Please reselect.`} ); } @@ -386,20 +344,17 @@ export function TicketRuleModal({ return errors; }, [cleanFields]); - const initialData = useMemo(() => { - return cleanFields.reduce>( - (accumulator, field: IssueConfigField) => { - accumulator[field.name] = field.default; - return accumulator; - }, - {} - ); - }, [cleanFields]); - const hasFormErrors = useMemo(() => { - return hasErrorInFields({fields: cleanFields}); + return cleanFields.some(field => field.name === 'error' && field.type === 'blank'); }, [cleanFields]); + // Key changes when field config changes, forcing the form to remount with fresh defaults. + // Includes field names and defaults so the form resets even when only defaults change. + const formKey = useMemo( + () => cleanFields.map(f => `${f.name}:${JSON.stringify(f.default)}`).join(','), + [cleanFields] + ); + if (isPending) { return ; } @@ -414,26 +369,11 @@ export function TicketRuleModal({ } return ( - +
+

{title}

+
+ {link ? tct( @@ -445,9 +385,27 @@ export function TicketRuleModal({ {ticketType} )} - } - getFieldProps={getTicketRuleFieldProps} - /> + {Object.entries(formErrors).map(([name, errorNode]) => ( + {errorNode} + ))} + ( +
+ {t('Apply Changes')} +
+ )} + /> + +
); } From 03843853ace1aaf6879eb11281937cfcf85aa6f2 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Thu, 2 Apr 2026 12:19:39 +0200 Subject: [PATCH 09/16] ref: distinguish between backendJsonAutoSaveForm and backendJsonSubmitForm --- ...c.tsx => backendJsonAutoSaveForm.spec.tsx} | 16 ++++++------- ...{index.tsx => backendJsonAutoSaveForm.tsx} | 2 +- ...bmitForm.tsx => backendJsonSubmitForm.tsx} | 0 .../choiceMapperAdapter.spec.tsx | 22 ++++++++--------- .../projectMapperAdapter.spec.tsx | 22 ++++++++--------- .../tableAdapter.spec.tsx | 24 +++++++++---------- .../externalIssues/externalIssueForm.tsx | 2 +- .../externalIssues/ticketRuleModal.tsx | 2 +- .../configureIntegration.tsx | 4 ++-- 9 files changed, 47 insertions(+), 47 deletions(-) rename static/app/components/backendJsonFormAdapter/{index.spec.tsx => backendJsonAutoSaveForm.spec.tsx} (91%) rename static/app/components/backendJsonFormAdapter/{index.tsx => backendJsonAutoSaveForm.tsx} (99%) rename static/app/components/backendJsonFormAdapter/{submitForm.tsx => backendJsonSubmitForm.tsx} (100%) diff --git a/static/app/components/backendJsonFormAdapter/index.spec.tsx b/static/app/components/backendJsonFormAdapter/backendJsonAutoSaveForm.spec.tsx similarity index 91% rename from static/app/components/backendJsonFormAdapter/index.spec.tsx rename to static/app/components/backendJsonFormAdapter/backendJsonAutoSaveForm.spec.tsx index f7549915548824..5a7c7786e784f1 100644 --- a/static/app/components/backendJsonFormAdapter/index.spec.tsx +++ b/static/app/components/backendJsonFormAdapter/backendJsonAutoSaveForm.spec.tsx @@ -2,17 +2,17 @@ import {OrganizationFixture} from 'sentry-fixture/organization'; import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; -import {BackendJsonFormAdapter} from './'; +import {BackendJsonAutoSaveForm} from './backendJsonAutoSaveForm'; const org = OrganizationFixture(); const mutationOptions = { mutationFn: jest.fn().mockResolvedValue({}), }; -describe('BackendJsonFormAdapter', () => { +describe('BackendJsonAutoSaveForm', () => { it('renders boolean field as Switch', () => { render( - { it('renders text field as Input', () => { render( - { it('renders select field with options', () => { render( - { it('renders table field with add button', () => { render( - { it('boolean toggle triggers POST', async () => { render( - { it('handles disabled fields', () => { render( - ; } -export function BackendJsonFormAdapter< +export function BackendJsonAutoSaveForm< TField extends JsonFormAdapterFieldConfig, TData, TContext, diff --git a/static/app/components/backendJsonFormAdapter/submitForm.tsx b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx similarity index 100% rename from static/app/components/backendJsonFormAdapter/submitForm.tsx rename to static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx diff --git a/static/app/components/backendJsonFormAdapter/choiceMapperAdapter.spec.tsx b/static/app/components/backendJsonFormAdapter/choiceMapperAdapter.spec.tsx index ec3ce98392f640..77f2612077361c 100644 --- a/static/app/components/backendJsonFormAdapter/choiceMapperAdapter.spec.tsx +++ b/static/app/components/backendJsonFormAdapter/choiceMapperAdapter.spec.tsx @@ -2,7 +2,7 @@ import {OrganizationFixture} from 'sentry-fixture/organization'; import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; -import {BackendJsonFormAdapter} from './'; +import {BackendJsonAutoSaveForm} from './backendJsonAutoSaveForm'; const org = OrganizationFixture(); const mutationOptions = { @@ -12,7 +12,7 @@ const mutationOptions = { describe('ChoiceMapperAdapter', () => { it('renders choice_mapper with empty value showing only Add button', async () => { render( - { it('renders choice_mapper table with existing values', async () => { render( - { it('choice_mapper add row does not immediately submit', async () => { render( - { it('choice_mapper add row then fill select triggers mutation', async () => { render( - { it('choice_mapper does not submit until all columns in every row are filled', async () => { render( - { it('choice_mapper remove row triggers mutation', async () => { render( - { it('choice_mapper update cell value triggers mutation', async () => { render( - { }); render( - { }); render( - { }; render( - { it('renders empty state with two dropdowns and disabled Add button', () => { render( - { it('renders existing mappings with icon, label, link, arrow, IdBadge, delete', () => { render( - { it('shows "Deleted" for unknown mapped items', () => { render( - { it('shows "Deleted" for unknown Sentry projects', () => { render( - { it('add mapping triggers mutation', async () => { render( - { it('delete mapping triggers mutation', async () => { render( - { it('filters already-used external items from mapped dropdown', async () => { render( - { it('does not filter Sentry projects (many-to-one)', async () => { render( - { it('Add button disabled until both selections are made', async () => { render( - { }; render( - > @@ -34,7 +34,7 @@ const mutationOptions = { describe('TableAdapter', () => { it('renders empty state with only Add button visible', () => { render( - { it('renders existing rows with column headers and input fields', () => { render( - { it('add row does NOT immediately save', async () => { render( - { it('edit cell does NOT save on every keystroke', async () => { render( - { it('edit cell triggers mutation on blur when all fields filled', async () => { render( - { it('blur does NOT save if any non-id field is empty', async () => { render( - { it('delete row triggers mutation after confirmation', async () => { render( - { it('delete confirmation shows custom message', async () => { render( - { it('pressing Enter in a cell triggers save when all fields are filled', async () => { render( - { it('pressing Enter in a cell does NOT save when a field is empty', async () => { render( - { }; render( - {integration.configOrganization.map(fieldConfig => ( - Date: Thu, 2 Apr 2026 13:58:01 +0200 Subject: [PATCH 10/16] backendJsonSubmitForm tests --- .../backendJsonSubmitForm.spec.tsx | 670 ++++++++++++++++++ .../backendJsonSubmitForm.tsx | 89 ++- 2 files changed, 758 insertions(+), 1 deletion(-) create mode 100644 static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.spec.tsx diff --git a/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.spec.tsx b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.spec.tsx new file mode 100644 index 00000000000000..dbfdc950b137ac --- /dev/null +++ b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.spec.tsx @@ -0,0 +1,670 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; + +import { + render, + renderGlobalModal, + screen, + userEvent, + waitFor, +} from 'sentry-test/reactTestingLibrary'; +import {selectEvent} from 'sentry-test/selectEvent'; + +import {addErrorMessage} from 'sentry/actionCreators/indicator'; +import {RequestError} from 'sentry/utils/requestError/requestError'; + +import {BackendJsonSubmitForm} from './backendJsonSubmitForm'; + +jest.mock('sentry/actionCreators/indicator'); + +const org = OrganizationFixture(); + +describe('BackendJsonSubmitForm', () => { + const onSubmit = jest.fn().mockResolvedValue({}); + + afterEach(() => { + jest.clearAllMocks(); + MockApiClient.clearMockResponses(); + }); + + describe('basic field rendering', () => { + it('renders boolean field as Switch', () => { + render( + , + {organization: org} + ); + + expect(screen.getByText('Enable Sync')).toBeInTheDocument(); + expect(screen.getByRole('checkbox')).toBeInTheDocument(); + }); + + it('renders text field as Input', () => { + render( + , + {organization: org} + ); + + expect(screen.getByText('Webhook URL')).toBeInTheDocument(); + expect(screen.getByRole('textbox', {name: /webhook url/i})).toHaveValue( + 'https://example.com' + ); + }); + + it('renders textarea field', () => { + render( + , + {organization: org} + ); + + expect(screen.getByText('Description')).toBeInTheDocument(); + expect(screen.getByRole('textbox', {name: /description/i})).toHaveValue( + 'Hello world' + ); + }); + + it('renders select field with options', () => { + render( + , + {organization: org} + ); + + expect(screen.getByText('Priority')).toBeInTheDocument(); + expect(screen.getByText('Medium')).toBeInTheDocument(); + }); + + it('renders multiple fields together', () => { + render( + , + {organization: org} + ); + + expect(screen.getByText('Title')).toBeInTheDocument(); + expect(screen.getByText('Description')).toBeInTheDocument(); + expect(screen.getByText('Priority')).toBeInTheDocument(); + }); + + it('handles disabled fields', () => { + render( + , + {organization: org} + ); + + expect(screen.getByRole('textbox', {name: /title/i})).toBeDisabled(); + }); + }); + + describe('submission', () => { + it('submit button calls onSubmit with all field values', async () => { + render( + , + {organization: org} + ); + + await userEvent.click(screen.getByRole('button', {name: 'Create'})); + + await waitFor(() => { + expect(onSubmit).toHaveBeenCalledWith( + expect.objectContaining({title: 'My Issue', priority: 'high'}) + ); + }); + }); + + it('blocks submission when required fields are empty', async () => { + render( + , + {organization: org} + ); + + await userEvent.click(screen.getByRole('button', {name: 'Create'})); + + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it('shows error toast on submit failure', async () => { + const error = new RequestError('POST', '/api/test/', new Error('Bad Request')); + error.responseJSON = {detail: 'Something went wrong'}; + const failingSubmit = jest.fn().mockRejectedValue(error); + + render( + , + {organization: org} + ); + + await userEvent.click(screen.getByRole('button', {name: 'Create'})); + + await waitFor(() => { + expect(addErrorMessage).toHaveBeenCalledWith('Something went wrong'); + }); + }); + + it('renders blank fields as nothing but disables submit', () => { + render( + , + {organization: org} + ); + + expect(screen.getByRole('button', {name: 'Create'})).toBeDisabled(); + }); + + it('respects submitDisabled prop', () => { + render( + , + {organization: org} + ); + + expect(screen.getByRole('button', {name: 'Create'})).toBeDisabled(); + }); + + it('shows loading indicator when isLoading', () => { + render( + , + {organization: org} + ); + + expect(screen.getByTestId('loading-indicator')).toBeInTheDocument(); + expect(screen.queryByRole('textbox', {name: /title/i})).not.toBeInTheDocument(); + }); + + it('supports initialValues override', () => { + render( + , + {organization: org} + ); + + expect(screen.getByRole('textbox', {name: /title/i})).toHaveValue( + 'Overridden Title' + ); + }); + + it('renders footer with SubmitButton when footer prop provided', () => { + render( + ( +
+ Custom Submit +
+ )} + />, + {organization: org} + ); + + expect(screen.getByTestId('custom-footer')).toBeInTheDocument(); + expect(screen.getByRole('button', {name: 'Custom Submit'})).toBeInTheDocument(); + }); + }); + + describe('select variants', () => { + it('renders multi-select field and allows multiple selections', async () => { + render( + , + {organization: org} + ); + + await selectEvent.select(screen.getByRole('textbox', {name: 'Labels'}), 'bug'); + await selectEvent.select(screen.getByRole('textbox', {name: 'Labels'}), 'feature'); + + await userEvent.click(screen.getByRole('button', {name: 'Create'})); + + await waitFor(() => { + expect(onSubmit).toHaveBeenCalledWith( + expect.objectContaining({labels: ['bug', 'feature']}) + ); + }); + }); + + it('renders async select with static choices before search', async () => { + render( + , + {organization: org} + ); + + // Open the select — static choices should be available + await userEvent.click(screen.getByRole('textbox', {name: 'Repository'})); + expect(await screen.findByText('My Repo')).toBeInTheDocument(); + expect(screen.getByText('Other Repo')).toBeInTheDocument(); + }); + + it('async select fetches from URL on search', async () => { + // Catch-all for unmatched queries + MockApiClient.addMockResponse({ + url: '/search', + body: [], + }); + const searchResponse = MockApiClient.addMockResponse({ + url: '/search', + match: [MockApiClient.matchQuery({field: 'repo', query: 'test'})], + body: [{value: 'my-org/test-repo', label: 'test-repo'}], + }); + + render( + , + {organization: org} + ); + + const textbox = screen.getByRole('textbox', {name: 'Repository'}); + await userEvent.click(textbox); + await userEvent.type(textbox, 'test'); + + await waitFor(() => expect(searchResponse).toHaveBeenCalled()); + expect(await screen.findByText('test-repo')).toBeInTheDocument(); + }); + }); + + describe('dynamic fields', () => { + it('calls onFieldChange when updatesForm field changes', async () => { + const onFieldChange = jest.fn(); + + render( + , + {organization: org} + ); + + await selectEvent.select( + screen.getByRole('textbox', {name: 'Project'}), + 'Project 1' + ); + + expect(onFieldChange).toHaveBeenCalledWith('project', 'proj-1'); + }); + }); + + describe('table adapter', () => { + const tableField = { + name: 'service_table', + type: 'table' as const, + label: 'Services', + columnKeys: ['service', 'key'], + columnLabels: {service: 'Service', key: 'Integration Key'}, + addButtonText: 'Add Service', + confirmDeleteMessage: 'Delete this service?', + }; + + it('renders table with add button', () => { + render( + , + {organization: org} + ); + + expect(screen.getByRole('button', {name: /Add Service/})).toBeInTheDocument(); + }); + + it('can add a row, fill cells, and submit', async () => { + render( + , + {organization: org} + ); + + await userEvent.click(screen.getByRole('button', {name: /Add Service/})); + + const inputs = screen.getAllByRole('textbox'); + await userEvent.type(inputs[0]!, 'PagerDuty'); + await userEvent.type(inputs[1]!, 'abc123'); + + await userEvent.click(screen.getByRole('button', {name: 'Save'})); + + await waitFor(() => { + expect(onSubmit).toHaveBeenCalledWith( + expect.objectContaining({ + service_table: expect.arrayContaining([ + expect.objectContaining({service: 'PagerDuty', key: 'abc123'}), + ]), + }) + ); + }); + }); + + it('can delete a row with confirmation and submit', async () => { + renderGlobalModal(); + + render( + , + {organization: org} + ); + + await userEvent.click(screen.getByRole('button', {name: 'Delete'})); + await userEvent.click(await screen.findByRole('button', {name: 'Confirm'})); + + await userEvent.click(screen.getByRole('button', {name: 'Save'})); + + await waitFor(() => { + expect(onSubmit).toHaveBeenCalledWith( + expect.objectContaining({service_table: []}) + ); + }); + }); + }); + + describe('choice_mapper adapter', () => { + const choiceMapperField = { + name: 'status_mapping', + type: 'choice_mapper' as const, + label: 'Status Mapping', + addButtonText: 'Add Repo', + addDropdown: { + items: [ + {value: 'repo1', label: 'my-org/repo1'}, + {value: 'repo2', label: 'my-org/repo2'}, + ], + }, + columnLabels: {on_resolve: 'When Resolved'}, + mappedColumnLabel: 'Repository', + mappedSelectors: { + on_resolve: { + choices: [ + ['closed', 'Closed'], + ['open', 'Open'], + ] as Array<[string, string]>, + }, + }, + }; + + it('renders with add button', async () => { + render( + , + {organization: org} + ); + + expect(await screen.findByText('Add Repo')).toBeInTheDocument(); + }); + + it('can add a row, fill column, and submit', async () => { + render( + , + {organization: org} + ); + + // Add a repo + await userEvent.click(screen.getByText('Add Repo')); + await userEvent.click(await screen.findByRole('option', {name: 'my-org/repo1'})); + + // Fill the column selector + await userEvent.click(screen.getByText('Select...')); + await userEvent.click(await screen.findByText('Closed')); + + await userEvent.click(screen.getByRole('button', {name: 'Save'})); + + await waitFor(() => { + expect(onSubmit).toHaveBeenCalledWith( + expect.objectContaining({ + status_mapping: {repo1: {on_resolve: 'closed'}}, + }) + ); + }); + }); + }); + + describe('project_mapper adapter', () => { + const projectMapperField = { + name: 'project_mappings', + type: 'project_mapper' as const, + label: 'Project Mappings', + mappedDropdown: { + items: [ + {value: 'proj-1', label: 'Vercel Project 1'}, + {value: 'proj-2', label: 'Vercel Project 2'}, + ] as const, + placeholder: 'Vercel project\u2026', + }, + sentryProjects: [ + {id: 101, slug: 'sentry-frontend', name: 'Sentry Frontend'}, + {id: 102, slug: 'sentry-backend', name: 'Sentry Backend'}, + ] as const, + }; + + it('renders with dropdowns and disabled add button', () => { + render( + , + {organization: org} + ); + + expect(screen.getByText('Vercel project\u2026')).toBeInTheDocument(); + expect(screen.getByRole('button', {name: /Add project/i})).toBeDisabled(); + }); + + it('can add a mapping and submit', async () => { + render( + , + {organization: org} + ); + + // Select external project + await userEvent.click(screen.getByText('Vercel project\u2026')); + await userEvent.click(await screen.findByText('Vercel Project 1')); + + // Select Sentry project (placeholder uses unicode ellipsis, options use slug) + await userEvent.click(screen.getByText('Sentry project\u2026')); + await userEvent.click(await screen.findByText('sentry-frontend')); + + // Add mapping + await userEvent.click(screen.getByRole('button', {name: /Add project/i})); + + // Submit + await userEvent.click(screen.getByRole('button', {name: 'Save'})); + + await waitFor(() => { + expect(onSubmit).toHaveBeenCalledWith( + expect.objectContaining({ + project_mappings: [[101, 'proj-1']], + }) + ); + }); + }); + }); +}); diff --git a/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx index 619468c6b1faa8..88588df986a97c 100644 --- a/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx +++ b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx @@ -1,4 +1,4 @@ -import {useMemo} from 'react'; +import {useMemo, useState, type ReactNode} from 'react'; import {queryOptions} from '@tanstack/react-query'; import {z} from 'zod'; @@ -12,6 +12,9 @@ import {t} from 'sentry/locale'; import type {SelectValue} from 'sentry/types/core'; import {RequestError} from 'sentry/utils/requestError/requestError'; +import {ChoiceMapperDropdown, ChoiceMapperTable} from './choiceMapperAdapter'; +import {ProjectMapperAddRow, ProjectMapperTable} from './projectMapperAdapter'; +import {TableBody, TableHeaderRow} from './tableAdapter'; import type {JsonFormAdapterFieldConfig} from './types'; import {getDefaultForField, transformChoices} from './utils'; @@ -133,6 +136,11 @@ export function BackendJsonSubmitForm({ onFieldChange, footer, }: BackendJsonSubmitFormProps) { + // Labels for choice_mapper rows (maps key to display label) + const [choiceMapperLabels, setChoiceMapperLabels] = useState< + Record> + >({}); + const defaultValues = useMemo( () => computeDefaultValues(fields, initialValues), [fields, initialValues] @@ -365,6 +373,85 @@ export function BackendJsonSubmitForm({ /> ); + case 'table': { + const tableValue = fieldApi.state.value as Array< + Record + >; + return ( + + + + + {}} + disabled={field.disabled} + /> + + ); + } + case 'project_mapper': { + const mapperValue = fieldApi.state.value as Array<[number, string]>; + return ( + + + + + ); + } + case 'choice_mapper': { + const choiceValue = fieldApi.state.value as Record< + string, + Record + >; + const fieldLabels = choiceMapperLabels[field.name] ?? {}; + return ( + + + { + setChoiceMapperLabels(prev => ({ + ...prev, + [field.name]: { + ...prev[field.name], + [key]: label, + }, + })); + }} + onChange={handleChange} + disabled={field.disabled} + /> + + {}} + disabled={field.disabled} + /> + + ); + } default: return null; } From b49688507efc83ab253ebbb0bcb4753433697e61 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Thu, 2 Apr 2026 14:00:40 +0200 Subject: [PATCH 11/16] codeowners --- .github/CODEOWNERS | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index ad6b4a2fd9239f..40b9c719b31a8e 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -824,6 +824,7 @@ tests/sentry/api/endpoints/test_organization_attribute_mappings.py @get /static/app/components/core/ @getsentry/design-engineering /static/app/components/dnd/ @getsentry/design-engineering /static/app/components/pageFilters/ @getsentry/design-engineering +/static/app/components/backendJsonFormAdapter/ @getsentry/design-engineering /static/app/icons/ @getsentry/design-engineering /static/app/stories/ @getsentry/design-engineering /static/app/components/commandPalette/ @getsentry/design-engineering From b548bcb9c9ca2c545213f50c1e3e894d18d4d2c5 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Thu, 2 Apr 2026 14:01:39 +0200 Subject: [PATCH 12/16] =?UTF-8?q?=E2=9C=82=EF=B8=8F=20knip=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- static/app/views/alerts/rules/issue/ruleNode.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/views/alerts/rules/issue/ruleNode.tsx b/static/app/views/alerts/rules/issue/ruleNode.tsx index fcafbd0f5a158a..ae61c64b0cc632 100644 --- a/static/app/views/alerts/rules/issue/ruleNode.tsx +++ b/static/app/views/alerts/rules/issue/ruleNode.tsx @@ -254,7 +254,7 @@ function TextField({ ); } -export type FormField = { +type FormField = { // The rest is configuration for the form field [key: string]: any; // Type of form fields From 43ec621f13b3fb3d54b8532988a3fabc26974b92 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Thu, 2 Apr 2026 15:12:39 +0200 Subject: [PATCH 13/16] fix: guard against invalid response format --- .../backendJsonSubmitForm.spec.tsx | 35 +++++++++++++++++++ .../backendJsonSubmitForm.tsx | 19 ++++++---- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.spec.tsx b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.spec.tsx index dbfdc950b137ac..1a866ad8b211f7 100644 --- a/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.spec.tsx +++ b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.spec.tsx @@ -416,6 +416,41 @@ describe('BackendJsonSubmitForm', () => { await waitFor(() => expect(searchResponse).toHaveBeenCalled()); expect(await screen.findByText('test-repo')).toBeInTheDocument(); }); + + it('async select gracefully handles non-array API response', async () => { + // Simulate backend returning HTML error page or non-array response + MockApiClient.addMockResponse({ + url: '/search', + body: '502 Bad Gateway' as any, + }); + + render( + , + {organization: org} + ); + + const textbox = screen.getByRole('textbox', {name: 'Repository'}); + await userEvent.click(textbox); + await userEvent.type(textbox, '123'); + + // Should not crash — falls back to empty options + await waitFor(() => { + // The select should still be interactive (no crash) + expect(screen.getByRole('textbox', {name: 'Repository'})).toBeInTheDocument(); + }); + }); }); describe('dynamic fields', () => { diff --git a/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx index 88588df986a97c..33a4d50f95836a 100644 --- a/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx +++ b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx @@ -263,13 +263,18 @@ export function BackendJsonSubmitForm({ if (!debouncedInput) { return staticOptions; } - return API_CLIENT.requestPromise(field.url!, { - query: buildAsyncSelectQuery( - field.name, - debouncedInput, - dynamicFieldValues - ), - }); + const response = await API_CLIENT.requestPromise( + field.url!, + { + query: buildAsyncSelectQuery( + field.name, + debouncedInput, + dynamicFieldValues + ), + } + ); + // API may return non-array responses (e.g. error objects) + return Array.isArray(response) ? response : []; }, }); if (field.multiple) { From 38031c4c493d9852f498b16d8939292cf3d79ae8 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Fri, 3 Apr 2026 09:59:22 +0200 Subject: [PATCH 14/16] ref(externalIssueForm): replace submitSpan state with local span variable --- .../app/components/externalIssues/externalIssueForm.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/static/app/components/externalIssues/externalIssueForm.tsx b/static/app/components/externalIssues/externalIssueForm.tsx index 77bb9f35efd611..e28dc1aa7741c4 100644 --- a/static/app/components/externalIssues/externalIssueForm.tsx +++ b/static/app/components/externalIssues/externalIssueForm.tsx @@ -129,7 +129,6 @@ export function ExternalIssueForm({ const [hasTrackedLoad, setHasTrackedLoad] = useState(false); const [loadSpan, setLoadSpan] = useState(null); - const [submitSpan, setSubmitSpan] = useState(null); const [action, setAction] = useState('create'); const [isDynamicallyRefetching, setIsDynamicallyRefetching] = useState(false); @@ -264,7 +263,7 @@ export function ExternalIssueForm({ const handleSubmit = useCallback( async (values: Record) => { - setSubmitSpan(startSpan('submit')); + const span = startSpan('submit'); try { const data: IntegrationExternalIssue = await api.requestPromise(endpointString, { method: action === 'create' ? 'POST' : 'PUT', @@ -279,10 +278,10 @@ export function ExternalIssueForm({ addSuccessMessage(MESSAGES_BY_ACTION[action]); onChange(); closeModal(); - submitSpan?.end(); + span?.end(); return data; } catch (err) { - submitSpan?.end(); + span?.end(); throw err; } }, @@ -293,7 +292,6 @@ export function ExternalIssueForm({ organization, group, integration, - submitSpan, startSpan, closeModal, onChange, From bcca7b6235072500061d645dea5d612e84e40883 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Fri, 3 Apr 2026 11:33:47 +0200 Subject: [PATCH 15/16] ref(ticketRuleModal): improve handling of async options and caching logic --- .../backendJsonSubmitForm.tsx | 24 +++- .../externalIssues/ticketRuleModal.spec.tsx | 120 ++++++++++++++++++ .../externalIssues/ticketRuleModal.tsx | 78 +++++++++++- 3 files changed, 213 insertions(+), 9 deletions(-) diff --git a/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx index 33a4d50f95836a..533463bd081539 100644 --- a/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx +++ b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx @@ -1,4 +1,4 @@ -import {useMemo, useState, type ReactNode} from 'react'; +import {useMemo, useRef, useState, type ReactNode, useEffect} from 'react'; import {queryOptions} from '@tanstack/react-query'; import {z} from 'zod'; @@ -54,6 +54,14 @@ interface BackendJsonSubmitFormProps { * Whether the form is in a loading state (e.g., dynamic field refetch in progress). */ isLoading?: boolean; + /** + * Called when async select options are fetched for a field. Use this to + * track fetched choices externally (e.g., for persisting them on submit). + */ + onAsyncOptionsFetched?: ( + fieldName: string, + options: Array> + ) => void; /** * Called when a field with `updatesForm: true` changes value. */ @@ -133,9 +141,16 @@ export function BackendJsonSubmitForm({ initialValues, isLoading, dynamicFieldValues, + onAsyncOptionsFetched, onFieldChange, footer, }: BackendJsonSubmitFormProps) { + // Ref to avoid including the callback in queryKey (would cause refetches) + const onAsyncOptionsFetchedRef = useRef(onAsyncOptionsFetched); + useEffect(() => { + onAsyncOptionsFetchedRef.current = onAsyncOptionsFetched; + }); + // Labels for choice_mapper rows (maps key to display label) const [choiceMapperLabels, setChoiceMapperLabels] = useState< Record> @@ -256,6 +271,7 @@ export function BackendJsonSubmitForm({ field.url, debouncedInput, dynamicFieldValues, + JSON.stringify(onAsyncOptionsFetchedRef), ], queryFn: async (): Promise< Array> @@ -274,7 +290,11 @@ export function BackendJsonSubmitForm({ } ); // API may return non-array responses (e.g. error objects) - return Array.isArray(response) ? response : []; + const results = Array.isArray(response) ? response : []; + if (results.length > 0) { + onAsyncOptionsFetchedRef.current?.(field.name, results); + } + return results; }, }); if (field.multiple) { diff --git a/static/app/components/externalIssues/ticketRuleModal.spec.tsx b/static/app/components/externalIssues/ticketRuleModal.spec.tsx index ed4ed3d7f8a8a9..4ae25782e1233b 100644 --- a/static/app/components/externalIssues/ticketRuleModal.spec.tsx +++ b/static/app/components/externalIssues/ticketRuleModal.spec.tsx @@ -279,6 +279,56 @@ describe('ProjectAlerts -> TicketRuleModal', () => { await submitSuccess(); }); + it('should persist async select saved value when modal is reopened', async () => { + // Simulate reopening with a previously saved async field value. + // instance.dynamic_form_fields contains the saved field config with + // choices from the previous search. The backend returns empty choices + // for async fields, but the saved choices should be restored. + const reporterField: IssueConfigField = { + label: 'Reporter', + required: false, + url: 'http://example.com', + type: 'select', + name: 'reporter', + choices: [], // Backend returns empty choices for async fields + }; + + addMockConfigsAPICall(reporterField); + + render( + , + {organization} + ); + // Wait for loading to finish and verify the saved value is displayed + expect(await screen.findByText('Joe Smith')).toBeInTheDocument(); + + // Submit should include the saved value + await submitSuccess(); + const formData = onSubmitAction.mock.calls[0][0]; + expect(formData.reporter).toBe('saved-user-id'); + }); + it('should get async options from URL', async () => { await renderTicketRuleModal(); @@ -339,6 +389,76 @@ describe('ProjectAlerts -> TicketRuleModal', () => { await selectEvent.select(menu, 'Joe'); await submitSuccess(); + + // The fieldOptionsCache (2nd arg) should include the search result + // so the parent can persist it in dynamic_form_fields + const fieldOptionsCache = onSubmitAction.mock.calls[0][1]; + expect(fieldOptionsCache).toHaveProperty('assignee'); + }); + + it('should persist async search choices in fieldOptionsCache on submit', async () => { + await renderTicketRuleModal(); + + const dynamicQuery = MockApiClient.addMockResponse({ + url: '/organizations/org-slug/integrations/1/', + match: [ + MockApiClient.matchQuery({ + action: 'create', + issuetype: issueTypeCode, + project: '10000', + }), + ], + method: 'GET', + body: { + createIssueConfig: [ + ...defaultIssueConfig, + { + label: 'Assignee', + required: true, + url: 'http://example.com', + type: 'select', + name: 'assignee', + }, + ], + }, + }); + + await selectEvent.select( + screen.getByRole('textbox', {name: 'Issue Type'}), + issueTypeLabel + ); + + MockApiClient.addMockResponse({ + url: 'http://example.com', + method: 'GET', + body: [], + }); + MockApiClient.addMockResponse({ + url: 'http://example.com', + match: [MockApiClient.matchQuery({field: 'assignee', query: 'Joe'})], + method: 'GET', + body: [{label: 'Joe', value: 'Joe'}], + }); + + expect(dynamicQuery).toHaveBeenCalled(); + const menu = screen.getByRole('textbox', {name: 'Assignee'}); + await userEvent.click(menu); + await userEvent.type(menu, 'Joe'); + await waitFor(() => + expect(screen.getAllByText('Joe').length).toBeGreaterThanOrEqual(1) + ); + await selectEvent.select(menu, 'Joe'); + + await submitSuccess(); + + // The second argument to onSubmitAction is the fieldOptionsCache. + // It should include the async search result so the choice persists + // when the modal is re-opened. + const fieldOptionsCache = onSubmitAction.mock.calls[0][1]; + expect(fieldOptionsCache).toHaveProperty('assignee'); + expect(fieldOptionsCache.assignee).toEqual( + expect.arrayContaining([['Joe', 'Joe']]) + ); }); }); }); diff --git a/static/app/components/externalIssues/ticketRuleModal.tsx b/static/app/components/externalIssues/ticketRuleModal.tsx index 4590b25248a0c6..55b6396a289550 100644 --- a/static/app/components/externalIssues/ticketRuleModal.tsx +++ b/static/app/components/externalIssues/ticketRuleModal.tsx @@ -16,7 +16,7 @@ import {LoadingError} from 'sentry/components/loadingError'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {t, tct} from 'sentry/locale'; import type {TicketActionData} from 'sentry/types/alerts'; -import type {Choices} from 'sentry/types/core'; +import type {Choices, SelectValue} from 'sentry/types/core'; import type {IntegrationIssueConfig, IssueConfigField} from 'sentry/types/integrations'; import {defined} from 'sentry/utils'; import {parseQueryKey} from 'sentry/utils/api/apiQueryKey'; @@ -90,6 +90,25 @@ export function TicketRuleModal({ const [isDynamicallyRefetching, setIsDynamicallyRefetching] = useState(false); + // Track async select options fetched via search so they can be persisted + // in the rule config when the form is submitted. + const [asyncOptionsCache, setAsyncOptionsCache] = useState>({}); + const handleAsyncOptionsFetched = useCallback( + (fieldName: string, options: Array>) => { + setAsyncOptionsCache(prev => ({ + ...prev, + [fieldName]: options.map( + o => + [o.value, typeof o.label === 'string' ? o.label : String(o.value)] as [ + string | number, + string, + ] + ), + })); + }, + [] + ); + const {url: endpointString} = parseQueryKey( makeIntegrationIssueConfigTicketRuleQueryKey({ orgSlug: organization.slug, @@ -228,7 +247,9 @@ export function TicketRuleModal({ const handleSubmit = useCallback( (values: Record) => { - // Build a cache of field choices for async fields so the parent can persist them + // Build a cache of field choices so the parent can persist them. + // Start with static choices from the config, then overlay search results + // captured via onAsyncOptionsFetched. const fieldOptionsCache: Record = {}; const config = integrationDetails?.[getConfigName(action)] || []; for (const field of config) { @@ -236,12 +257,16 @@ export function TicketRuleModal({ fieldOptionsCache[field.name] = field.choices as Choices; } } + // Merge async search results (overwrites static choices for the same field) + for (const [fieldName, choices] of Object.entries(asyncOptionsCache)) { + fieldOptionsCache[fieldName] = choices; + } onSubmitAction(cleanData(values) as Record, fieldOptionsCache); addSuccessMessage(t('Changes applied.')); closeModal(); }, - [cleanData, onSubmitAction, closeModal, integrationDetails] + [cleanData, onSubmitAction, closeModal, integrationDetails, asyncOptionsCache] ); const [lastChangedField, setLastChangedField] = useState>({}); @@ -283,6 +308,23 @@ export function TicketRuleModal({ }, ]; + // Build a map of saved choices from instance.dynamic_form_fields so we can + // restore async select options that were previously fetched via search. + const savedFields = Object.values(instance?.dynamic_form_fields || {}); + const savedChoicesMap = new Map( + savedFields + .filter( + (f): f is IssueConfigField => + typeof f === 'object' && + f !== null && + 'url' in f && + 'choices' in f && + Array.isArray(f.choices) && + f.choices.length > 0 + ) + .map(f => [f.name, f.choices as Choices]) + ); + const configFields = (integrationDetails?.[getConfigName(action)] || []) as JsonFormAdapterFieldConfig[]; @@ -304,13 +346,29 @@ export function TicketRuleModal({ let shouldDefaultChoice = true; if (field.type === 'select' || field.type === 'choice') { - const choices = field.choices || []; - shouldDefaultChoice = !!(Array.isArray(prevChoice) - ? prevChoice.every(value => choices.some(tuple => tuple[0] === value)) - : choices.some(item => item[0] === prevChoice)); + // For async select fields, always trust the saved value — choices + // are fetched dynamically and won't be in the static choices list. + if ('url' in field && field.url) { + shouldDefaultChoice = true; + } else { + const choices = field.choices || []; + shouldDefaultChoice = !!(Array.isArray(prevChoice) + ? prevChoice.every(value => choices.some(tuple => tuple[0] === value)) + : choices.some(item => item[0] === prevChoice)); + } } if (shouldDefaultChoice) { + // For async fields, also restore saved choices so the select can + // display the label for the saved value. + const savedChoices = savedChoicesMap.get(field.name); + if (savedChoices && 'url' in field && field.url) { + return { + ...field, + default: prevChoice, + choices: savedChoices as Array<[string, string]>, + }; + } return {...field, default: prevChoice}; } @@ -327,6 +385,11 @@ export function TicketRuleModal({ field.default && !(Array.isArray(field.default) && !field.default.length) ) { + // Skip validation for async select fields — their choices are + // fetched dynamically and won't contain the saved value until searched. + if ('url' in field && field.url) { + continue; + } const fieldChoices = (field.choices || []) as Choices; const found = fieldChoices.find(([value, _]) => Array.isArray(field.default) @@ -397,6 +460,7 @@ export function TicketRuleModal({ submitDisabled={hasFormErrors} isLoading={isDynamicallyRefetching} dynamicFieldValues={dynamicFieldValues} + onAsyncOptionsFetched={handleAsyncOptionsFetched} onFieldChange={onFieldChange} footer={({SubmitButton, disabled}) => (