diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 40f02ebc420fd4..218090ebba86c1 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -828,6 +828,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 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( - -): 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 {getDefaultForField, getZodType, transformChoices} from './utils'; interface BackendJsonFormAdapterProps< TField extends JsonFormAdapterFieldConfig, @@ -87,7 +23,7 @@ interface BackendJsonFormAdapterProps< initialValue?: FieldValue; } -export function BackendJsonFormAdapter< +export function BackendJsonAutoSaveForm< TField extends JsonFormAdapterFieldConfig, TData, TContext, @@ -104,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 ( @@ -304,6 +240,8 @@ export function BackendJsonFormAdapter< /> ); + case 'blank': + return null; default: unreachable(field); return null; diff --git a/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.spec.tsx b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.spec.tsx new file mode 100644 index 00000000000000..1a866ad8b211f7 --- /dev/null +++ b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.spec.tsx @@ -0,0 +1,705 @@ +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(); + }); + + 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', () => { + 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 new file mode 100644 index 00000000000000..533463bd081539 --- /dev/null +++ b/static/app/components/backendJsonFormAdapter/backendJsonSubmitForm.tsx @@ -0,0 +1,491 @@ +import {useMemo, useRef, useState, type ReactNode, useEffect} from 'react'; +import {queryOptions} from '@tanstack/react-query'; +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 {ChoiceMapperDropdown, ChoiceMapperTable} from './choiceMapperAdapter'; +import {ProjectMapperAddRow, ProjectMapperTable} from './projectMapperAdapter'; +import {TableBody, TableHeaderRow} from './tableAdapter'; +import type {JsonFormAdapterFieldConfig} from './types'; +import {getDefaultForField, 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 | void; + /** + * 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; + /** + * 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). + */ + 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. + */ + 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; +} + +/** + * 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 +): Record { + const defaults: Record = {}; + for (const field of fields) { + if (field.name && field.type !== 'blank') { + defaults[field.name] = + initialValues?.[field.name] ?? field.default ?? getDefaultForField(field); + } + } + 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, + 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> + >({}); + + const defaultValues = useMemo( + () => computeDefaultValues(fields, initialValues), + [fields, initialValues] + ); + + const validationSchema = useMemo(() => buildValidationSchema(fields), [fields]); + + const form = useScrapsForm({ + ...defaultFormOptions, + defaultValues, + validators: { + onSubmit: validationSchema, + }, + onSubmit: async ({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')); + } + } + }, + }); + + const hasErrors = fields.some( + field => field.name === 'error' && field.type === 'blank' + ); + + const buttonDisabled = hasErrors || !!submitDisabled || !!isLoading; + + const submitButton = footer ? ( + footer({SubmitButton: form.SubmitButton, disabled: buttonDisabled}) + ) : ( + {submitLabel} + ); + + return ( + + {isLoading && } + {!isLoading && ( + + {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, + JSON.stringify(onAsyncOptionsFetchedRef), + ], + queryFn: async (): Promise< + Array> + > => { + if (!debouncedInput) { + return staticOptions; + } + const response = await API_CLIENT.requestPromise( + field.url!, + { + query: buildAsyncSelectQuery( + field.name, + debouncedInput, + dynamicFieldValues + ), + } + ); + // API may return non-array responses (e.g. error objects) + const results = Array.isArray(response) ? response : []; + if (results.length > 0) { + onAsyncOptionsFetchedRef.current?.(field.name, results); + } + return results; + }, + }); + if (field.multiple) { + return ( + + + + ); + } + return ( + + + + ); + } + if (field.multiple) { + return ( + + + + ); + } + return ( + + + + ); + case 'secret': + return ( + + + + ); + case 'string': + case 'text': + case 'url': + case 'email': + return ( + + + + ); + 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; + } + }} + + ))} + + )} + {submitButton} + + ); +} 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( - ; + /** + * 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..d34a6e7629e1ac --- /dev/null +++ b/static/app/components/backendJsonFormAdapter/utils.ts @@ -0,0 +1,74 @@ +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 getDefaultForField(field: JsonFormAdapterFieldConfig): unknown { + switch (field.type) { + 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 field.multiple ? [] : null; + case 'blank': + return null; + default: + unreachable(field); + 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 509b1468287eac..652aa7c3e39a80 100644 --- a/static/app/components/externalIssues/externalIssueForm.spec.tsx +++ b/static/app/components/externalIssues/externalIssueForm.spec.tsx @@ -2,9 +2,19 @@ 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 {addErrorMessage} from 'sentry/actionCreators/indicator'; import {ExternalIssueForm} from 'sentry/components/externalIssues/externalIssueForm'; + +jest.mock('sentry/actionCreators/indicator'); import { makeClosableHeader, makeCloseButton, @@ -12,25 +22,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: []}); @@ -75,7 +66,6 @@ describe('ExternalIssueForm', () => { }; describe('create', () => { - // TODO: expand tests beforeEach(() => { formConfig = { createIssueConfig: [], @@ -88,6 +78,142 @@ describe('ExternalIssueForm', () => { it('renders', async () => { 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: [ + { + 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: [ @@ -107,6 +233,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}/`, @@ -199,40 +400,115 @@ describe('ExternalIssueForm', () => { 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(); - // 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(); + }); + + 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', () => { @@ -308,25 +584,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', @@ -340,13 +623,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..e28dc1aa7741c4 100644 --- a/static/app/components/externalIssues/externalIssueForm.tsx +++ b/static/app/components/externalIssues/externalIssueForm.tsx @@ -4,23 +4,16 @@ 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'; 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/backendJsonSubmitForm'; +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 +23,6 @@ import type { Integration, IntegrationExternalIssue, IntegrationIssueConfig, - IssueConfigField, } from 'sentry/types/integrations'; import type {Organization} from 'sentry/types/organization'; import {trackAnalytics} from 'sentry/utils/analytics'; @@ -85,7 +77,7 @@ const SUBMIT_LABEL_BY_ACTION = { interface ExternalIssueFormProps extends ModalRenderProps { group: Group; integration: Integration; - onChange: (onSuccess?: () => void, onError?: () => void) => void; + onChange: () => void; } function makeIntegrationIssueConfigQueryKey({ @@ -121,9 +113,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({ @@ -137,9 +129,7 @@ export function ExternalIssueForm({ const [hasTrackedLoad, setHasTrackedLoad] = useState(false); 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,90 +261,85 @@ 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) => { + const span = 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', + }); + addSuccessMessage(MESSAGES_BY_ACTION[action]); + onChange(); + closeModal(); + span?.end(); + return data; + } catch (err) { + span?.end(); + throw err; + } }, - [organization, group, integration, submitSpan, action, closeModal, onChange] + [ + api, + endpointString, + action, + organization, + group, + integration, + startSpan, + closeModal, + onChange, + ] ); + // 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: FieldValue) => { + (fieldName: string, value: unknown) => { if (dynamicFieldValues.hasOwnProperty(fieldName)) { + setLastChangedField({[fieldName]: value}); 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]); + // 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 (
-

{title}

+ {title}
@@ -372,7 +357,7 @@ export function ExternalIssueForm({ return (
-

{title}

+ {title}
@@ -382,38 +367,39 @@ 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/ticketRuleModal.spec.tsx b/static/app/components/externalIssues/ticketRuleModal.spec.tsx index 961f2a2db5ba26..4ae25782e1233b 100644 --- a/static/app/components/externalIssues/ticketRuleModal.spec.tsx +++ b/static/app/components/externalIssues/ticketRuleModal.spec.tsx @@ -1,6 +1,6 @@ import {OrganizationFixture} from 'sentry-fixture/organization'; -import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; +import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; import {selectEvent} from 'sentry-test/selectEvent'; import {addSuccessMessage} from 'sentry/actionCreators/indicator'; @@ -122,16 +122,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'); @@ -289,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(); @@ -321,23 +361,104 @@ 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(); + + // 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 aa715119c1bcfc..55b6396a289550 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,25 +7,16 @@ 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/backendJsonSubmitForm'; +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'; 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'; @@ -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,17 +88,27 @@ 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); + // 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, @@ -225,7 +226,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 +245,99 @@ 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 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) { + if (field.url && field.choices) { + 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, cache, onSubmitAction, closeModal] + [cleanData, onSubmitAction, closeModal, integrationDetails, asyncOptionsCache] ); + 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, - }) + // 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[]; + + 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 +345,60 @@ export function TicketRuleModal({ let shouldDefaultChoice = true; - if (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)); + if (field.type === 'select' || field.type === 'choice') { + // 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) { - field.default = prevChoice; + // 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}; } 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) ) { + // 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) - ? 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 +407,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 +432,11 @@ export function TicketRuleModal({ } return ( - +
+

{title}

+
+ {link ? tct( @@ -445,9 +448,28 @@ export function TicketRuleModal({ {ticketType} )} - } - getFieldProps={getTicketRuleFieldProps} - /> + {Object.entries(formErrors).map(([name, errorNode]) => ( + {errorNode} + ))} + ( +
+ {t('Apply Changes')} +
+ )} + /> + + ); } 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'); -} diff --git a/static/app/components/pluginConfig.spec.tsx b/static/app/components/pluginConfig.spec.tsx index 7fddde5aa09137..7a4e0960b2b909 100644 --- a/static/app/components/pluginConfig.spec.tsx +++ b/static/app/components/pluginConfig.spec.tsx @@ -3,8 +3,6 @@ import {WebhookPluginConfigFixture} from 'sentry-fixture/integrationListDirector import {initializeOrg} from 'sentry-test/initializeOrg'; import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; -import {plugins} from 'sentry/plugins'; - import {PluginConfig} from './pluginConfig'; describe('PluginConfig', () => { @@ -16,7 +14,21 @@ describe('PluginConfig', () => { MockApiClient.addMockResponse({ url: `/projects/${organization.slug}/${project.slug}/plugins/${webhookPlugin.id}/`, method: 'GET', - body: webhookPlugin, + body: { + ...webhookPlugin, + config: [ + { + name: 'urls', + label: 'Callback URLs', + type: 'textarea', + placeholder: 'https://sentry.io/callback/url', + required: false, + help: 'Enter callback URLs, separated by newlines.', + value: 'https://example.com/hook', + defaultValue: '', + }, + ], + }, }); const testWebhookMock = MockApiClient.addMockResponse({ url: `/projects/${organization.slug}/${project.slug}/plugins/${webhookPlugin.id}/`, @@ -24,11 +36,9 @@ describe('PluginConfig', () => { body: {detail: 'No errors returned'}, }); - expect(plugins.isLoaded(webhookPlugin)).toBe(false); render(); - expect(plugins.isLoaded(webhookPlugin)).toBe(true); - await userEvent.click(screen.getByRole('button', {name: 'Test Plugin'})); + await userEvent.click(await screen.findByRole('button', {name: 'Test Plugin'})); expect(await screen.findByText('"No errors returned"')).toBeInTheDocument(); expect(testWebhookMock).toHaveBeenCalledWith( @@ -39,4 +49,53 @@ describe('PluginConfig', () => { }) ); }); + + it('renders config fields from backend', async () => { + const webhookPlugin = WebhookPluginConfigFixture({enabled: true}); + + MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${project.slug}/plugins/${webhookPlugin.id}/`, + method: 'GET', + body: { + ...webhookPlugin, + config: [ + { + name: 'urls', + label: 'Callback URLs', + type: 'textarea', + placeholder: 'https://sentry.io/callback/url', + required: false, + help: 'Enter callback URLs, separated by newlines.', + value: '', + defaultValue: '', + }, + ], + }, + }); + + render(); + + expect(await screen.findByText('Callback URLs')).toBeInTheDocument(); + }); + + it('renders auth error state', async () => { + const webhookPlugin = WebhookPluginConfigFixture({enabled: true}); + + MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${project.slug}/plugins/${webhookPlugin.id}/`, + method: 'GET', + body: { + ...webhookPlugin, + config_error: 'You need to associate an identity', + auth_url: '/auth/associate/webhooks/', + }, + }); + + render(); + + expect( + await screen.findByText('You need to associate an identity') + ).toBeInTheDocument(); + expect(screen.getByRole('button', {name: 'Associate Identity'})).toBeInTheDocument(); + }); }); diff --git a/static/app/components/pluginConfig.tsx b/static/app/components/pluginConfig.tsx index 2bc6994e10801e..08cddbde4623a6 100644 --- a/static/app/components/pluginConfig.tsx +++ b/static/app/components/pluginConfig.tsx @@ -1,7 +1,8 @@ -import {useEffect, useRef, useState} from 'react'; +import {useMemo, useRef, useState} from 'react'; import styled from '@emotion/styled'; -import {Button} from '@sentry/scraps/button'; +import {Alert} from '@sentry/scraps/alert'; +import {Button, LinkButton} from '@sentry/scraps/button'; import {Flex, Grid} from '@sentry/scraps/layout'; import { @@ -10,19 +11,95 @@ import { addSuccessMessage, } from 'sentry/actionCreators/indicator'; import {hasEveryAccess} from 'sentry/components/acl/access'; +import {BackendJsonSubmitForm} from 'sentry/components/backendJsonFormAdapter/backendJsonSubmitForm'; +import type {JsonFormAdapterFieldConfig} from 'sentry/components/backendJsonFormAdapter/types'; +import {LoadingError} from 'sentry/components/loadingError'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {Panel} from 'sentry/components/panels/panel'; import {PanelAlert} from 'sentry/components/panels/panelAlert'; import {PanelBody} from 'sentry/components/panels/panelBody'; import {PanelHeader} from 'sentry/components/panels/panelHeader'; import {t} from 'sentry/locale'; -import {plugins} from 'sentry/plugins'; import {PluginIcon} from 'sentry/plugins/components/pluginIcon'; import type {Plugin} from 'sentry/types/integrations'; import type {Project} from 'sentry/types/project'; -import {useApi} from 'sentry/utils/useApi'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; +import {trackIntegrationAnalytics} from 'sentry/utils/integrationUtil'; +import {fetchMutation, useApiQuery} from 'sentry/utils/queryClient'; import {useOrganization} from 'sentry/utils/useOrganization'; +/** + * Field config shape returned by the backend's PluginWithConfigSerializer. + */ +interface BackendPluginField { + label: string; + name: string; + type: string; + choices?: Array<[string, string]>; + default?: unknown; + defaultValue?: unknown; + hasSavedValue?: boolean; + help?: string; + isDeprecated?: boolean; + isHidden?: boolean; + placeholder?: string; + prefix?: string; + readonly?: boolean; + required?: boolean; + value?: unknown; +} + +interface PluginWithConfig extends Plugin { + auth_url?: string; + config?: BackendPluginField[]; + config_error?: string; +} + +/** + * Maps a backend plugin field to the JsonFormAdapterFieldConfig shape + * expected by BackendJsonSubmitForm. + */ +function mapPluginField(field: BackendPluginField): JsonFormAdapterFieldConfig { + const base = { + name: field.name, + label: field.label, + required: field.required, + help: field.help ?? undefined, + placeholder: field.placeholder ?? undefined, + disabled: field.readonly, + default: field.defaultValue ?? field.default, + }; + + // Backend uses 'bool', adapter uses 'boolean' + const type = field.type === 'bool' ? 'boolean' : field.type; + + switch (type) { + case 'boolean': + return {...base, type: 'boolean'} as JsonFormAdapterFieldConfig; + case 'select': + case 'choice': + return { + ...base, + type: type as 'select' | 'choice', + choices: field.choices, + } as JsonFormAdapterFieldConfig; + case 'secret': + return {...base, type: 'secret'} as JsonFormAdapterFieldConfig; + case 'textarea': + return {...base, type: 'textarea'} as JsonFormAdapterFieldConfig; + case 'number': + return {...base, type: 'number'} as JsonFormAdapterFieldConfig; + case 'email': + return {...base, type: 'email'} as JsonFormAdapterFieldConfig; + case 'url': + return {...base, type: 'url'} as JsonFormAdapterFieldConfig; + case 'string': + case 'text': + default: + return {...base, type: 'text'} as JsonFormAdapterFieldConfig; + } +} + interface PluginConfigProps { plugin: Plugin; project: Project; @@ -36,42 +113,66 @@ export function PluginConfig({ enabled, onDisablePlugin, }: PluginConfigProps) { - const api = useApi(); const organization = useOrganization(); - // If passed via props, use that value instead of from `data` const isEnabled = typeof enabled === 'undefined' ? plugin.enabled : enabled; const hasWriteAccess = hasEveryAccess(['project:write'], {organization, project}); const [testResults, setTestResults] = useState(''); - const [isPluginLoading, setIsPluginLoading] = useState(!plugins.isLoaded(plugin)); - const loadingPluginIdRef = useRef(null); - - useEffect(() => { - // Avoid loading the same plugin multiple times - if (!plugins.isLoaded(plugin) && loadingPluginIdRef.current !== plugin.id) { - setIsPluginLoading(true); - loadingPluginIdRef.current = plugin.id; - plugins.load(plugin, () => { - setIsPluginLoading(false); - }); + + const pluginEndpoint = getApiUrl( + '/projects/$organizationIdOrSlug/$projectIdOrSlug/plugins/$pluginId/', + { + path: { + organizationIdOrSlug: organization.slug, + projectIdOrSlug: project.slug, + pluginId: plugin.id, + }, + } + ); + + const { + data: pluginData, + isPending, + isError, + refetch, + } = useApiQuery([pluginEndpoint], { + staleTime: 0, + }); + + const wasConfiguredRef = useRef(false); + + const {fields, initialValues} = useMemo(() => { + if (!pluginData?.config) { + return {fields: [], initialValues: {}}; } - }, [plugin]); + + let configured = false; + const vals: Record = {}; + const mapped: JsonFormAdapterFieldConfig[] = []; + + for (const field of pluginData.config) { + if (field.value) { + configured = true; + } + vals[field.name] = field.value ?? field.defaultValue ?? ''; + mapped.push(mapPluginField(field)); + } + + wasConfiguredRef.current = configured; + return {fields: mapped, initialValues: vals}; + }, [pluginData]); const handleTestPlugin = async () => { setTestResults(''); addLoadingMessage(t('Sending test...')); try { - const pluginEndpointData = await api.requestPromise( - `/projects/${organization.slug}/${project.slug}/plugins/${plugin.id}/`, - { - method: 'POST', - data: { - test: true, - }, - } - ); + const response = (await fetchMutation({ + method: 'POST', + url: pluginEndpoint, + data: {test: true}, + })) as {detail: string}; - setTestResults(JSON.stringify(pluginEndpointData.detail)); + setTestResults(JSON.stringify(response.detail)); addSuccessMessage(t('Test Complete!')); } catch (_err) { addErrorMessage( @@ -84,6 +185,86 @@ export function PluginConfig({ onDisablePlugin?.(plugin); }; + const handleSubmit = async (values: Record) => { + if (!wasConfiguredRef.current) { + trackIntegrationAnalytics('integrations.installation_start', { + integration: plugin.id, + integration_type: 'plugin', + view: 'plugin_details', + already_installed: false, + organization, + }); + } + + addLoadingMessage(t('Saving changes\u2026')); + + try { + await fetchMutation({ + method: 'PUT', + url: pluginEndpoint, + data: values, + }); + + addSuccessMessage(t('Success!')); + + trackIntegrationAnalytics('integrations.config_saved', { + integration: plugin.id, + integration_type: 'plugin', + view: 'plugin_details', + already_installed: wasConfiguredRef.current, + organization, + }); + + if (!wasConfiguredRef.current) { + trackIntegrationAnalytics('integrations.installation_complete', { + integration: plugin.id, + integration_type: 'plugin', + view: 'plugin_details', + already_installed: false, + organization, + }); + } + + refetch(); + } catch (_err) { + addErrorMessage(t('Unable to save changes. Please try again.')); + } + }; + + // Auth error state (e.g. OAuth plugins needing identity association) + if (pluginData?.config_error) { + let authUrl = pluginData.auth_url ?? ''; + if (authUrl.includes('?')) { + authUrl += '&next=' + encodeURIComponent(document.location.pathname); + } else { + authUrl += '?next=' + encodeURIComponent(document.location.pathname); + } + return ( + + + + + {plugin.name} + + + +
+ + + {pluginData.config_error} + + + + {t('Associate Identity')} + + + + ); + } + return (
- {isPluginLoading ? ( + {isPending ? ( - ) : ( - plugins.get(plugin).renderSettings({ - organization, - project, - }) - )} + ) : isError ? ( + + ) : fields.length > 0 ? ( + + ) : null} ); 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 diff --git a/static/app/views/settings/organizationIntegrations/configureIntegration.tsx b/static/app/views/settings/organizationIntegrations/configureIntegration.tsx index c9df3969854a88..da7b32afaf40d0 100644 --- a/static/app/views/settings/organizationIntegrations/configureIntegration.tsx +++ b/static/app/views/settings/organizationIntegrations/configureIntegration.tsx @@ -9,7 +9,7 @@ import {TabList, Tabs} from '@sentry/scraps/tabs'; import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator'; import {Access} from 'sentry/components/acl/access'; -import {BackendJsonFormAdapter} from 'sentry/components/backendJsonFormAdapter'; +import {BackendJsonAutoSaveForm} from 'sentry/components/backendJsonFormAdapter/backendJsonAutoSaveForm'; import type {FieldValue} from 'sentry/components/backendJsonFormAdapter/types'; import {Confirm} from 'sentry/components/confirm'; import {List} from 'sentry/components/list'; @@ -410,7 +410,7 @@ function ConfigureIntegration() { } > {integration.configOrganization.map(fieldConfig => ( -