Skip to content

ref(forms): Migrate highlights settings#115778

Merged
priscilawebdev merged 8 commits into
masterfrom
priscilawebdev/ref/de-1251-highlights-settings-form-migration
May 21, 2026
Merged

ref(forms): Migrate highlights settings#115778
priscilawebdev merged 8 commits into
masterfrom
priscilawebdev/ref/de-1251-highlights-settings-form-migration

Conversation

@priscilawebdev
Copy link
Copy Markdown
Member

@priscilawebdev priscilawebdev commented May 19, 2026

Replace the legacy highlights JsonForm with AutoSaveForm text areas for tags and context.

This keeps the existing autosave behavior, preserves the project cache updates,
and tightens the tests around the autosave requests.

closes https://linear.app/getsentry/issue/DE-1251/migrate-eventshighlightshighlightssettingsformtsx-from-legacy-form

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 19, 2026

DE-1251

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 19, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

📊 Type Coverage Diff

Metric Before After Delta
Coverage 93.59% 93.59% ±0%
Typed 135,603 135,611 🟢 +8
Untyped 9,292 9,292 ±0
🔍 3 new type safety issues introduced

Type assertions (as) (3 new)

File Line Detail
static/app/components/core/form/autoSaveForm.tsx 208 as never{ [name]: {message: t('Failed to save')}, } as never
static/app/components/core/form/autoSaveForm.tsx 218 as never{[name]: true} as never
static/app/components/core/form/autoSaveForm.tsx 224 `as Record<
    TFieldName,
    SchemaOutput<TSchema>[TFieldName]
  >` — `parsedValue.data as Record< TFieldName, SchemaOutput<TSchema>[TFieldName] >` |

This is informational only and does not block the PR.

@priscilawebdev priscilawebdev force-pushed the priscilawebdev/ref/de-1251-highlights-settings-form-migration branch from 8438cf1 to e3f74f3 Compare May 19, 2026 11:20
@priscilawebdev priscilawebdev force-pushed the priscilawebdev/ref/de-1251-highlights-settings-form-migration branch from e3f74f3 to 0021112 Compare May 19, 2026 11:22
@priscilawebdev priscilawebdev force-pushed the priscilawebdev/ref/de-1251-highlights-settings-form-migration branch from 0021112 to ea42b1f Compare May 19, 2026 11:27
@priscilawebdev priscilawebdev force-pushed the priscilawebdev/ref/de-1251-highlights-settings-form-migration branch 2 times, most recently from 0bd5d8d to 26c9707 Compare May 19, 2026 11:31
@priscilawebdev priscilawebdev force-pushed the priscilawebdev/ref/de-1251-highlights-settings-form-migration branch 3 times, most recently from a631887 to fb3d394 Compare May 19, 2026 11:36
@priscilawebdev priscilawebdev marked this pull request as ready for review May 20, 2026 08:12
@priscilawebdev priscilawebdev requested a review from a team as a code owner May 20, 2026 08:12
@priscilawebdev priscilawebdev requested a review from a team May 20, 2026 08:12
return {};
}

return JSON.parse(highlightContext) as NonNullable<DetailedProject['highlightContext']>;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we re-use the schema here instead of doing another manual JSON.parse ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed in principle ....and I tried this. To re-use the schema we need AutoSaveForm to apply the transform on submit (so mutationFn receives the parsed output instead of the raw form value). This required a lot of changes in other files (PR is updated)... What is your take on this?

Comment thread static/app/components/events/highlights/highlightsSettingsForm.tsx Outdated
@priscilawebdev priscilawebdev force-pushed the priscilawebdev/ref/de-1251-highlights-settings-form-migration branch from ecc4f1f to cc24014 Compare May 20, 2026 11:55
@priscilawebdev priscilawebdev force-pushed the priscilawebdev/ref/de-1251-highlights-settings-form-migration branch from bf0dbd7 to 2ac4318 Compare May 20, 2026 12:19
@priscilawebdev priscilawebdev force-pushed the priscilawebdev/ref/de-1251-highlights-settings-form-migration branch from 2ac4318 to 249df1e Compare May 20, 2026 12:21
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lots of unrelated formatting changes here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes... I was expecting these to be auto fixed in this PR 😅

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`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(), …)` — not `z.object({})`, which declares zero keys and will strip everything at submit time.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can also use passthrough to allow arbitrary values

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passthrough is deprecated, so I am using looseObject({}) instead 710fe6e

Restore single quotes, brace spacing, and arrow-param style to match
the rest of the file; keep only the new "Transformed Submit Values"
section as the intended change.
Switch the choice_mapper schema in backendJsonFormAdapter to
z.looseObject({}) so arbitrary keys pass through, matching the modern
Zod 4 idiom (replaces deprecated .passthrough()). Mention z.looseObject
alongside z.record in the AutoSaveForm doc warning.
@priscilawebdev priscilawebdev enabled auto-merge (squash) May 21, 2026 06:04
@priscilawebdev priscilawebdev merged commit cf93ddb into master May 21, 2026
71 checks passed
@priscilawebdev priscilawebdev deleted the priscilawebdev/ref/de-1251-highlights-settings-form-migration branch May 21, 2026 06:11
JonasBa pushed a commit that referenced this pull request May 21, 2026
Replace the legacy highlights JsonForm with AutoSaveForm text areas for
tags and context.

This keeps the existing autosave behavior, preserves the project cache
updates,
and tightens the tests around the autosave requests.

closes
https://linear.app/getsentry/issue/DE-1251/migrate-eventshighlightshighlightssettingsformtsx-from-legacy-form
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants