Skip to content

ref(forms): Migrate addCodeOwnerModal off legacy Form#115256

Open
priscilawebdev wants to merge 19 commits into
masterfrom
priscila/ref/migrate-addcodeownermodal
Open

ref(forms): Migrate addCodeOwnerModal off legacy Form#115256
priscilawebdev wants to merge 19 commits into
masterfrom
priscila/ref/migrate-addcodeownermodal

Conversation

@priscilawebdev
Copy link
Copy Markdown
Member

@priscilawebdev priscilawebdev commented May 11, 2026

Replace the legacy Form + SelectField wrapper with useScrapsForm and
field.Select. The form value drives the codeowners file query via
useStore, keeping behavior identical while removing the deprecated
form system.

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

github-actions Bot commented May 11, 2026

📊 Type Coverage Diff

Metric Before After Delta
Coverage 93.47% 93.47% ±0%
Typed 135,132 135,138 🟢 +6
Untyped 9,437 9,437 ±0
🔍 2 new type safety issues introduced

any-typed symbols (2 new)

File Line Detail
static/app/views/settings/project/projectOwnership/addCodeOwnerModal.tsx 104 Body (param(binding))
static/app/views/settings/project/projectOwnership/addCodeOwnerModal.tsx 105 Footer (param(binding))

This is informational only and does not block the PR.

Use Stack with gap to space the select and result panel, and override
the Panel's default margin-bottom so spacing is driven entirely by the
Stack layout primitive.
The integrations query was fetched eagerly in the parent but only used
in the LinkCodeOwners branch, which renders when no code mappings
exist. Move the query into that component so it only runs when needed,
and switch from the deprecated useApiQuery to apiOptions + useQuery.
Follow the form.Subscribe pattern: extract CodeownersFileStatus and
AddFileButton sub-components, each subscribing to the codeMappingId
field where it is consumed. The shared useCodeownersFile hook reuses
react-query's cache so only one fetch runs.

Also inlines the now-single-use baseUrl in ApplyCodeMappings.
- Drop redundant aria-label on Add File button (visible text already
  provides the accessible name)
- Wrap the Setup Integration LinkButton text in t()
- Remove duplicate text inside the tct strong tag; tct injects the
  template's inner text as children
- Panel + styled PanelBody grids -> Container + Grid
- IntegrationsList styled div -> Stack with paddingTop
- LinkButton with child icon + styled name -> icon prop
- Bare div text -> Text as=p
- Drops styled, Panel, PanelBody imports
Replace raw <p> tags with the scraps Text component per the design
system guidelines.
SourceFile and NoSourceFile each wrapped their grid in an identical
Container. Move the wrapper up into CodeownersFileStatus so the
children just render the grid layout.
Replace findByRole on the always-present button with an explicit
waitFor on the enabled state, since what we actually need to wait for
is the codeownersFile mock to resolve.
space.md is 8px in scraps but the legacy panel used 12px. Use lg (12px)
to match the original spacing.
- Check err instanceof RequestError and fall back to a generic toast
  for unexpected error shapes, matching the form-migration reference
  pattern.
- Drop gcTime: 0 — redundant for a component-scoped useMutation with
  no mutationKey.
@priscilawebdev
Copy link
Copy Markdown
Member Author

bugbot run

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 11, 2026

DE-999

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

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 c605a76. Configure here.

Narrow mutationError.responseJSON.raw via typeof at the call site and
pass it as a string prop to ErrorMessage. Replaces the as {raw?: string}
cast that didn't match RequestError.responseJSON's actual unknown
shape.
DRF returns serializer.errors as Record<string, string[]>, so
responseJSON.raw is string[], not string. The legacy cast and my
recent typeof string guard were both incorrect; with the typeof guard
the inline ErrorMessage would never render.

Narrow via Array.isArray + typeof rawError[0] === 'string', pass the
first error message to ErrorMessage, and drop the rawError[0]!
indexing inside ErrorMessage so we split the actual message string
rather than its first character.
@priscilawebdev priscilawebdev marked this pull request as ready for review May 11, 2026 09:10
@priscilawebdev priscilawebdev requested a review from a team as a code owner May 11, 2026 09:10
Comment thread static/app/views/settings/project/projectOwnership/addCodeOwnerModal.tsx Outdated
</Button>
<form.Subscribe selector={state => state.values.codeMappingId}>
{codeMappingId => (
<AddFileButton
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.

shouldn’t this just be a <form.SubmitButton> and the mutation happens inside the onSubmit handler of the form? It’s a bit weird to have a form that we never actually submit ...

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.

oh yes. Good catch! I will update that

Comment on lines -337 to -366
const StyledSelectField = styled(SelectField)`
border-bottom: None;
padding-right: 16px;
`;
const FileResult = styled('div')`
width: inherit;
`;
const NoSourceFileBody = styled(PanelBody)`
display: grid;
padding: 12px;
grid-template-columns: 30px 1fr;
align-items: center;
`;
const SourceFileBody = styled(PanelBody)`
display: grid;
padding: 12px;
grid-template-columns: 30px 1fr auto;
align-items: center;
`;

const IntegrationsList = styled('div')`
display: grid;
gap: ${p => p.theme.space.md};
justify-items: center;
margin-top: ${p => p.theme.space.xl};
`;

const IntegrationName = styled('p')`
padding-left: 10px;
`;
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.

this is the best kind of removal 🔥

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.

it's for sure a good feeling haha

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a3b289c. Configure here.

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