Skip to content

Commit e378806

Browse files
committed
fix: keep timeline ordered and complete after submitting reviews and comments
Submitting a review inserted its event at the front of the ascending timeline, rendering it at the top of the conversation until the next refetch moved it into place, and the eventual-consistency fallback dropped the reviewer's previous review. Refetches after adding a comment, replying to a thread, editing/deleting, or deleting/restoring the branch could serve the stale cached timeline, silently dropping just-created items. Reviews are now inserted at the end (their real chronological spot) using the review identity returned by the submit mutation, and every refetch that must observe fresh data invalidates the cache first.
1 parent da904a8 commit e378806

4 files changed

Lines changed: 51 additions & 47 deletions

File tree

src/browser/components/pr-overview.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,9 @@ export const PROverview = memo(function PROverview() {
374374
author_association: newComment.author_association,
375375
actor: newComment.user!,
376376
} as TimelineEvent);
377+
// Invalidate so later timeline refetches can't serve the cached
378+
// pre-comment timeline and drop the local event.
379+
github.invalidatePR(owner, repo, pr.number);
377380
setCommentText("");
378381
} catch (error) {
379382
console.error("Failed to add comment:", error);
@@ -990,6 +993,8 @@ export const PROverview = memo(function PROverview() {
990993
await github.createPRComment(owner, repo, pr.number, body, {
991994
reply_to_id: commentId,
992995
});
996+
// Invalidate so the refetches below can't serve stale cached data
997+
github.invalidatePR(owner, repo, pr.number);
993998
// Refresh threads to show new comment
994999
const result = await github.getReviewThreads(owner, repo, pr.number);
9951000
store.setReviewThreads(result.threads);
@@ -1059,6 +1064,8 @@ export const PROverview = memo(function PROverview() {
10591064
);
10601065

10611066
const refreshConversation = useCallback(async () => {
1067+
// Invalidate so the refetch below can't serve the stale cached timeline
1068+
github.invalidatePR(owner, repo, pr.number);
10621069
const [newComments, newTimeline] = await Promise.all([
10631070
github.getPRComments(owner, repo, pr.number).catch(() => []),
10641071
github.getPRTimeline(owner, repo, pr.number).catch(() => []),

src/browser/contexts/github.tsx

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2267,12 +2267,22 @@ function createGitHubStore() {
22672267
reviewId: string,
22682268
event: "APPROVE" | "REQUEST_CHANGES" | "COMMENT",
22692269
body?: string
2270-
): Promise<void> {
2270+
): Promise<{ databaseId: number; submittedAt: string | null } | null> {
22712271
if (!batcher) throw new Error("Not initialized");
2272-
await batcher.query(
2273-
`mutation ($input: SubmitPullRequestReviewInput!) { submitPullRequestReview(input: $input) { pullRequestReview { id } } }`,
2272+
const result = await batcher.query<{
2273+
submitPullRequestReview: {
2274+
pullRequestReview: {
2275+
databaseId: number;
2276+
submittedAt: string | null;
2277+
} | null;
2278+
} | null;
2279+
}>(
2280+
`mutation ($input: SubmitPullRequestReviewInput!) { submitPullRequestReview(input: $input) { pullRequestReview { databaseId submittedAt } } }`,
22742281
{ input: { pullRequestReviewId: reviewId, event, body: body ?? "" } }
22752282
);
2283+
const review = result.submitPullRequestReview?.pullRequestReview;
2284+
if (!review) return null;
2285+
return { databaseId: review.databaseId, submittedAt: review.submittedAt };
22762286
}
22772287

22782288
async function getReviewReactions(reviewNodeId: string): Promise<Reaction[]> {

src/browser/contexts/pr-review/index.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3982,6 +3982,9 @@ export class PRReviewStore {
39823982
try {
39833983
await this.github.deleteBranch(owner, repo, pr.head.ref);
39843984

3985+
// Invalidate so the refetch below can't serve the stale cached timeline
3986+
this.invalidatePRCaches(owner, repo, pr.number);
3987+
39853988
// Refetch timeline to show delete event
39863989
const updatedTimeline = await this.github
39873990
.getPRTimeline(owner, repo, pr.number)
@@ -4012,6 +4015,9 @@ export class PRReviewStore {
40124015
try {
40134016
await this.github.restoreBranch(owner, repo, pr.head.ref, pr.head.sha);
40144017

4018+
// Invalidate so the refetch below can't serve the stale cached timeline
4019+
this.invalidatePRCaches(owner, repo, pr.number);
4020+
40154021
// Refetch timeline to show restore event
40164022
const updatedTimeline = await this.github
40174023
.getPRTimeline(owner, repo, pr.number)

src/browser/contexts/pr-review/useReviewActions.ts

Lines changed: 25 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,31 @@ export function useReviewActions() {
3333

3434
if (reviewNodeId) {
3535
try {
36-
// Submit via GraphQL - we'll find the review ID after refreshing
37-
await github.submitPendingReview(
36+
const submitted = await github.submitPendingReview(
3837
reviewNodeId,
3938
event,
4039
state.reviewBody
4140
);
41+
if (submitted) {
42+
// Build the review from server identity so the UI can show it
43+
// immediately, even if the refetch below is stale.
44+
newReview = {
45+
id: submitted.databaseId,
46+
user: currentUser
47+
? {
48+
login: currentUser,
49+
avatar_url: `https://avatars.githubusercontent.com/${currentUser}`,
50+
}
51+
: null,
52+
state:
53+
event === "APPROVE"
54+
? "APPROVED"
55+
: event === "REQUEST_CHANGES"
56+
? "CHANGES_REQUESTED"
57+
: "COMMENTED",
58+
submitted_at: submitted.submittedAt,
59+
} as Review;
60+
}
4261
submittedViaGraphQL = true;
4362
} catch {
4463
// GraphQL failed (e.g. pending review was already submitted).
@@ -96,53 +115,15 @@ export function useReviewActions() {
96115

97116
// If the review we just submitted isn't in the re-fetched data yet
98117
// (eventual consistency), add it manually so it appears immediately.
99-
const addReviewToArray = (review: Review) => {
100-
// Remove any stale review by the same user so the new one takes effect
101-
const existingIdx = reviews.findIndex(
102-
(r) => r.user?.login === review.user?.login
103-
);
104-
if (existingIdx !== -1) reviews.splice(existingIdx, 1);
105-
reviews.unshift(review);
106-
};
107-
118+
// The timeline is ascending, so a just-created review goes last.
108119
if (newReview?.id && !reviews.some((r) => r.id === newReview!.id)) {
109-
addReviewToArray(newReview);
110-
timeline.unshift({
120+
reviews.unshift(newReview);
121+
timeline.push({
111122
id: newReview.id,
112123
event: "reviewed",
113124
actor: { login: currentUser ?? "", avatar_url: "" },
114-
created_at: new Date().toISOString(),
125+
created_at: newReview.submitted_at ?? new Date().toISOString(),
115126
} as TimelineEvent);
116-
} else if (!newReview && currentUser) {
117-
// GraphQL path: submitPendingReview returns void, so newReview is null.
118-
// Construct a synthetic review so the UI updates immediately.
119-
const submittedAt = new Date().toISOString();
120-
const syntheticReview = {
121-
id: Date.now(),
122-
user: {
123-
login: currentUser,
124-
avatar_url: `https://avatars.githubusercontent.com/${currentUser}`,
125-
},
126-
state:
127-
event === "APPROVE"
128-
? "APPROVED"
129-
: event === "REQUEST_CHANGES"
130-
? "CHANGES_REQUESTED"
131-
: "COMMENTED",
132-
submitted_at: submittedAt,
133-
} as Review;
134-
if (!reviews.some((r) => r.user?.login === currentUser)) {
135-
addReviewToArray(syntheticReview);
136-
timeline.unshift({
137-
id: syntheticReview.id,
138-
event: "reviewed",
139-
actor: {
140-
login: currentUser,
141-
avatar_url: `https://avatars.githubusercontent.com/${currentUser}`,
142-
},
143-
created_at: submittedAt,
144-
} as TimelineEvent);
145-
}
146127
}
147128

148129
store.setComments(newComments as ReviewComment[]);

0 commit comments

Comments
 (0)