mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
fix(github): complete PR reactions for CodeRabbit reviews (#13456)
* feat(github): add PR comment reaction controls * feat(github): add full PR comment reaction picker * fix(github): cover all reactable PR comment paths * fix(github): reconcile comment reactions with main * fix(github): preserve focus on failed reaction removal
This commit is contained in:
@@ -4044,6 +4044,60 @@ describe('GitHub GraphQL rate-limit guard', () => {
|
||||
expect(noteRateLimitSpendMock).not.toHaveBeenCalledWith('graphql')
|
||||
})
|
||||
|
||||
it('maps review summary reaction subjects from GraphQL', async () => {
|
||||
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
|
||||
ghExecFileAsyncMock
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
data: {
|
||||
repository: {
|
||||
pullRequest: {
|
||||
reviewThreads: { nodes: [] },
|
||||
comments: { nodes: [] },
|
||||
reviews: {
|
||||
nodes: [
|
||||
{
|
||||
id: 'PRR_44',
|
||||
databaseId: 44,
|
||||
author: {
|
||||
__typename: 'Bot',
|
||||
login: 'coderabbitai',
|
||||
avatarUrl: 'https://avatar'
|
||||
},
|
||||
body: 'Automated review summary',
|
||||
createdAt: '2026-04-01T00:00:00Z',
|
||||
url: 'https://github.com/acme/widgets/pull/7#pullrequestreview-44',
|
||||
reactionGroups: [
|
||||
{
|
||||
content: 'ROCKET',
|
||||
viewerHasReacted: true,
|
||||
reactors: { totalCount: 2 }
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
.mockResolvedValueOnce({ stdout: '[]' })
|
||||
.mockResolvedValueOnce({ stdout: '[]' })
|
||||
|
||||
await expect(getPRComments('/repo-root', 7)).resolves.toEqual([
|
||||
expect.objectContaining({
|
||||
id: 44,
|
||||
reactionSubjectId: 'PRR_44',
|
||||
isBot: true,
|
||||
reactions: [{ content: 'rocket', count: 2, viewerHasReacted: true }]
|
||||
})
|
||||
])
|
||||
expect(ghExecFileAsyncMock.mock.calls[0]?.[0]).toEqual(
|
||||
expect.arrayContaining([expect.stringContaining('reviews(first: 100)')])
|
||||
)
|
||||
})
|
||||
|
||||
it('uses explicit PR repo for comments when a fork PR is discovered', async () => {
|
||||
rateLimitGuardMock.mockImplementation(((bucket: string) =>
|
||||
bucket === 'graphql'
|
||||
|
||||
@@ -4157,6 +4157,23 @@ query($owner: String!, $repo: String!, $pr: Int!) {
|
||||
}
|
||||
}
|
||||
}
|
||||
reviews(first: 100) {
|
||||
nodes {
|
||||
id
|
||||
databaseId
|
||||
author { __typename login avatarUrl(size: 48) }
|
||||
body
|
||||
createdAt
|
||||
url
|
||||
reactionGroups {
|
||||
content
|
||||
viewerHasReacted
|
||||
reactors {
|
||||
totalCount
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}`
|
||||
@@ -4284,6 +4301,7 @@ export async function getPRComments(
|
||||
url: string
|
||||
reactionGroups?: GitHubGraphQLReactionGroup[] | null
|
||||
}
|
||||
let graphQLReviewSummaries: PRComment[] | undefined
|
||||
const reviewComments: PRComment[] = []
|
||||
if (threadsResult.status === 'fulfilled' && threadsResult.value) {
|
||||
const threadsData = JSON.parse(threadsResult.value.stdout) as {
|
||||
@@ -4292,6 +4310,7 @@ export async function getPRComments(
|
||||
pullRequest: {
|
||||
reviewThreads: { nodes: GQLThread[] }
|
||||
comments?: { nodes: GQLIssueComment[] }
|
||||
reviews?: { nodes: GQLIssueComment[] }
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -4313,6 +4332,21 @@ export async function getPRComments(
|
||||
if (graphQLIssueComments.length > 0) {
|
||||
issueComments = graphQLIssueComments
|
||||
}
|
||||
graphQLReviewSummaries = (pullRequest.reviews?.nodes ?? [])
|
||||
.filter((review) => review.body?.trim())
|
||||
.map(
|
||||
(review): PRComment => ({
|
||||
id: review.databaseId,
|
||||
author: review.author?.login ?? 'ghost',
|
||||
authorAvatarUrl: review.author?.avatarUrl ?? '',
|
||||
body: review.body,
|
||||
createdAt: review.createdAt,
|
||||
url: review.url,
|
||||
isBot: review.author?.__typename === 'Bot',
|
||||
reactionSubjectId: review.id,
|
||||
reactions: mapGraphQLReactionGroups(review.reactionGroups)
|
||||
})
|
||||
)
|
||||
|
||||
const threads = pullRequest.reviewThreads.nodes
|
||||
for (const thread of threads) {
|
||||
@@ -4353,7 +4387,9 @@ export async function getPRComments(
|
||||
html_url: string
|
||||
}
|
||||
let reviewSummaries: PRComment[] = []
|
||||
if (reviewsResult.status === 'fulfilled') {
|
||||
if (graphQLReviewSummaries) {
|
||||
reviewSummaries = graphQLReviewSummaries
|
||||
} else if (reviewsResult.status === 'fulfilled') {
|
||||
reviewSummaries = (JSON.parse(reviewsResult.value.stdout) as RESTReview[])
|
||||
.filter((r) => r.body?.trim())
|
||||
.map(
|
||||
|
||||
@@ -37,9 +37,15 @@ export function CommentReactions({
|
||||
}: {
|
||||
reactions?: GitHubReaction[]
|
||||
className?: string
|
||||
onReactionChange?: (content: GitHubReactionContent, reacted: boolean) => Promise<void> | void
|
||||
onReactionChange?: (
|
||||
content: GitHubReactionContent,
|
||||
reacted: boolean
|
||||
) => Promise<boolean> | boolean
|
||||
}): React.JSX.Element | null {
|
||||
const visibleReactions = (reactions ?? []).filter((reaction) => reaction.count > 0)
|
||||
const addReactionButtonRef = React.useRef<HTMLButtonElement>(null)
|
||||
const pickerGroupRef = React.useRef<HTMLDivElement>(null)
|
||||
const mutationPendingRef = React.useRef(false)
|
||||
const [open, setOpen] = React.useState(false)
|
||||
const [pendingContent, setPendingContent] = React.useState<GitHubReactionContent | null>(null)
|
||||
if (visibleReactions.length === 0 && !onReactionChange) {
|
||||
@@ -48,16 +54,25 @@ export function CommentReactions({
|
||||
|
||||
const changeReaction = async (
|
||||
content: GitHubReactionContent,
|
||||
reacted: boolean
|
||||
reacted: boolean,
|
||||
closePicker: boolean,
|
||||
focusTriggerAfterChange = false
|
||||
): Promise<void> => {
|
||||
if (!onReactionChange || pendingContent) {
|
||||
if (!onReactionChange || mutationPendingRef.current) {
|
||||
return
|
||||
}
|
||||
mutationPendingRef.current = true
|
||||
setPendingContent(content)
|
||||
setOpen(false)
|
||||
if (focusTriggerAfterChange) {
|
||||
addReactionButtonRef.current?.focus()
|
||||
}
|
||||
try {
|
||||
await onReactionChange(content, reacted)
|
||||
const changed = await onReactionChange(content, reacted)
|
||||
if (changed && closePicker) {
|
||||
setOpen(false)
|
||||
}
|
||||
} finally {
|
||||
mutationPendingRef.current = false
|
||||
setPendingContent(null)
|
||||
}
|
||||
}
|
||||
@@ -76,7 +91,7 @@ export function CommentReactions({
|
||||
type="button"
|
||||
variant="outline"
|
||||
size="xs"
|
||||
disabled={pendingContent !== null}
|
||||
aria-disabled={pendingContent !== null}
|
||||
className={cn(
|
||||
'h-6 gap-1 rounded-full px-2 text-[12px] font-normal',
|
||||
reaction.viewerHasReacted && 'border-ring bg-accent text-accent-foreground'
|
||||
@@ -91,7 +106,14 @@ export function CommentReactions({
|
||||
value2: reaction.count === 1 ? '' : 's'
|
||||
}
|
||||
)}
|
||||
onClick={() => void changeReaction(reaction.content, !reaction.viewerHasReacted)}
|
||||
onClick={() =>
|
||||
void changeReaction(
|
||||
reaction.content,
|
||||
!reaction.viewerHasReacted,
|
||||
false,
|
||||
Boolean(reaction.viewerHasReacted && reaction.count === 1)
|
||||
)
|
||||
}
|
||||
>
|
||||
<span aria-hidden="true">{REACTION_EMOJI[reaction.content]}</span>
|
||||
<span className="tabular-nums">{reaction.count}</span>
|
||||
@@ -121,10 +143,11 @@ export function CommentReactions({
|
||||
<TooltipTrigger asChild>
|
||||
<PopoverTrigger asChild>
|
||||
<Button
|
||||
ref={addReactionButtonRef}
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-xs"
|
||||
disabled={pendingContent !== null}
|
||||
aria-disabled={pendingContent !== null}
|
||||
className="text-muted-foreground hover:text-foreground"
|
||||
aria-label={pickerLabel}
|
||||
>
|
||||
@@ -136,8 +159,23 @@ export function CommentReactions({
|
||||
{pickerLabel}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
<PopoverContent align="start" side="top" sideOffset={6} className="w-auto p-1.5">
|
||||
<div className="grid grid-cols-4 gap-1" aria-label={pickerLabel} role="group">
|
||||
<PopoverContent
|
||||
align="start"
|
||||
side="top"
|
||||
sideOffset={6}
|
||||
className="w-auto p-1.5"
|
||||
onOpenAutoFocus={(event) => {
|
||||
event.preventDefault()
|
||||
pickerGroupRef.current?.focus()
|
||||
}}
|
||||
>
|
||||
<div
|
||||
ref={pickerGroupRef}
|
||||
className="grid grid-cols-4 gap-1"
|
||||
aria-label={pickerLabel}
|
||||
role="group"
|
||||
tabIndex={-1}
|
||||
>
|
||||
{GITHUB_REACTION_ORDER.map((content) => {
|
||||
const reaction = reactions?.find((candidate) => candidate.content === content)
|
||||
const reacted = Boolean(reaction?.viewerHasReacted)
|
||||
@@ -147,7 +185,7 @@ export function CommentReactions({
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-sm"
|
||||
disabled={pendingContent !== null}
|
||||
aria-disabled={pendingContent !== null}
|
||||
className={cn('text-lg', reacted && 'bg-accent text-accent-foreground')}
|
||||
aria-label={
|
||||
reacted
|
||||
@@ -163,7 +201,7 @@ export function CommentReactions({
|
||||
)
|
||||
}
|
||||
aria-pressed={reacted}
|
||||
onClick={() => void changeReaction(content, !reacted)}
|
||||
onClick={() => void changeReaction(content, !reacted, true)}
|
||||
>
|
||||
<span aria-hidden="true">{REACTION_EMOJI[content]}</span>
|
||||
</Button>
|
||||
|
||||
@@ -2945,10 +2945,14 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
)
|
||||
|
||||
const handleSetReaction = useCallback(
|
||||
async (comment: PRComment, content: GitHubReactionContent, reacted: boolean): Promise<void> => {
|
||||
async (
|
||||
comment: PRComment,
|
||||
content: GitHubReactionContent,
|
||||
reacted: boolean
|
||||
): Promise<boolean> => {
|
||||
const reactionSubjectId = comment.reactionSubjectId
|
||||
if (!repo || !prNumber || !pr?.prRepo || !reactionSubjectId) {
|
||||
return
|
||||
return false
|
||||
}
|
||||
const requestKey = checksPanelAsyncResultKey(
|
||||
prCacheKey,
|
||||
@@ -2968,7 +2972,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
{ repoId: repo.id, prRepo: pr.prRepo }
|
||||
)
|
||||
if (!isCurrentAsyncResult(requestKey) || ok) {
|
||||
return
|
||||
return ok
|
||||
}
|
||||
setComments((current) =>
|
||||
restoreReactionOnSubject(current, reactionSubjectId, content, previousReaction)
|
||||
@@ -2979,6 +2983,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
'Failed to update reaction.'
|
||||
)
|
||||
)
|
||||
return false
|
||||
},
|
||||
[branch, isCurrentAsyncResult, pr, prCacheKey, prNumber, repo, setPRCommentReaction]
|
||||
)
|
||||
|
||||
@@ -1756,7 +1756,7 @@ function CommentRow({
|
||||
comment: PRComment,
|
||||
content: GitHubReactionContent,
|
||||
reacted: boolean
|
||||
) => Promise<void>
|
||||
) => Promise<boolean>
|
||||
onQueueForAgent?: () => void
|
||||
}): React.JSX.Element {
|
||||
const automated = isBotPRComment(comment, botAuthorOverrides)
|
||||
@@ -2070,7 +2070,7 @@ function PRCommentGroupView({
|
||||
comment: PRComment,
|
||||
content: GitHubReactionContent,
|
||||
reacted: boolean
|
||||
) => Promise<void>
|
||||
) => Promise<boolean>
|
||||
onQueueForAgent?: () => void
|
||||
}): React.JSX.Element {
|
||||
// Reply targets a specific comment id so any comment in a thread — root or
|
||||
@@ -2219,7 +2219,7 @@ function ResolvedCommentGroupsSection({
|
||||
comment: PRComment,
|
||||
content: GitHubReactionContent,
|
||||
reacted: boolean
|
||||
) => Promise<void>
|
||||
) => Promise<boolean>
|
||||
}): React.JSX.Element | null {
|
||||
if (groups.length === 0) {
|
||||
return null
|
||||
@@ -2344,7 +2344,7 @@ export function PRCommentsList({
|
||||
comment: PRComment,
|
||||
content: GitHubReactionContent,
|
||||
reacted: boolean
|
||||
) => Promise<void>
|
||||
) => Promise<boolean>
|
||||
}): React.JSX.Element {
|
||||
const presentation = React.useMemo(() => getPRCommentPresentationClasses(), [])
|
||||
const [commentFilter, setCommentFilter] = useState<PRCommentAudienceFilter>('all')
|
||||
|
||||
@@ -10,7 +10,7 @@ export type PRCommentsSidebarSeed = {
|
||||
export const FIXTURE_COMMENTS: PRComment[] = [
|
||||
{
|
||||
id: 101,
|
||||
author: 'alice',
|
||||
author: 'coderabbitai',
|
||||
authorAvatarUrl: '',
|
||||
body: 'Please update this handler before merge.',
|
||||
createdAt: '2026-05-14T10:00:00.000Z',
|
||||
@@ -18,6 +18,7 @@ export const FIXTURE_COMMENTS: PRComment[] = [
|
||||
reactionSubjectId: 'PRRC_101',
|
||||
threadId: 'thread-open',
|
||||
path: 'src/handler.ts',
|
||||
isBot: true,
|
||||
isResolved: false
|
||||
},
|
||||
{
|
||||
|
||||
@@ -57,7 +57,7 @@ test.describe('PR comments sidebar cards view', () => {
|
||||
|
||||
await expect(orcaPage.getByText('Needs review · 1')).toBeVisible()
|
||||
await expect(orcaPage.getByText('Please update this handler before merge.')).toBeVisible()
|
||||
await expect(orcaPage.getByText('alice')).toBeVisible()
|
||||
await expect(orcaPage.getByText('coderabbitai')).toBeVisible()
|
||||
await expect(orcaPage.getByText('LGTM on the overall approach.')).toBeVisible()
|
||||
|
||||
const openThreadCard = orcaPage.getByTestId('pr-comment-group').filter({
|
||||
@@ -75,7 +75,7 @@ test.describe('PR comments sidebar cards view', () => {
|
||||
'Please update this handler before merge.',
|
||||
'LGTM on the overall approach.'
|
||||
)
|
||||
await expectOpenTextNotShiftedLeft(openThreadCard, conversationCard, 'alice', 'bob')
|
||||
await expectOpenTextNotShiftedLeft(openThreadCard, conversationCard, 'coderabbitai', 'bob')
|
||||
|
||||
const resolvedTrigger = orcaPage.getByRole('button', { name: 'Resolved · 1' })
|
||||
await expect(resolvedTrigger).toBeVisible()
|
||||
@@ -124,37 +124,106 @@ test.describe('PR comments sidebar cards view', () => {
|
||||
expect(positions[1]).toBeLessThan(positions[2])
|
||||
})
|
||||
|
||||
test('adds reactions to conversation and review-thread comments', async ({ orcaPage }) => {
|
||||
test('adds reactions to conversation and review-thread comments', async ({
|
||||
orcaPage
|
||||
}, testInfo) => {
|
||||
const { worktreeId } = await seedPRCommentsSidebarFixture(orcaPage)
|
||||
await openChecks(orcaPage, worktreeId)
|
||||
await expect(orcaPage.getByText('Needs review · 1')).toBeVisible({ timeout: 10_000 })
|
||||
|
||||
const reviewThreadCard = orcaPage.getByTestId('pr-comment-group').filter({
|
||||
hasText: 'Please update this handler before merge.'
|
||||
})
|
||||
const threadReactionButton = reviewThreadCard.getByRole('button', { name: 'Add reaction' })
|
||||
await orcaPage.screenshot({ path: testInfo.outputPath('reaction-before.png') })
|
||||
await threadReactionButton.click()
|
||||
await expect(orcaPage.getByRole('group', { name: 'Add reaction' })).toBeFocused()
|
||||
await orcaPage.waitForTimeout(300)
|
||||
await orcaPage.screenshot({ path: testInfo.outputPath('reaction-picker.png') })
|
||||
await orcaPage.getByRole('button', { name: 'Add rocket reaction' }).click()
|
||||
await expect(orcaPage.getByRole('group', { name: 'Add reaction' })).toBeHidden()
|
||||
const selectedRocket = reviewThreadCard.getByRole('button', { name: '1 rocket reaction' })
|
||||
await expect(selectedRocket).toHaveAttribute('aria-pressed', 'true')
|
||||
await selectedRocket.focus()
|
||||
await orcaPage.waitForTimeout(300)
|
||||
await orcaPage.screenshot({ path: testInfo.outputPath('reaction-after.png') })
|
||||
await selectedRocket.press('Enter')
|
||||
await expect(selectedRocket).toHaveCount(0)
|
||||
await expect(threadReactionButton).toBeFocused()
|
||||
|
||||
const conversationCard = orcaPage.getByTestId('pr-comment-group').filter({
|
||||
hasText: 'LGTM on the overall approach.'
|
||||
})
|
||||
const conversationReactionButton = conversationCard.getByRole('button', {
|
||||
name: 'Add reaction'
|
||||
})
|
||||
await conversationReactionButton.evaluate((element) => (element as HTMLElement).click())
|
||||
const heartReactionButton = orcaPage.getByRole('button', { name: 'Add heart reaction' })
|
||||
await conversationReactionButton.click()
|
||||
const conversationPicker = orcaPage.getByRole('group', { name: 'Add reaction' }).last()
|
||||
const heartReactionButton = conversationPicker.getByRole('button', {
|
||||
name: 'Add heart reaction'
|
||||
})
|
||||
await expect(heartReactionButton).toBeVisible()
|
||||
await heartReactionButton.evaluate((element) => (element as HTMLElement).click())
|
||||
await expect(
|
||||
conversationCard.getByRole('button', { name: '1 heart reaction' })
|
||||
).toHaveAttribute('aria-pressed', 'true')
|
||||
await expect(orcaPage.getByRole('button', { name: 'Add rocket reaction' })).toHaveCount(0)
|
||||
})
|
||||
|
||||
test('keeps reaction focus while a remote mutation fails', async ({ orcaPage }) => {
|
||||
const { worktreeId } = await seedPRCommentsSidebarFixture(orcaPage)
|
||||
await openChecks(orcaPage, worktreeId)
|
||||
await expect(orcaPage.getByText('Needs review · 1')).toBeVisible({ timeout: 10_000 })
|
||||
await orcaPage.evaluate(() => {
|
||||
window.__store?.setState({
|
||||
setPRCommentReaction: async () => {
|
||||
await new Promise((resolve) => window.setTimeout(resolve, 300))
|
||||
return false
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
const reviewThreadCard = orcaPage.getByTestId('pr-comment-group').filter({
|
||||
hasText: 'Please update this handler before merge.'
|
||||
})
|
||||
const threadReactionButton = reviewThreadCard.getByRole('button', { name: 'Add reaction' })
|
||||
await threadReactionButton.evaluate((element) => (element as HTMLElement).click())
|
||||
await orcaPage
|
||||
.getByRole('button', { name: 'Add rocket reaction' })
|
||||
.evaluate((element) => (element as HTMLElement).click())
|
||||
await expect(
|
||||
reviewThreadCard.getByRole('button', { name: '1 rocket reaction' })
|
||||
).toHaveAttribute('aria-pressed', 'true')
|
||||
const addReaction = reviewThreadCard.getByRole('button', { name: 'Add reaction' })
|
||||
await addReaction.focus()
|
||||
await addReaction.press('Enter')
|
||||
const picker = orcaPage.getByRole('group', { name: 'Add reaction' })
|
||||
await expect(picker).toBeFocused()
|
||||
const rocket = picker.getByRole('button', { name: /rocket reaction/ })
|
||||
await rocket.focus()
|
||||
await rocket.press('Enter')
|
||||
await expect(rocket).toBeFocused()
|
||||
await expect(rocket).toHaveAttribute('aria-disabled', 'true')
|
||||
await rocket.press('Enter')
|
||||
await expect(picker).toBeVisible()
|
||||
await expect(rocket).toHaveAttribute('aria-disabled', 'false')
|
||||
await expect(rocket).toBeFocused()
|
||||
await expect(rocket).toHaveAccessibleName('Add rocket reaction')
|
||||
|
||||
await orcaPage.evaluate(() => {
|
||||
window.__store?.setState({ setPRCommentReaction: async () => true })
|
||||
})
|
||||
await rocket.press('Enter')
|
||||
const selectedRocket = reviewThreadCard.getByRole('button', { name: '1 rocket reaction' })
|
||||
await expect(selectedRocket).toHaveAttribute('aria-pressed', 'true')
|
||||
await orcaPage.evaluate(() => {
|
||||
window.__store?.setState({
|
||||
setPRCommentReaction: async () => {
|
||||
await new Promise((resolve) => window.setTimeout(resolve, 300))
|
||||
return false
|
||||
}
|
||||
})
|
||||
})
|
||||
await selectedRocket.focus()
|
||||
await selectedRocket.press('Enter')
|
||||
await expect(addReaction).toBeFocused()
|
||||
await expect(addReaction).toHaveAttribute('aria-disabled', 'true')
|
||||
await expect(selectedRocket).toHaveCount(0)
|
||||
await expect(selectedRocket).toHaveCount(1)
|
||||
await expect(addReaction).toHaveAttribute('aria-disabled', 'false')
|
||||
await expect(addReaction).toBeFocused()
|
||||
})
|
||||
|
||||
test('queues an open thread for the agent from the visible row action and menu fallback', async ({
|
||||
|
||||
Reference in New Issue
Block a user