ref(settings): migrate integration external mapping form to new form system by TkDodo · Pull Request #110298 · getsentry/sentry

TkDodo · 2026-03-10T12:36:02Z

No description provided.

… submission handling

…rnal-mapping-form

linear-code · 2026-03-10T12:36:06Z

DE-948 Migrate integrations

TkDodo · 2026-03-10T12:36:22Z

@cursoragent review

cursor · 2026-03-10T12:36:32Z

Perfect! Now let me create a comprehensive code review summary:

Code Review Summary

I've completed a thorough review of this form migration PR. All tests pass, linting passes, and TypeScript has no errors. Here's my detailed analysis:

✅ Migration Quality: Excellent

This is a high-quality migration from the legacy Form/FormModel system to the new TanStack-based form system. The changes properly follow Sentry's form migration guidelines.

What Changed

Core Migration:

✅ Converted class component → function components (InlineMappingForm + ModalMappingForm)
✅ Replaced Form/FormModel/FieldFromConfig → useScrapsForm + AutoSaveField
✅ Added proper Zod schemas for validation
✅ Modernized data fetching with queryOptions and useMutation
✅ Proper error handling with setFieldErrors

API Simplification:

✅ Removed complex state management (queryResults, combineResultsById)
✅ Eliminated unnecessary props: dataEndpoint, sentryNamesMapper, onResults, mappingKey
✅ Cleaner type definitions with discriminated unions (InlineProps | ModalProps)

Improved UX:

✅ AutoSaveField for inline forms (auto-saves on change)
✅ Manual submit for modal forms (with proper validation)
✅ Better separation of concerns between inline and modal variants

Test Coverage

All 12 tests pass, including new tests for edge cases:

✅ Rendering with no mapping, full mapping, and suggested mapping
✅ Both inline and modal variants
✅ Form submission with correct data transformation
✅ Auto-save behavior in inline mode
✅ Re-selection after save (regression test)
✅ Search functionality with debounced queries
✅ DefaultOptions support

Code Quality

Strengths:

Clean separation between inline and modal form logic
Proper use of core layout components (Stack, Flex)
Good type safety with Zod schemas
Comprehensive test coverage with realistic mock data
Follows React Testing Library best practices

Minor Observations:

Line 230 has a type cast as unknown as ReturnType<typeof makeTeamSelectQueryOptions> - this is a TypeScript workaround for conditional type narrowing. While not ideal, it's safe and functional. Could be eliminated by using a generic helper function, but the current approach is acceptable.
The mergeOptions function properly handles edge cases:
- Merges defaultOptions without duplicates
- Ensures current mapping is always available in the select
- Handles both full mappings and suggestions

Integration with Parent Components

The changes to parent components (integrationExternalTeamMappings.tsx, integrationExternalUserMappings.tsx, integrationExternalMappings.tsx) are excellent:

✅ Simplified modal rendering (removed Fragment wrapper, passes modalProps directly)
✅ Added proper async handling for refetches (onSubmitSuccess returns Promise)
✅ Removed complex query result caching logic
✅ Cleaner prop passing

Linting & Type Safety

✅ All pre-commit hooks pass (eslint, prettier, stylelint)
✅ No TypeScript errors
✅ Proper use of semantic HTML and ARIA roles in tests

Recommendation: ✅ APPROVE

This PR is ready to merge. The migration is well-executed, maintains backward compatibility at the API level, improves code maintainability, and has comprehensive test coverage. The code quality is high and follows all Sentry development best practices.

…rnal-mapping-form

github-actions · 2026-03-10T13:47:20Z

Backend Test Failures

Failures on 49f8573 in this run:

tests/sentry/profiles/test_task.py::DeobfuscationViaSymbolicator::test_basic_resolving — log

