feat(data-forwarding): migrate project override form to useScrapsForm#108807
Conversation
github-actions
bot
added
the
Scope: Frontend
…rwarding-edit-form Merge forms.tsx conflict by combining both branches: keep dataForwarderOverrideSchema (used by ProjectOverrideForm) and adopt buildDataForwardingProviderSchema and buildDataForwardingProviderConfig from the incoming branch (used by the edit form). Drop the legacy JsonFormObject-based getProjectOverrideForm and related constants that were superseded by the new useScrapsForm-based component. Co-Authored-By: Claude <noreply@anthropic.com>
…/data-forwarding-form
…/data-forwarding-form
JonasBa
force-pushed
the
jb/settings/data-forwarding-form
branch
from
1c1b6c9 to
467ae8c
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
static/app/views/settings/organizationDataForwarding/components/projectOverrideForm.tsx
Show resolved
Hide resolved
static/app/views/settings/organizationDataForwarding/components/projectOverrideForm.tsx
Show resolved
Hide resolved
…/data-forwarding-form
| write_key: z.string(), | ||
| // Splunk | ||
| instance_url: z.string(), | ||
| token: z.string(), | ||
| index: z.string(), | ||
| source: z.string(), | ||
| }); | ||
|
|
||
| /** | ||
| * Builds a provider-specific validation schema by extending the base form schema | ||
| * with required-field checks for the given provider. |
There was a problem hiding this comment.
Bug: Submitting the project override form sends empty strings for unmodified fields, which incorrectly overwrites the global data forwarding configuration on the backend.
Severity: HIGH
Suggested Fix
Before submitting the form, filter the overrides object to remove key-value pairs where the value is an empty string. This will prevent unmodified fields from being sent to the backend and incorrectly overwriting the base configuration.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: static/app/views/settings/organizationDataForwarding/util/forms.tsx#L47-L74
Potential issue: The new project override form initializes all provider-specific fields
with empty strings. When a user saves the form, these empty strings are sent in the
`overrides` payload, even for fields the user did not modify. The backend uses
`dict.update()` to apply these overrides, which causes the empty strings to overwrite
valid global configuration values. This can corrupt the project's data forwarding
settings, causing forwarding to fail or behave incorrectly as essential fields like
`queue_url` become empty.
| <form.AppField name="source"> | ||
| {field => ( | ||
| <field.Layout.Row | ||
| padding="md" | ||
| label={t('Source')} | ||
| hintText={t('The source to use for the events.')} | ||
| > | ||
| <field.Input | ||
| value={field.state.value ?? ''} | ||
| onChange={field.handleChange} |
There was a problem hiding this comment.
Bug: The project override form does not update with new data after a successful save, continuing to show stale, pre-save values to the user.
Severity: MEDIUM
Suggested Fix
Add a useEffect hook that monitors changes to the projectConfig data. When projectConfig changes (e.g., after a successful save and data refetch), call form.setValues() or a similar method to explicitly update the form's state with the new data, ensuring the UI reflects the latest configuration.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
static/app/views/settings/organizationDataForwarding/components/projectOverrideForm.tsx#L299-L308
Potential issue: The form's state is not updated after a successful save. The
`useScrapsForm` hook is initialized with `defaultValues` only once. After a user saves
changes, the underlying data is refetched, but the form does not reset to reflect these
new values because `defaultValues` is not reactive. As a result, the form continues to
display the stale, pre-save data, which can confuse the user into thinking their changes
were not saved.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| project_id: `${project.id}`, | ||
| overrides, | ||
| is_enabled, | ||
| }); |
There was a problem hiding this comment.
Override form submits all provider fields, not just relevant ones
High Severity
The onSubmit handler sends all form fields as overrides, including empty-string values for fields that belong to other providers. For example, saving a Segment override also sends queue_url: '', region: '', access_key: '', etc. The edit form correctly uses buildDataForwardingProviderConfig to filter fields by provider before submission, but the override form skips this filtering entirely. The old implementation only included provider-specific fields in the form model, so this is a regression.
Additional Locations (1)
| is_enabled, | ||
| }); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Form state not reset after "Clear Override" mutation
Medium Severity
The old code had a useEffect that called formModel.setInitialData whenever projectConfig changed, keeping the form in sync after mutations. The new code only passes defaultValues to useScrapsForm, which is read once on mount. After clicking "Clear Override" (which bypasses the form and directly calls the mutation), the query is invalidated and fresh data arrives, but the form fields retain stale override values. The user sees old data in the inputs even though the server-side overrides were cleared.
Additional Locations (1)
This PR splits the data forwarded forms into separate schemas, using useFieldGroup to manage form configuration between the otherwise duplicate fields
JonasBa
merged commit 99b75af
into
jb/settings/data-forwarding-edit-form
| <Flex justify="end" gap="md" padding="lg 0"> | ||
| <Button | ||
| size="sm" | ||
| disabled={disabled} | ||
| onClick={() => { | ||
| updateDataForwarder({ | ||
| project_id: `${project.id}`, | ||
| overrides: {}, | ||
| is_enabled: projectConfig?.isEnabled ?? false, | ||
| }); | ||
| }} |
There was a problem hiding this comment.
Bug: The ProjectOverrideForm does not update its state after the "Clear Override" action, causing the UI to display stale data until the component is remounted.
Severity: MEDIUM
Suggested Fix
Implement a useEffect hook that observes the projectConfig prop for changes. When projectConfig is updated, call form.setValues() to synchronize the form's state with the new data, ensuring the UI accurately reflects the current configuration.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
static/app/views/settings/organizationDataForwarding/components/projectOverrideForm.tsx#L318-L328
Potential issue: When a user clicks "Clear Override" in the `ProjectOverrideForm`, a
mutation successfully clears the data on the backend and refetches the updated
configuration. However, the form's state is not synchronized with this new data. The
form is initialized once with `defaultValues` from `projectConfig.overrides` but lacks a
mechanism, like a `useEffect` hook, to update the form state when the `projectConfig`
prop changes. As a result, the UI continues to display the old, stale override values,
confusing the user into thinking the operation failed, until the form is closed and
reopened.
2 participants
Migrates the per-project override form from the legacy
JsonForm/FormModelsystem touseScrapsForm(TanStack-based), completing the data forwarding form migration.The old implementation rendered each project override as a flat
Panel/PanelHeaderwith aJsonForminside. The new implementation usesDisclosurefrom@sentry/scraps/disclosure, giving a cleaner collapsed/expanded UX with status tags (Forwarding,Forwarding with Overrides,Disabled) visible in the title row without expanding.Also adds
dataForwarderOverrideSchematoutil/forms.tsxand removes all remaining legacy helpers (getProjectOverrideForm,getProviderForm, the provider form constants, andCalmTag), leavingutil/forms.tsxwith onlygetCreateTooltipand the two Zod schemas.Stacked on: #108806
Fix DE-945