Skip to content

ref(settings): Migrate project issue grouping form to scraps form#116465

Merged
priscilawebdev merged 12 commits into
masterfrom
prims/migrate-project-issue-grouping-form
Jun 3, 2026
Merged

ref(settings): Migrate project issue grouping form to scraps form#116465
priscilawebdev merged 12 commits into
masterfrom
prims/migrate-project-issue-grouping-form

Conversation

@priscilawebdev

@priscilawebdev priscilawebdev commented May 29, 2026

Copy link
Copy Markdown
Member

Replace the legacy JsonForm/FormModel implementation of the project Issue
Grouping settings (Fingerprint Rules, Stack Trace Rules, and the read-only
Derived Grouping Enhancements) with the TanStack-based scraps form system.

Each rule was a saveOnBlur:false field, so the migration uses useScrapsForm
with an explicit Save/Cancel button and the 'applies to future events' info
notice, instead of AutoSaveForm. The derived enhancements field stays
read-only.

Wrap each form in FormSearch and regenerate generatedFieldRegistry.ts so the
rules remain searchable in SettingsSearch, and drop the now-unused
projectIssueGrouping data file. Add the fingerprintingRules,
groupingEnhancements, and derivedGroupingEnhancements fields (returned by the
project detail API) to the DetailedProject type.

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 29, 2026
@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

📊 Type Coverage Diff

✅ no issues found

The legacy Form displayed server-side validation errors inline. The initial
scraps migration only showed a generic error toast, hiding the specific
message returned for invalid fingerprint or stack trace rule syntax.

Use setFieldErrors in the mutation catch to map API field errors back onto
the form field, falling back to the generic toast when the response has no
field-specific errors. Add interaction tests covering a successful save and
inline error display.

Co-Authored-By: Claude <noreply@anthropic.com>
Update the migrated Fingerprint and Stack Trace rules forms to save through
useUpdateProject, which keeps ProjectsStore and the detailed-project query cache
in sync (with optimistic update and invalidation) instead of a bespoke mutation.

Keep Cancel/Save always visible with the info alert gated on the dirty state, and
render each section under its panel header. Adds the grouping rule fields to the
DetailedProject type and fixture, and regenerates the form field registry so the
fields stay searchable in settings.

Co-Authored-By: Claude <noreply@anthropic.com>
…ct-issue-grouping-form

# Conflicts:
#	static/app/components/core/form/generatedFieldRegistry.ts
#	static/app/types/project.tsx
#	tests/js/fixtures/project.ts
Match the organization slug form layout: the dirty-state info alert now spans
full width above a right-aligned Cancel/Save row, instead of sitting inline beside
the buttons. Also drop the non-null assertion from the spec by scoping queries to
the fingerprint form with within().

Co-Authored-By: Claude <noreply@anthropic.com>
@priscilawebdev

Copy link
Copy Markdown
Member Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit f2bf628. Configure here.

Replace the one-off RuleDescription and RuleExample styled components with the
core Text component (variant=muted/size=sm for descriptions, as=pre/monospace for
the examples), removing the last emotion styled usage from the view.

Co-Authored-By: Claude <noreply@anthropic.com>
Select the fingerprint Save button by render order with an if-guard instead of
throwing when scoping to the form. No behavior change.

Co-Authored-By: Claude <noreply@anthropic.com>
priscilawebdev and others added 2 commits June 3, 2026 08:21
Text does not support `as="pre"` (TextPrimitive excludes pre), causing a
typecheck failure. Render the rule examples as a div with `wrap="pre"` to
preserve the preformatted whitespace, matching the original styled pre.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The grouping rule examples lost their gray background when migrating off the
global `pre` stylesheet rule. Wrap them in a Container with the secondary
background, padding, and radius to restore the boxed callout appearance.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@priscilawebdev priscilawebdev marked this pull request as ready for review June 3, 2026 06:44
@priscilawebdev priscilawebdev requested review from a team as code owners June 3, 2026 06:44
@linear-code

linear-code Bot commented Jun 3, 2026

Copy link
Copy Markdown

DE-989

Comment thread static/app/views/settings/projectIssueGrouping/index.tsx
The useUpdateProject refactor dropped the setFieldErrors handling (and its
test) added to surface server-side validation errors inline. Invalid
fingerprint/stack-trace rule syntax fell back to a generic toast instead of
showing the actionable message on the field. Restore the inline handling and
its regression test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the getAllByRole index + comment with closest('form') scoping, matching the inline-error test and avoiding a non-null assertion.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
return;
}
addErrorMessage(t('Unable to save changes.'));
}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The form is reset with the submitted value, not the server's normalized response, causing the UI to be out of sync with the actual saved state.
Severity: MEDIUM

Suggested Fix

Instead of resetting the form with the submitted value, the form should be updated with the normalized data returned from the server. This can be achieved by using the updatedProject data from the useUpdateProject mutation's onSuccess callback to reset the form. For example, formApi.reset({fingerprintingRules: updatedProject.fingerprintingRules}). This ensures the form's state always reflects the true state from the backend.

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/projectIssueGrouping/index.tsx#L153

Potential issue: After a successful save of fingerprinting or stack trace rules, the
form is reset using `formApi.reset(value)`, where `value` is the original
client-submitted data. However, the server may normalize this data (e.g., by stripping
whitespace). The updated, normalized data is stored in the `ProjectsStore` and React
Query cache, but the form component does not re-initialize or update its state to
reflect this change. This results in the UI displaying a value that is out of sync with
what is actually saved on the backend.

Also affects:

  • static/app/views/settings/projectIssueGrouping/index.tsx:202~202

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.

correct, but at the moment there is no normalization on the backend and the data is basically the same.

@priscilawebdev priscilawebdev merged commit 2fa0eb0 into master Jun 3, 2026
76 checks passed
@priscilawebdev priscilawebdev deleted the prims/migrate-project-issue-grouping-form branch June 3, 2026 07:15
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