mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 00:03:46 +00:00
fix(github): harden comment reaction updates
This commit is contained in:
@@ -225,10 +225,14 @@ describe('GitHub PR local runtime routing', () => {
|
||||
return { stdout: '[]' }
|
||||
}
|
||||
if (endpoint.endsWith('/pulls/7/comments/11/replies')) {
|
||||
return { stdout: JSON.stringify({ id: 12, user: null, body: 'Reply' }) }
|
||||
return {
|
||||
stdout: JSON.stringify({ id: 12, node_id: 'PRRC_reply_12', user: null, body: 'Reply' })
|
||||
}
|
||||
}
|
||||
if (endpoint.endsWith('/pulls/7/comments')) {
|
||||
return { stdout: JSON.stringify({ id: 13, user: null, body: 'Inline' }) }
|
||||
return {
|
||||
stdout: JSON.stringify({ id: 13, node_id: 'PRRC_inline_13', user: null, body: 'Inline' })
|
||||
}
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
@@ -250,7 +254,10 @@ describe('GitHub PR local runtime routing', () => {
|
||||
prRepo,
|
||||
localGitOptions
|
||||
)
|
||||
).resolves.toMatchObject({ ok: true })
|
||||
).resolves.toMatchObject({
|
||||
ok: true,
|
||||
comment: { reactionSubjectId: 'PRRC_reply_12' }
|
||||
})
|
||||
await expect(
|
||||
addPRReviewComment({
|
||||
repoPath: '/repo-root',
|
||||
@@ -263,7 +270,10 @@ describe('GitHub PR local runtime routing', () => {
|
||||
path: 'src/app.ts',
|
||||
line: 10
|
||||
})
|
||||
).resolves.toMatchObject({ ok: true })
|
||||
).resolves.toMatchObject({
|
||||
ok: true,
|
||||
comment: { reactionSubjectId: 'PRRC_inline_13' }
|
||||
})
|
||||
await expect(
|
||||
updatePRTitle('/repo-root', 7, 'New title', null, prRepo, localGitOptions)
|
||||
).resolves.toBe(true)
|
||||
|
||||
@@ -4561,6 +4561,7 @@ export async function resolveReviewThread(
|
||||
function mapReviewCommentResponse(
|
||||
data: {
|
||||
id?: number
|
||||
node_id?: string | null
|
||||
user: { login: string; avatar_url: string; type?: string } | null
|
||||
body?: string
|
||||
created_at?: string
|
||||
@@ -4576,6 +4577,7 @@ function mapReviewCommentResponse(
|
||||
): PRComment {
|
||||
return {
|
||||
id: data.id ?? Date.now(),
|
||||
reactionSubjectId: data.node_id?.trim() || undefined,
|
||||
author: data.user?.login ?? 'You',
|
||||
authorAvatarUrl: data.user?.avatar_url ?? '',
|
||||
body: data.body ?? body,
|
||||
|
||||
@@ -195,6 +195,7 @@ describe('issue source operations', () => {
|
||||
ghExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
id: 9,
|
||||
node_id: 'IC_enterprise_9',
|
||||
user: { login: 'octo', avatar_url: '', type: 'User' },
|
||||
body: 'Enterprise comment',
|
||||
created_at: '2026-07-16T00:00:00.000Z',
|
||||
@@ -208,7 +209,10 @@ describe('issue source operations', () => {
|
||||
repo: 'orca',
|
||||
host: 'github.acme-corp.com'
|
||||
})
|
||||
).resolves.toMatchObject({ ok: true })
|
||||
).resolves.toMatchObject({
|
||||
ok: true,
|
||||
comment: { reactionSubjectId: 'IC_enterprise_9' }
|
||||
})
|
||||
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
|
||||
[
|
||||
|
||||
@@ -473,6 +473,7 @@ export async function addIssueComment(
|
||||
)
|
||||
const data = JSON.parse(stdout) as {
|
||||
id?: number
|
||||
node_id?: string | null
|
||||
user: { login: string; avatar_url: string; type?: string } | null
|
||||
body?: string
|
||||
created_at?: string
|
||||
@@ -483,6 +484,7 @@ export async function addIssueComment(
|
||||
}
|
||||
const comment: PRComment = {
|
||||
id: data.id,
|
||||
reactionSubjectId: data.node_id?.trim() || undefined,
|
||||
author: data.user?.login ?? 'You',
|
||||
authorAvatarUrl: data.user?.avatar_url ?? '',
|
||||
body: data.body ?? body,
|
||||
|
||||
@@ -25,7 +25,7 @@ import { useActiveWorktree, useRepoById } from '@/store/selectors'
|
||||
import { useChecksPanelTerminalWorktree } from './use-checks-panel-terminal-worktree'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { openHttpLink } from '@/lib/http-link-routing'
|
||||
import { setReactionOnSubject } from '@/lib/pr-comment-reactions'
|
||||
import { restoreReactionOnSubject, setReactionOnSubject } from '@/lib/pr-comment-reactions'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { DetachedHeadBadge } from '@/components/DetachedHeadBadge'
|
||||
import {
|
||||
@@ -2957,7 +2957,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
pr.prRepo,
|
||||
pr.headSha
|
||||
)
|
||||
const previousReactions = comment.reactions
|
||||
const previousReaction = comment.reactions?.find((reaction) => reaction.content === content)
|
||||
setComments((current) => setReactionOnSubject(current, reactionSubjectId, content, reacted))
|
||||
const ok = await setPRCommentReaction(
|
||||
repo.path,
|
||||
@@ -2971,11 +2971,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
return
|
||||
}
|
||||
setComments((current) =>
|
||||
current.map((entry) =>
|
||||
entry.reactionSubjectId === reactionSubjectId
|
||||
? { ...entry, reactions: previousReactions }
|
||||
: entry
|
||||
)
|
||||
restoreReactionOnSubject(current, reactionSubjectId, content, previousReaction)
|
||||
)
|
||||
toast.error(
|
||||
translate(
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { PRComment } from '../../../shared/types'
|
||||
import { setCommentReaction, setReactionOnSubject } from './pr-comment-reactions'
|
||||
import {
|
||||
restoreCommentReaction,
|
||||
setCommentReaction,
|
||||
setReactionOnSubject
|
||||
} from './pr-comment-reactions'
|
||||
|
||||
function comment(overrides: Partial<PRComment> = {}): PRComment {
|
||||
return {
|
||||
@@ -46,4 +50,22 @@ describe('setCommentReaction', () => {
|
||||
expect(result[0].reactions).toEqual([{ content: 'eyes', count: 1, viewerHasReacted: true }])
|
||||
expect(result[1]).toBe(other)
|
||||
})
|
||||
|
||||
it('restores only the failed reaction while preserving concurrent updates', () => {
|
||||
const result = restoreCommentReaction(
|
||||
comment({
|
||||
reactions: [
|
||||
{ content: 'heart', count: 3, viewerHasReacted: true },
|
||||
{ content: 'eyes', count: 1, viewerHasReacted: false }
|
||||
]
|
||||
}),
|
||||
'heart',
|
||||
{ content: 'heart', count: 2, viewerHasReacted: false }
|
||||
)
|
||||
|
||||
expect(result.reactions).toEqual([
|
||||
{ content: 'heart', count: 2, viewerHasReacted: false },
|
||||
{ content: 'eyes', count: 1, viewerHasReacted: false }
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -11,6 +11,13 @@ export const GITHUB_REACTION_ORDER: readonly GitHubReactionContent[] = [
|
||||
'eyes'
|
||||
]
|
||||
|
||||
function sortReactions(reactions: GitHubReaction[]): GitHubReaction[] {
|
||||
return reactions.sort(
|
||||
(left, right) =>
|
||||
GITHUB_REACTION_ORDER.indexOf(left.content) - GITHUB_REACTION_ORDER.indexOf(right.content)
|
||||
)
|
||||
}
|
||||
|
||||
export function setCommentReaction(
|
||||
comment: PRComment,
|
||||
content: GitHubReactionContent,
|
||||
@@ -28,17 +35,27 @@ export function setCommentReaction(
|
||||
count: nextCount,
|
||||
viewerHasReacted: reacted
|
||||
}
|
||||
const nextReactions = reactions
|
||||
.filter((reaction) => reaction.content !== content)
|
||||
.concat(nextCount > 0 ? nextReaction : [])
|
||||
.sort(
|
||||
(left, right) =>
|
||||
GITHUB_REACTION_ORDER.indexOf(left.content) - GITHUB_REACTION_ORDER.indexOf(right.content)
|
||||
)
|
||||
const nextReactions = sortReactions(
|
||||
reactions
|
||||
.filter((reaction) => reaction.content !== content)
|
||||
.concat(nextCount > 0 ? nextReaction : [])
|
||||
)
|
||||
|
||||
return { ...comment, reactions: nextReactions.length > 0 ? nextReactions : undefined }
|
||||
}
|
||||
|
||||
export function restoreCommentReaction(
|
||||
comment: PRComment,
|
||||
content: GitHubReactionContent,
|
||||
previousReaction?: GitHubReaction
|
||||
): PRComment {
|
||||
const reactions = sortReactions([
|
||||
...(comment.reactions ?? []).filter((reaction) => reaction.content !== content),
|
||||
...(previousReaction ? [previousReaction] : [])
|
||||
])
|
||||
return { ...comment, reactions: reactions.length > 0 ? reactions : undefined }
|
||||
}
|
||||
|
||||
export function setReactionOnSubject(
|
||||
comments: readonly PRComment[],
|
||||
reactionSubjectId: string,
|
||||
@@ -51,3 +68,16 @@ export function setReactionOnSubject(
|
||||
: comment
|
||||
)
|
||||
}
|
||||
|
||||
export function restoreReactionOnSubject(
|
||||
comments: readonly PRComment[],
|
||||
reactionSubjectId: string,
|
||||
content: GitHubReactionContent,
|
||||
previousReaction?: GitHubReaction
|
||||
): PRComment[] {
|
||||
return comments.map((comment) =>
|
||||
comment.reactionSubjectId === reactionSubjectId
|
||||
? restoreCommentReaction(comment, content, previousReaction)
|
||||
: comment
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1713,7 +1713,10 @@ describe('createGitHubSlice PR comment mutations', () => {
|
||||
})
|
||||
|
||||
it('rolls back an optimistic reaction when GitHub rejects it', async () => {
|
||||
mockApi.gh.setPRCommentReaction.mockResolvedValueOnce(false)
|
||||
let resolveReaction!: (ok: boolean) => void
|
||||
mockApi.gh.setPRCommentReaction.mockImplementationOnce(
|
||||
() => new Promise<boolean>((resolve) => (resolveReaction = resolve))
|
||||
)
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
const repoId = 'repo-id'
|
||||
@@ -1739,14 +1742,33 @@ describe('createGitHubSlice PR comment mutations', () => {
|
||||
}
|
||||
} as unknown as Partial<AppState>)
|
||||
|
||||
const ok = await store.getState().setPRCommentReaction(repoPath, 12, 'IC_10', 'heart', true, {
|
||||
const pending = store.getState().setPRCommentReaction(repoPath, 12, 'IC_10', 'heart', true, {
|
||||
repoId,
|
||||
prRepo: { owner: 'Acme', repo: 'Widgets' }
|
||||
})
|
||||
const optimisticEntry = store.getState().commentsCache[cacheKey]
|
||||
store.setState({
|
||||
commentsCache: {
|
||||
...store.getState().commentsCache,
|
||||
[cacheKey]: {
|
||||
...optimisticEntry,
|
||||
data: (optimisticEntry?.data ?? []).map((comment) => ({
|
||||
...comment,
|
||||
reactions: [
|
||||
...(comment.reactions ?? []),
|
||||
{ content: 'eyes' as const, count: 1, viewerHasReacted: false }
|
||||
]
|
||||
}))
|
||||
}
|
||||
}
|
||||
})
|
||||
resolveReaction(false)
|
||||
const ok = await pending
|
||||
|
||||
expect(ok).toBe(false)
|
||||
expect(store.getState().commentsCache[cacheKey]?.data?.[0].reactions).toEqual([
|
||||
{ content: 'heart', count: 2, viewerHasReacted: false }
|
||||
{ content: 'heart', count: 2, viewerHasReacted: false },
|
||||
{ content: 'eyes', count: 1, viewerHasReacted: false }
|
||||
])
|
||||
})
|
||||
|
||||
|
||||
@@ -75,7 +75,7 @@ import {
|
||||
type TaskSourceContext
|
||||
} from '../../../../shared/task-source-context'
|
||||
import { normalizeGitHubPRForBranchOutcome } from '../../../../shared/github-pr-for-branch-outcome'
|
||||
import { setReactionOnSubject } from '@/lib/pr-comment-reactions'
|
||||
import { restoreReactionOnSubject, setReactionOnSubject } from '@/lib/pr-comment-reactions'
|
||||
|
||||
// ─── ProjectV2 cache types ────────────────────────────────────────────
|
||||
// Why: separate from CacheEntry<T> — project-view has a single GraphQL source (no issue/PR fallback) and a distinct error union.
|
||||
@@ -3879,6 +3879,9 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
||||
const previousComment = get().commentsCache[cacheKey]?.data?.find(
|
||||
(comment) => comment.reactionSubjectId === reactionSubjectId
|
||||
)
|
||||
const previousReaction = previousComment?.reactions?.find(
|
||||
(reaction) => reaction.content === content
|
||||
)
|
||||
set((state) => {
|
||||
const entry = state.commentsCache[cacheKey]
|
||||
if (!entry?.data) {
|
||||
@@ -3941,10 +3944,11 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
||||
...state.commentsCache,
|
||||
[cacheKey]: {
|
||||
...entry,
|
||||
data: entry.data.map((comment) =>
|
||||
comment.reactionSubjectId === reactionSubjectId
|
||||
? { ...comment, reactions: previousComment.reactions }
|
||||
: comment
|
||||
data: restoreReactionOnSubject(
|
||||
entry.data,
|
||||
reactionSubjectId,
|
||||
content,
|
||||
previousReaction
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,22 +31,30 @@ export async function openChecks(page: Page, worktreeId: string): Promise<void>
|
||||
const state = window.__store?.getState()
|
||||
state?.setActiveWorktree(targetWorktreeId)
|
||||
state?.setRightSidebarOpen(true)
|
||||
state?.setRightSidebarTab('checks')
|
||||
}, worktreeId)
|
||||
await expect
|
||||
.poll(
|
||||
() =>
|
||||
page.evaluate((targetWorktreeId) => {
|
||||
const state = window.__store?.getState()
|
||||
return (
|
||||
state?.activeWorktreeId === targetWorktreeId &&
|
||||
state.rightSidebarOpen &&
|
||||
state.rightSidebarTab === 'checks'
|
||||
)
|
||||
return state?.activeWorktreeId === targetWorktreeId && state.rightSidebarOpen
|
||||
}, worktreeId),
|
||||
{ timeout: 5_000 }
|
||||
)
|
||||
.toBe(true)
|
||||
const checksButton = page.getByRole('button', { name: 'Checks', exact: true })
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
if ((await page.evaluate(() => window.__store?.getState().rightSidebarTab)) !== 'checks') {
|
||||
await checksButton.click()
|
||||
}
|
||||
await page.waitForTimeout(250)
|
||||
return page.evaluate(() => window.__store?.getState().rightSidebarTab)
|
||||
},
|
||||
{ timeout: 10_000 }
|
||||
)
|
||||
.toBe('checks')
|
||||
}
|
||||
|
||||
export async function seedCreatePrComposer(page: Page): Promise<{
|
||||
|
||||
Reference in New Issue
Block a user