tests/sentry/profiles/test_task.py:627: in test_basic_resolving
    assert android_profile["profile"]["methods"] == [
E   AssertionError: assert [{'class_name...oolean', ...}] == [{'class_name...oolean', ...}]
E     
E     At index 0 diff: {'class_name': 'org.slf4j.helpers.Util$ClassContextSecurityManager', 'name': 'getClassContext', 'signature': '()', 'source_file': 'Util.java', 'source_line': 67, 'data': {'deobfuscation_status': 'deobfuscated'}} != {'data': {'deobfuscation_status': 'deobfuscated'}, 'name': 'getClassContext', 'class_name': 'org.slf4j.helpers.Util$ClassContextSecurityManager', 'signature': '()', 'source_file': 'Something.java', 'source_line': 67}
E     
E     Full diff:
E       [
E           {
E               'class_name': 'org.slf4j.helpers.Util$ClassContextSecurityManager',
E               'data': {
E                   'deobfuscation_status': 'deobfuscated',
E               },
E               'name': 'getClassContext',
E               'signature': '()',
E     -         'source_file': 'Something.java',
E     ?                         ^^^^ - ^^
E     +         'source_file': 'Util.java',
E     ?                         ^  ^
E               'source_line': 67,
E           },
E           {
E               'class_name': 'org.slf4j.helpers.Util$ClassContextSecurityManager',
E               'data': {
E                   'deobfuscation_status': 'deobfuscated',
E               },
E               'name': 'getExtraClassContext',
E               'signature': '(): boolean',
E     -         'source_file': 'Else.java',
E     ?                         ^ --
E     +         'source_file': 'Util.java',
E     ?                         ^^^
E               'source_line': 69,
E           },
E       ]

tests/sentry/profiles/test_task.py::DeobfuscationViaSymbolicator::test_inline_resolving — log

tests/sentry/profiles/test_task.py:683: in test_inline_resolving
    assert android_profile["profile"]["methods"] == [
E   AssertionError: assert [{'class_name...andler', ...}] == [{'class_name...andler', ...}]
E     
E     At index 0 diff: {'class_name': 'io.sentry.sample.-$$Lambda$r3Avcbztes2hicEObh02jjhQqd4', 'name': 'onClick', 'signature': '()', 'source_file': '-.java', 'source_line': 2, 'data': {'deobfuscation_status': 'deobfuscated'}} != {'class_name': 'io.sentry.sample.-$$Lambda$r3Avcbztes2hicEObh02jjhQqd4', 'data': {'deobfuscation_status': 'deobfuscated'}, 'name': 'onClick', 'signature': '()', 'source_file': None, 'source_line': 2}
E     
E     Full diff:
E       [
E           {
E               'class_name': 'io.sentry.sample.-$$Lambda$r3Avcbztes2hicEObh02jjhQqd4',
E               'data': {
E                   'deobfuscation_status': 'deobfuscated',
E               },
E               'name': 'onClick',
E               'signature': '()',
E     -         'source_file': None,
E     ?                        ^^^^
E     +         'source_file': '-.java',
E     ?                        ^^^^^^^^
E               'source_line': 2,
E           },
E           {
E               'class_name': 'io.sentry.sample.MainActivity',
E               'data': {
E                   'deobfuscation_status': 'deobfuscated',
E               },
E               'inline_frames': [
E                   {
E                       'class_name': 'io.sentry.sample.MainActivity',
E                       'data': {
E                           'deobfuscation_status': 'deobfuscated',
E                       },
E                       'name': 'onClickHandler',
E                       'signature': '()',
E                       'source_file': 'MainActivity.java',
E                       'source_line': 40,
E                   },
E                   {
E                       'class_name': 'io.sentry.sample.MainActivity',
E                       'data': {
E                           'deobfuscation_status': 'deobfuscated',
E                       },
E                       'name': 'foo',
E                       'signature': '()',
E                       'source_file': 'MainActivity.java',
E                       'source_line': 44,
E                   },
E                   {
E                       'class_name': 'io.sentry.sample.MainActivity',
E                       'data': {
... (14 more lines)

static/app/views/settings/organizationIntegrations/integrationExternalTeamMappings.tsx

…rnal-mapping-form

github-actions · 2026-03-10T15:45:09Z

Backend Test Failures

Failures on 50a92d2 in this run:

tests/sentry/spans/test_buffer.py::test_hdel_redirect_map_batch_size[3-2] — log

tests/sentry/spans/test_buffer.py:1316: in test_hdel_redirect_map_batch_size
    assert len(batch_sizes_seen) == expected_batches
E   assert 3 == 2
E    +  where 3 = len([3, 3, 100])

tests/sentry/spans/test_buffer.py::test_hdel_redirect_map_batch_size[2-3] — log

tests/sentry/spans/test_buffer.py:1316: in test_hdel_redirect_map_batch_size
    assert len(batch_sizes_seen) == expected_batches
E   assert 4 == 3
E    +  where 4 = len([2, 2, 2, 100])

github-actions · 2026-03-13T10:02:43Z

Backend Test Failures

Failures on f32ee5b in this run:

tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_and_snuba_hourly_counts_match — log

tests/sentry/issues/escalating/test_escalating.py:365: in test_eap_and_snuba_hourly_counts_match
    assert snuba_count == eap_count == 6
E   assert 0 == 6

tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_and_snuba_past_counts_aggregate_same_hour

— log

tests/sentry/issues/escalating/test_escalating.py:439: in test_eap_and_snuba_past_counts_aggregate_same_hour
    assert len(snuba_results) == len(eap_results) == 1
E   assert 0 == 1
E    +  where 0 = len([])

tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_and_snuba_past_counts_match_multiple_groups

— log

tests/sentry/issues/escalating/test_escalating.py:423: in test_eap_and_snuba_past_counts_match_multiple_groups
    assert eap_counts[group_a.id] == 2
E   KeyError: 233

tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_hourly_count_excludes_old_events — log

tests/sentry/issues/escalating/test_escalating.py:382: in test_eap_hourly_count_excludes_old_events
    assert snuba_count == eap_count == 1
E   assert 0 == 1

tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_and_snuba_past_counts_match_single_group

— log

tests/sentry/issues/escalating/test_escalating.py:396: in test_eap_and_snuba_past_counts_match_single_group
    assert len(snuba_results) == len(eap_results) == 1
E   assert 0 == 1
E    +  where 0 = len([])

…ct callbacks

…rnal-mapping-form

github-actions · 2026-03-13T13:48:08Z

Backend Test Failures

Failures on ee7f759 in this run:

tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_and_snuba_past_counts_aggregate_same_hour

— log

tests/sentry/issues/escalating/test_escalating.py:439: in test_eap_and_snuba_past_counts_aggregate_same_hour
    assert len(snuba_results) == len(eap_results) == 1
E   assert 0 == 1
E    +  where 0 = len([])

tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_and_snuba_past_counts_match_multiple_groups

— log

tests/sentry/issues/escalating/test_escalating.py:423: in test_eap_and_snuba_past_counts_match_multiple_groups
    assert eap_counts[group_a.id] == 2
E   KeyError: 233

tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_hourly_count_excludes_old_events — log

tests/sentry/issues/escalating/test_escalating.py:382: in test_eap_hourly_count_excludes_old_events
    assert snuba_count == eap_count == 1
E   assert 0 == 1

tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_and_snuba_hourly_counts_match — log

tests/sentry/issues/escalating/test_escalating.py:365: in test_eap_and_snuba_hourly_counts_match
    assert snuba_count == eap_count == 6
E   assert 0 == 6

tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_and_snuba_past_counts_match_single_group

— log

tests/sentry/issues/escalating/test_escalating.py:396: in test_eap_and_snuba_past_counts_match_single_group
    assert len(snuba_results) == len(eap_results) == 1
E   assert 0 == 1
E    +  where 0 = len([])

TkDodo · 2026-03-13T14:56:06Z

static/app/components/core/select/select.tsx

changes in this file are so that we can support object values in our select component. We need that because when the user selects a team, we need more than one value from that team (we need slug and id).

react-select actually supports this easily because it wants the value to be a full option (incl. label etc), and it also returns that in onChange. However we have decided to make our select abstraction work differently - it only takes the value from inside the option and we then try to match it back against options to pass it forward to react-select.

this complicates things a lot, because objects aren’t === comparable, and if we give react-select a raw object it treats it as option but it might not have the correct structure.

I really hope these changes don’t break any other parts but I don’t think selects with object values are used a lot. Suffice to say I 🤬 select 😂

TkDodo · 2026-03-13T14:57:05Z

static/app/views/settings/organizationIntegrations/integrationExternalTeamMappings.tsx

+      onSubmitSuccess={async () => {
+        await fetchData();
+      }}


this ensures we refetch the queries that populate our form after form submission.

tests/acceptance/test_organization_integration_configuration_tabs.py

…rnal-mapping-form

This reverts commit a274f93.

natemoo-re

Great refactor! No notes.

natemoo-re · 2026-03-17T15:28:40Z

static/app/views/settings/featureFlags/changeTracking/newProviderForm.tsx

+      return mutation.mutateAsync(schema.parse(value)).catch(error => {
+        if (error instanceof RequestError) {
+          setFieldErrors(formApi, error);


👌 nice refactor!

…rnal-mapping-form # Conflicts: # static/app/components/core/select/select.tsx

…rnal-mapping-form

…system (#110298) Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>

TkDodo added 9 commits March 10, 2026 11:17

feat: integrate modal support for external mapping form with improved…

ca71e01

… submission handling

fix: async select

cec4461

ref: mutation endpoint

d3f9425

ref: immutable updates

99e2055

fix: defaultOptions

81e694f

fix: refetch data after change

25afec0

fix: error handling

8d78d22

ref: type improvements

ef60198

Merge branch 'master' into tkdodo/ref/de-948-migrate-integration-exte…

3bc8836

…rnal-mapping-form

github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 10, 2026

vercel bot deployed to Preview March 10, 2026 12:38 View deployment

ref: avoid type assertion

7647a0c

vercel bot deployed to Preview March 10, 2026 12:48 View deployment

TkDodo added 2 commits March 10, 2026 14:23

fix: stabilize selectors

406f488

Merge branch 'master' into tkdodo/ref/de-948-migrate-integration-exte…

356c85d

…rnal-mapping-form

vercel bot deployed to Preview March 10, 2026 13:26 View deployment

TkDodo marked this pull request as ready for review March 10, 2026 13:52

TkDodo requested review from a team as code owners March 10, 2026 13:52

sentry bot reviewed Mar 10, 2026

View reviewed changes

static/app/views/settings/organizationIntegrations/integrationExternalTeamMappings.tsx Outdated Show resolved Hide resolved

cursor bot reviewed Mar 10, 2026

View reviewed changes

static/app/views/settings/organizationIntegrations/integrationExternalTeamMappings.tsx Outdated Show resolved Hide resolved

Merge branch 'master' into tkdodo/ref/de-948-migrate-integration-exte…

cb12e87

…rnal-mapping-form

vercel bot deployed to Preview March 10, 2026 15:25 View deployment

fix: keep track of team slugs with a ref

6856881

TkDodo added 2 commits March 13, 2026 14:11

fix(select): prevent unmatched object values from crashing react-sele…

a2ebcef

…ct callbacks

Merge branch 'master' into tkdodo/ref/de-948-migrate-integration-exte…

b592426

…rnal-mapping-form

vercel bot deployed to Preview March 13, 2026 13:14 View deployment

fix(select): unmatched object fallback

5f3e107

vercel bot deployed to Preview March 13, 2026 13:28 View deployment

TkDodo commented Mar 13, 2026

View reviewed changes

Merge branch 'master' into tkdodo/ref/de-948-migrate-integration-exte…

6e84699

…rnal-mapping-form

vercel bot deployed to Preview March 16, 2026 10:05 View deployment

TkDodo requested review from a team and JonasBa March 16, 2026 10:47

Merge branch 'master' into tkdodo/ref/de-948-migrate-integration-exte…

1484478

…rnal-mapping-form

vercel bot deployed to Preview March 16, 2026 15:38 View deployment

ref: AutoSaveField becomes AutoSaveForm

db0c89c

vercel bot deployed to Preview March 16, 2026 15:54 View deployment

fix: issueSeerBadge Quick Fix positioning

a274f93

vercel bot deployed to Preview March 16, 2026 17:33 View deployment

Revert "fix: issueSeerBadge Quick Fix positioning"

2af0bdf

This reverts commit a274f93.

vercel bot deployed to Preview March 16, 2026 18:13 View deployment

natemoo-re approved these changes Mar 17, 2026

View reviewed changes

Merge branch 'master' into tkdodo/ref/de-948-migrate-integration-exte…

e5d67ed

…rnal-mapping-form # Conflicts: # static/app/components/core/select/select.tsx

vercel bot deployed to Preview March 18, 2026 09:06 View deployment

Merge branch 'master' into tkdodo/ref/de-948-migrate-integration-exte…

b6e8e77

…rnal-mapping-form

vercel bot deployed to Preview March 18, 2026 09:19 View deployment

fix: error handling

1ffd4c4

vercel bot deployed to Preview March 18, 2026 09:26 View deployment

TkDodo merged commit 356ed75 into master Mar 18, 2026
59 checks passed

TkDodo deleted the tkdodo/ref/de-948-migrate-integration-external-mapping-form branch March 18, 2026 09:43

JonasBa pushed a commit that referenced this pull request Mar 18, 2026

ref(settings): migrate integration external mapping form to new form …

90fabc8

…system (#110298) Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>

Uh oh!

Conversation

TkDodo commented Mar 10, 2026

Uh oh!

linear-code bot commented Mar 10, 2026

Uh oh!

TkDodo commented Mar 10, 2026

Uh oh!

cursor bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

✅ Migration Quality: Excellent

What Changed

Test Coverage

Code Quality

Integration with Parent Components

Linting & Type Safety

Recommendation: ✅ APPROVE

Uh oh!

github-actions bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backend Test Failures

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backend Test Failures

Uh oh!

github-actions bot commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backend Test Failures

Uh oh!

github-actions bot commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backend Test Failures

Uh oh!

TkDodo Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

TkDodo Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

natemoo-re left a comment

Choose a reason for hiding this comment

Uh oh!

natemoo-re Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cursor bot commented Mar 10, 2026 •

edited

Loading

github-actions bot commented Mar 10, 2026 •

edited

Loading

github-actions bot commented Mar 10, 2026 •

edited

Loading

github-actions bot commented Mar 13, 2026 •

edited

Loading

github-actions bot commented Mar 13, 2026 •

edited

Loading