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])

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

github-actions · 2026-03-10T16:08:30Z

Backend Test Failures

Failures on 7089bff in this run:

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])

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/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/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)

tests/sentry/spans/test_buffer.py::test_hdel_redirect_map_batch_size[5-1] — log

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

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

cursor

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.}

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

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

github-actions · 2026-03-10T17:21:07Z

Backend Test Failures

Failures on aa0c93a 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)

JonasBa · 2026-03-10T17:38:53Z

static/app/components/core/form/scrapsForm.tsx

+    for (const key of Object.keys(responseJSON)) {
+      if (typeof formValues === 'object' && formValues !== null && key in formValues) {
+        const value = responseJSON[key];
+        if (typeof value === 'string') {
+          fieldErrors[key] = {message: value};
+        } else if (Array.isArray(value) && value.length > 0) {
+          fieldErrors[key] = {
+            message: typeof value[0] === 'string' ? value[0] : String(value[0]),
+          };
+        }
+      }
+    }


Should we have a isJSONResponseError or some type of helper that could assert on the type or perform the extraction for us?

depends on if there’s a known structure that the backend sends. I know sometimes, it sends detail, which can be a string or an object too:

sentry/static/app/components/issues/groupList.tsx

Lines 256 to 262 in e16fe15

const detail = (queryError as RequestError | undefined)?.responseJSON?.detail;

if (typeof detail === 'string') {

return detail;

}

if (detail?.message) {

return detail.message;

}

A good solution would be to make sure all backend endpoints return field errors in a standardized format, then we can add our types accordingly directly to RequestError

…rnal-mapping-form

…g logic

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

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

cursor bot reviewed Mar 10, 2026

View reviewed changes

static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx Show resolved Hide resolved

TkDodo added 2 commits March 10, 2026 17:52

ref: remove sentryName

d2b88f2

ref: increase staleTime

b9a5154

vercel bot deployed to Preview March 10, 2026 17:01 View deployment

sentry bot reviewed Mar 10, 2026

View reviewed changes

static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx Show resolved Hide resolved

cursor bot reviewed Mar 10, 2026

View reviewed changes

static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx Show resolved Hide resolved

static/app/components/core/select/select.tsx Outdated Show resolved Hide resolved

JonasBa reviewed Mar 10, 2026

View reviewed changes

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

a78442e

…rnal-mapping-form

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

refactor(select): enhance type definitions and improve option matchin…

0415467

…g logic

vercel bot deployed to Preview March 10, 2026 18:36 View deployment

	const detail = (queryError as RequestError \| undefined)?.responseJSON?.detail;
	if (typeof detail === 'string') {
	return detail;
	}
	if (detail?.message) {
	return detail.message;
	}

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!

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!

cursor bot left a comment

Choose a reason for hiding this comment

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!

JonasBa Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

TkDodo Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 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 10, 2026 •

edited

Loading

github-actions bot commented Mar 10, 2026 •

edited

Loading