Skip to content

feat(vsts): Add API pipeline frontend flow#113095

Merged
evanpurkhiser merged 1 commit intomasterfrom
evanpurkhiser/feat-vsts-add-api-pipeline-frontend-flow
Apr 15, 2026
Merged

feat(vsts): Add API pipeline frontend flow#113095
evanpurkhiser merged 1 commit intomasterfrom
evanpurkhiser/feat-vsts-add-api-pipeline-frontend-flow

Conversation

@evanpurkhiser
Copy link
Copy Markdown
Member

@evanpurkhiser evanpurkhiser requested a review from a team as a code owner April 15, 2026 19:53
@evanpurkhiser evanpurkhiser requested a review from a team April 15, 2026 19:53
@linear-code
Copy link
Copy Markdown

linear-code bot commented Apr 15, 2026

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 15, 2026
Comment on lines +73 to +83
<DropdownMenu
triggerLabel={t('Select Azure DevOps organization')}
items={accounts.map(account => ({
key: account.accountId,
label: account.accountName,
}))}
isDisabled={isAdvancing}
onAction={key => {
advance({account: key as string});
}}
/>
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 onAction handler is passed as a top-level prop to DropdownMenu, but Sentry's implementation never invokes it, so advance is never called.
Severity: HIGH

Suggested Fix

Move onAction to each item in the items array instead of passing it as a top-level DropdownMenu prop: items={accounts.map(account => ({ key: account.accountId, label: account.accountName, onAction: () => advance({account: account.accountId}) }))} and remove the top-level onAction prop.

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/components/pipeline/pipelineIntegrationVsts.tsx#L73-L83

Potential issue: In `VstsAccountSelectionStep`, the `onAction` callback is passed as a
top-level prop to the `<DropdownMenu>` component (line 80). However, Sentry's custom
`DropdownMenuItem` always overrides react-aria's menu-level `onAction` by passing its
own `actionHandler` to `useMenuItem({ onAction: actionHandler })` (item.tsx:178). This
`actionHandler` only calls the **item-level** `onAction?.()` from `node.value`
(item.tsx:133), which is undefined here since items don't define `onAction`.
Consequently, selecting an account does nothing — `advance` is never called, and the
pipeline can never progress past the account selection step.

Did we get this right? 👍 / 👎 to inform future reviews.

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.

Not a real issue. The top-level onAction prop flows through {...props} into DropdownMenuList, which passes it to react-aria's useMenu(). React-aria fires the menu-level onAction(key) when any item is selected, independently of item-level onAction. The test in pipelineIntegrationVsts.spec.tsx confirms this works correctly.

@evanpurkhiser evanpurkhiser merged commit 8c22c81 into master Apr 15, 2026
66 checks passed
@evanpurkhiser evanpurkhiser deleted the evanpurkhiser/feat-vsts-add-api-pipeline-frontend-flow branch April 15, 2026 21:44
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