Clarify conflict details refresh state (#1875)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
7465fda785
commit
dcf5bf0dce
|
|
@ -50,7 +50,7 @@ import CommentMarkdown from '@/components/sidebar/CommentMarkdown'
|
|||
import { detectLanguage } from '@/lib/language-detect'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { buildDiffTree, type DiffTreeNode } from '@/components/pr-diff-tree'
|
||||
import { CHECK_COLOR, CHECK_ICON } from '@/components/right-sidebar/checks-helpers'
|
||||
import { CHECK_COLOR, CHECK_ICON } from '@/components/right-sidebar/checks-panel-content'
|
||||
import {
|
||||
filterPRCommentsByAudience,
|
||||
getPRCommentAudienceCounts,
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ import {
|
|||
MergeConflictNotice,
|
||||
ChecksList,
|
||||
PRCommentsList
|
||||
} from './checks-helpers'
|
||||
} from './checks-panel-content'
|
||||
import { ENTRY_REFRESH_GRACE_MS, shouldEntryRefresh } from './checks-entry-refresh'
|
||||
import type { PRInfo, PRCheckDetail, PRComment } from '../../../../shared/types'
|
||||
|
||||
|
|
@ -43,6 +43,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
const [commentsLoading, setCommentsLoading] = useState(false)
|
||||
const [emptyRefreshing, setEmptyRefreshing] = useState(false)
|
||||
const [isRefreshing, setIsRefreshing] = useState(false)
|
||||
const [conflictDetailsRefreshing, setConflictDetailsRefreshing] = useState(false)
|
||||
const [editingTitle, setEditingTitle] = useState(false)
|
||||
const [titleDraft, setTitleDraft] = useState('')
|
||||
const [titleSaving, setTitleSaving] = useState(false)
|
||||
|
|
@ -67,6 +68,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
setTitleSaving(false)
|
||||
setIsRefreshing(false)
|
||||
setEmptyRefreshing(false)
|
||||
setConflictDetailsRefreshing(false)
|
||||
conflictSummaryRefreshKeyRef.current = null
|
||||
}
|
||||
|
||||
|
|
@ -108,6 +110,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
useEffect(() => {
|
||||
if (!repo || isFolder || !branch || !pr || pr.mergeable !== 'CONFLICTING') {
|
||||
conflictSummaryRefreshKeyRef.current = null
|
||||
setConflictDetailsRefreshing(false)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -121,7 +124,17 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
// them so we don't keep rendering cached branch summaries or empty file
|
||||
// lists from an older payload.
|
||||
conflictSummaryRefreshKeyRef.current = refreshKey
|
||||
void fetchPRForBranch(repo.path, branch, { force: true, linkedPRNumber: linkedPR })
|
||||
setConflictDetailsRefreshing(true)
|
||||
void fetchPRForBranch(repo.path, branch, { force: true, linkedPRNumber: linkedPR }).finally(
|
||||
() => {
|
||||
// Why: fetchPRForBranch updates the PR cache before resolving, which
|
||||
// can rerun this effect. Only the current refresh key may clear the
|
||||
// spinner so stale requests don't race newer worktrees/branches.
|
||||
if (conflictSummaryRefreshKeyRef.current === refreshKey) {
|
||||
setConflictDetailsRefreshing(false)
|
||||
}
|
||||
}
|
||||
)
|
||||
}, [repo, isFolder, branch, pr, linkedPR, fetchPRForBranch])
|
||||
|
||||
// Fetch checks via cached store method
|
||||
|
|
@ -580,7 +593,10 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
</div>
|
||||
|
||||
<ConflictingFilesSection pr={pr} />
|
||||
<MergeConflictNotice pr={pr} />
|
||||
<MergeConflictNotice
|
||||
pr={pr}
|
||||
isRefreshingConflictDetails={isRefreshing || conflictDetailsRefreshing}
|
||||
/>
|
||||
{/* Why: when the PR has merge conflicts and no checks have been fetched,
|
||||
showing "No checks configured" is misleading — checks may exist but
|
||||
simply cannot run until conflicts are resolved. Hide the empty state. */}
|
||||
|
|
|
|||
|
|
@ -92,7 +92,7 @@ import {
|
|||
requestEditorSaveQuiesce
|
||||
} from '@/components/editor/editor-autosave'
|
||||
import { getConnectionId } from '@/lib/connection-context'
|
||||
import { PullRequestIcon } from './checks-helpers'
|
||||
import { PullRequestIcon } from './checks-panel-content'
|
||||
import type {
|
||||
DiffComment,
|
||||
GitBranchChangeEntry,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,54 @@
|
|||
import React from 'react'
|
||||
import { renderToStaticMarkup } from 'react-dom/server'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { PRInfo } from '../../../../shared/types'
|
||||
import { MergeConflictNotice } from './checks-panel-content'
|
||||
|
||||
function makePR(overrides: Partial<PRInfo> = {}): PRInfo {
|
||||
return {
|
||||
number: 42,
|
||||
title: 'Conflicting PR',
|
||||
state: 'open',
|
||||
url: 'https://github.com/acme/widgets/pull/42',
|
||||
checksStatus: 'pending',
|
||||
updatedAt: '2026-05-14T00:00:00Z',
|
||||
mergeable: 'CONFLICTING',
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
function renderNotice(pr: PRInfo, isRefreshingConflictDetails = false): string {
|
||||
return renderToStaticMarkup(
|
||||
React.createElement(MergeConflictNotice, { pr, isRefreshingConflictDetails })
|
||||
)
|
||||
}
|
||||
|
||||
describe('MergeConflictNotice', () => {
|
||||
it('does not claim conflict details are refreshing after the refresh has settled', () => {
|
||||
const markup = renderNotice(makePR())
|
||||
|
||||
expect(markup).toContain('Conflict file details are unavailable')
|
||||
expect(markup).not.toContain('Refreshing conflict details')
|
||||
})
|
||||
|
||||
it('shows refreshing copy while conflict details are actively refreshing', () => {
|
||||
const markup = renderNotice(makePR(), true)
|
||||
|
||||
expect(markup).toContain('Refreshing conflict details')
|
||||
})
|
||||
|
||||
it('hides when the conflicting file list is available', () => {
|
||||
const markup = renderNotice(
|
||||
makePR({
|
||||
conflictSummary: {
|
||||
baseRef: 'main',
|
||||
baseCommit: 'abc1234',
|
||||
commitsBehind: 2,
|
||||
files: ['src/conflict.ts']
|
||||
}
|
||||
})
|
||||
)
|
||||
|
||||
expect(markup).toBe('')
|
||||
})
|
||||
})
|
||||
|
|
@ -101,7 +101,13 @@ export function ConflictingFilesSection({ pr }: { pr: PRInfo }): React.JSX.Eleme
|
|||
}
|
||||
|
||||
/** Fallback shown when GitHub reports merge conflicts but no file list is available yet. */
|
||||
export function MergeConflictNotice({ pr }: { pr: PRInfo }): React.JSX.Element | null {
|
||||
export function MergeConflictNotice({
|
||||
pr,
|
||||
isRefreshingConflictDetails
|
||||
}: {
|
||||
pr: PRInfo
|
||||
isRefreshingConflictDetails: boolean
|
||||
}): React.JSX.Element | null {
|
||||
if (pr.mergeable !== 'CONFLICTING' || (pr.conflictSummary?.files.length ?? 0) > 0) {
|
||||
return null
|
||||
}
|
||||
|
|
@ -111,7 +117,11 @@ export function MergeConflictNotice({ pr }: { pr: PRInfo }): React.JSX.Element |
|
|||
<div className="text-[11px] font-medium text-foreground">
|
||||
This branch has conflicts that must be resolved
|
||||
</div>
|
||||
<div className="mt-1 text-[11px] text-muted-foreground">Refreshing conflict details…</div>
|
||||
<div className="mt-1 text-[11px] text-muted-foreground">
|
||||
{isRefreshingConflictDetails
|
||||
? 'Refreshing conflict details…'
|
||||
: 'Conflict file details are unavailable'}
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
Loading…
Reference in New Issue