diff --git a/static/app/components/backendJsonFormAdapter/utils.ts b/static/app/components/backendJsonFormAdapter/utils.ts index b60b5a3e1db89c..4a55b7b536f28e 100644 --- a/static/app/components/backendJsonFormAdapter/utils.ts +++ b/static/app/components/backendJsonFormAdapter/utils.ts @@ -20,7 +20,7 @@ export function getZodType(fieldType: JsonFormAdapterFieldConfig['type']) { case 'url': return z.url(); case 'choice_mapper': - return z.object({}); + return z.looseObject({}); case 'project_mapper': case 'table': return z.array(z.any()); diff --git a/static/app/components/core/form/autoSaveForm.mdx b/static/app/components/core/form/autoSaveForm.mdx index 0f7fb55034c17c..bc743135fb167b 100644 --- a/static/app/components/core/form/autoSaveForm.mdx +++ b/static/app/components/core/form/autoSaveForm.mdx @@ -88,6 +88,35 @@ const schema = z.object({ > [!NOTE] > Do NOT use toasts to communicate auto-save status. The built-in inline indicators are the correct feedback mechanism. Toasts are noisy and disruptive for fields that save frequently. +## Transformed Submit Values + +`AutoSaveForm` keeps field state typed as the schema input, but submits the schema's parsed output to the mutation. This matches `useScrapsForm` and lets you use transforms or narrower output types without extra parsing in `mutationFn`. + +> [!WARNING] +> The schema is applied to the form value on submit, so unknown keys are stripped per Zod's default behavior. For map-like fields with arbitrary keys, use `z.record(z.string(), …)` or `z.looseObject({})` — not `z.object({})`, which declares zero keys and will strip everything at submit time. + +```jsx +const schema = z.object({ + highlightContext: z.string().transform(value => JSON.parse(value)), +}); + +}) => + fetchMutation({url: '/project/', method: 'PUT', data}), + }} +> + {field => ( + + + + )} + +``` + ## Confirmation Dialogs For dangerous operations (security settings, permissions), use the `confirm` prop to show a confirmation modal before saving. It accepts a string (always shown) or a function (conditionally shown). diff --git a/static/app/components/core/form/autoSaveForm.spec.tsx b/static/app/components/core/form/autoSaveForm.spec.tsx index 50314e6d64cccc..992f9f856062ea 100644 --- a/static/app/components/core/form/autoSaveForm.spec.tsx +++ b/static/app/components/core/form/autoSaveForm.spec.tsx @@ -12,6 +12,10 @@ const testSchema = z.object({ testField: z.string(), }); +const transformedTestSchema = z.object({ + testField: z.string().transform(value => value.toUpperCase()), +}); + describe('AutoSaveForm', () => { describe('types', () => { it('should have data type flow towards callbacks', () => { @@ -64,7 +68,9 @@ describe('AutoSaveForm', () => { initialValue={serverState} mutationOptions={{ mutationFn: (data: {testField: string}) => { - return Promise.resolve({testField: data.testField.toUpperCase()}); + return Promise.resolve({ + testField: data.testField.toUpperCase(), + }); }, onSuccess: data => { setServerState(data.testField); @@ -95,6 +101,33 @@ describe('AutoSaveForm', () => { expect(input).toHaveValue('HELLO'); }); }); + + it('submits transformed schema values to the mutation', async () => { + const mutationFn = jest.fn((data: {testField: string}) => Promise.resolve(data)); + + render( + + {field => ( + + + + )} + + ); + + const input = screen.getByRole('textbox', {name: 'Name'}); + await userEvent.type(input, 'hello'); + await userEvent.tab(); + + await waitFor(() => { + expect(mutationFn).toHaveBeenCalledWith({testField: 'HELLO'}, expect.anything()); + }); + }); }); describe('error handling', () => { @@ -141,7 +174,9 @@ describe('AutoSaveForm', () => { mutationOptions={{ mutationFn: () => { const error = new RequestError('POST', '/test/', new Error('test')); - error.responseJSON = {testField: ['This value is not allowed']}; + error.responseJSON = { + testField: ['This value is not allowed'], + }; throw error; }, }} diff --git a/static/app/components/core/form/autoSaveForm.tsx b/static/app/components/core/form/autoSaveForm.tsx index 384ed449666355..89828d7fbf9d42 100644 --- a/static/app/components/core/form/autoSaveForm.tsx +++ b/static/app/components/core/form/autoSaveForm.tsx @@ -32,25 +32,28 @@ type ConfirmConfig = | React.ReactNode | ((value: TValue) => React.ReactNode | undefined); -/** Form data type coming from the schema */ +type SchemaInput = z.input; +type SchemaOutput = z.output; + +/** Form data type coming from the schema input */ type SchemaFieldName = Extract< - DeepKeys>, + DeepKeys>, string >; /** FieldApi’s TData must be DeepValue */ -type SchemaFieldValue< +type SchemaFieldInputValue< TSchema extends z.ZodObject, TFieldName extends SchemaFieldName, -> = DeepValue, TFieldName>; +> = DeepValue, TFieldName>; type AutoSaveFormRenderArg< TSchema extends z.ZodObject, TFieldName extends SchemaFieldName, > = FieldApi< - z.infer, + SchemaInput, TFieldName, - SchemaFieldValue, + SchemaFieldInputValue, // Field validators (all can be undefined to satisfy the constraints) undefined, // TOnMount undefined, // TOnChange @@ -106,7 +109,7 @@ interface AutoSaveFormProps< TData, TContext, TSchema extends z.ZodObject, - TFieldName extends Extract, string>, + TFieldName extends Extract, string>, > { /** * Render prop that receives field props and additional props @@ -118,7 +121,7 @@ interface AutoSaveFormProps< /** * Initial value - must match the schema's type for this field */ - initialValue: z.infer[TFieldName]; + initialValue: SchemaInput[TFieldName]; /** * TanStack Query mutation options - mutationFn receives single-field data @@ -126,7 +129,7 @@ interface AutoSaveFormProps< mutationOptions: UseMutationOptions< TData, Error, - NoInfer[TFieldName]>>, + NoInfer[TFieldName]>>, TContext >; @@ -156,7 +159,7 @@ interface AutoSaveFormProps< * // Function with conditional confirmation * confirm={(value) => value === 'dangerous' ? "This is irreversible!" : undefined} */ - confirm?: ConfirmConfig[TFieldName]>; + confirm?: ConfirmConfig[TFieldName]>; } export function AutoSaveForm< @@ -165,7 +168,7 @@ export function AutoSaveForm< // Will be fixed by https://github.com/typescript-eslint/typescript-eslint/pull/12206 // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-arguments TSchema extends z.ZodObject, - TFieldName extends Extract, string>, + TFieldName extends Extract, string>, >(props: AutoSaveFormProps) { const {name, schema, initialValue, mutationOptions, confirm, children} = props; const id = useId(); @@ -178,7 +181,7 @@ export function AutoSaveForm< formId: `${name}-${id}-(auto-save)`, defaultValues: {[name]: initialValue} as Record< TFieldName, - z.infer[TFieldName] + SchemaInput[TFieldName] >, validators: { onChange: schema.pick({[name]: true}) as never, @@ -202,7 +205,9 @@ export function AutoSaveForm< const hasBackendErrors = error instanceof RequestError ? setFieldErrors(formApi, error) : false; if (!hasBackendErrors) { - setFieldErrors(formApi, {[name]: {message: t('Failed to save')}} as never); + setFieldErrors(formApi, { + [name]: {message: t('Failed to save')}, + } as never); } }; @@ -210,6 +215,16 @@ export function AutoSaveForm< formApi.reset(); }; + const parsedValue = schema.pick({[name]: true} as never).safeParse(value); + + if (!parsedValue.success) { + return Promise.resolve(); + } + + const submittedValue = parsedValue.data as Record< + TFieldName, + SchemaOutput[TFieldName] + >; const fieldValue = value[name]; // Determine confirmation message @@ -226,7 +241,7 @@ export function AutoSaveForm< pendingConfirmRef.current = false; // Resolve on both success and failure - error handling is done by // TanStack Query (onError callback, mutation.isError state) - mutation.mutateAsync(value, {onError, onSuccess}).then(() => { + mutation.mutateAsync(submittedValue, {onError, onSuccess}).then(() => { resolve(); }, resolve); }, @@ -246,7 +261,7 @@ export function AutoSaveForm< // Resolve on both success and failure - error handling is done by // TanStack Query (onError callback, mutation.isError state) - return mutation.mutateAsync(value, {onError, onSuccess}).catch(() => {}); + return mutation.mutateAsync(submittedValue, {onError, onSuccess}).catch(() => {}); }, }); diff --git a/static/app/components/events/highlights/highlightsSettingsForm.spec.tsx b/static/app/components/events/highlights/highlightsSettingsForm.spec.tsx index 37f43328e976f3..e6cb2ef08af8b4 100644 --- a/static/app/components/events/highlights/highlightsSettingsForm.spec.tsx +++ b/static/app/components/events/highlights/highlightsSettingsForm.spec.tsx @@ -1,7 +1,7 @@ import {OrganizationFixture} from 'sentry-fixture/organization'; import {DetailedProjectFixture} from 'sentry-fixture/project'; -import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; +import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; import {HighlightsSettingsForm} from 'sentry/components/events/highlights/highlightsSettingsForm'; import * as analytics from 'sentry/utils/analytics'; @@ -53,12 +53,14 @@ describe('HighlightsSettingForm', () => { await userEvent.type(tagInput, `\n${newTag}`); await userEvent.click(screen.getByText('Highlights')); - expect(updateProjectMock).toHaveBeenCalledWith( - url, - expect.objectContaining({ - data: {highlightTags: [...highlightTags, newTag]}, - }) - ); + await waitFor(() => { + expect(updateProjectMock).toHaveBeenCalledWith( + url, + expect.objectContaining({ + data: {highlightTags: [...highlightTags, newTag]}, + }) + ); + }); expect(analyticsSpy).toHaveBeenCalledWith( 'highlights.project_settings.updated_manually', expect.anything() @@ -84,11 +86,36 @@ describe('HighlightsSettingForm', () => { await userEvent.paste(JSON.stringify(newContext)); await userEvent.click(screen.getByText('Highlights')); - expect(updateProjectMock).toHaveBeenCalledWith( + await waitFor(() => { + expect(updateProjectMock).toHaveBeenCalledWith( + url, + expect.objectContaining({ + data: {highlightContext: newContext}, + }) + ); + }); + }); + + it('should reject highlight context values that are valid JSON but not context mappings', async () => { + render(, {organization}); + await screen.findByText('Highlights'); + + const url = `/projects/${organization.slug}/${project.slug}/`; + const updateProjectMock = MockApiClient.addMockResponse({ url, - expect.objectContaining({ - data: {highlightContext: newContext}, - }) - ); + method: 'PUT', + body: {...project, highlightTags, highlightContext}, + }); + + const contextInput = screen.getByRole('textbox', {name: 'Highlighted Context'}); + + await userEvent.clear(contextInput); + await userEvent.paste('123'); + await userEvent.click(screen.getByText('Highlights')); + + await waitFor(() => { + expect(contextInput).toHaveAttribute('aria-invalid', 'true'); + }); + expect(updateProjectMock).not.toHaveBeenCalled(); }); }); diff --git a/static/app/components/events/highlights/highlightsSettingsForm.tsx b/static/app/components/events/highlights/highlightsSettingsForm.tsx index 0349d16befbbe5..94424d068a554a 100644 --- a/static/app/components/events/highlights/highlightsSettingsForm.tsx +++ b/static/app/components/events/highlights/highlightsSettingsForm.tsx @@ -1,116 +1,140 @@ -import {useQueryClient} from '@tanstack/react-query'; +import {mutationOptions} from '@tanstack/react-query'; +import {z} from 'zod'; +import {AutoSaveForm, FieldGroup} from '@sentry/scraps/form'; import {ExternalLink} from '@sentry/scraps/link'; import {addSuccessMessage} from 'sentry/actionCreators/indicator'; import {hasEveryAccess} from 'sentry/components/acl/access'; import {CONTEXT_DOCS_LINK} from 'sentry/components/events/contexts/utils'; -import {Form, type FormProps} from 'sentry/components/forms/form'; -import JsonForm from 'sentry/components/forms/jsonForm'; import {t, tct} from 'sentry/locale'; import type {DetailedProject} from 'sentry/types/project'; import {convertMultilineFieldValue, extractMultilineFields} from 'sentry/utils'; import {trackAnalytics} from 'sentry/utils/analytics'; -import { - makeDetailedProjectQueryKey, - useDetailedProject, -} from 'sentry/utils/project/useDetailedProject'; +import {useDetailedProject} from 'sentry/utils/project/useDetailedProject'; +import {useUpdateProject} from 'sentry/utils/project/useUpdateProject'; import {useOrganization} from 'sentry/utils/useOrganization'; interface HighlightsSettingsFormProps { - projectSlug: any; + projectSlug: string; } +const highlightTagsSchema = z.object({ + highlightTags: z.string(), +}); + +const highlightContextSchema = z.object({ + highlightContext: z.string().transform((value, ctx) => { + if (value === '') { + return {}; + } + try { + return z.record(z.string(), z.array(z.string())).parse(JSON.parse(value)); + } catch { + ctx.addIssue({code: 'custom', message: t('Invalid JSON')}); + return z.NEVER; + } + }), +}); + export function HighlightsSettingsForm({projectSlug}: HighlightsSettingsFormProps) { const organization = useOrganization(); const {data: project} = useDetailedProject({ orgSlug: organization.slug, projectSlug, }); - const queryClient = useQueryClient(); + if (!project) { return null; } - const access = new Set(organization.access.concat(project.access)); - const formProps: FormProps = { - saveOnBlur: true, - allowUndo: true, - initialData: { - highlightTags: project.highlightTags, - highlightContext: project.highlightContext, - }, - apiMethod: 'PUT', - apiEndpoint: `/projects/${organization.slug}/${projectSlug}/`, - onSubmitSuccess: (updatedProject: DetailedProject) => { - queryClient.setQueryData( - makeDetailedProjectQueryKey({ - orgSlug: organization.slug, - projectSlug: project.slug, - }), - prev => - updatedProject ? {headers: prev?.headers ?? {}, json: updatedProject} : prev - ); - trackAnalytics('highlights.project_settings.updated_manually', {organization}); - addSuccessMessage(`Successfully updated highlights for '${project.name}'`); - }, + + return ; +} + +interface LoadedHighlightsSettingsFormProps { + project: DetailedProject; +} + +function LoadedHighlightsSettingsForm({project}: LoadedHighlightsSettingsFormProps) { + const organization = useOrganization(); + const hasAccess = hasEveryAccess(['project:write'], {organization, project}); + const {mutateAsync: updateProject} = useUpdateProject(project); + + const handleSubmitSuccess = () => { + trackAnalytics('highlights.project_settings.updated_manually', {organization}); + addSuccessMessage(`Successfully updated highlights for '${project.name}'`); }; + + const highlightTagsMutationOptions = mutationOptions({ + mutationFn: (data: {highlightTags: string}) => + updateProject({highlightTags: extractMultilineFields(data.highlightTags)}), + onSuccess: handleSubmitSuccess, + }); + + const highlightContextMutationOptions = mutationOptions({ + mutationFn: (data: z.output) => updateProject(data), + onSuccess: handleSubmitSuccess, + }); + return ( -
- + + {field => ( + + + + )} + + + + {field => ( + , example: '{"user": ["id", "email"]}', } - ), - getValue: (val: string) => (val === '' ? {} : JSON.parse(val)), - setValue: (val: string) => { - const schema = JSON.stringify(val, null, 2); - if (schema === '{}') { - return ''; - } - return schema; - }, - validate: ({id, form}) => { - if (form.highlightContext) { - try { - JSON.parse(form.highlightContext); - } catch (e) { - return [[id, 'Invalid JSON']]; - } - } - return []; - }, - }, - ]} - /> - + )} + > + + + )} + + ); }