diff --git a/packages/source-control-gitlab/src/server/GitLabPullRequestCli.ts b/packages/source-control-gitlab/src/server/GitLabPullRequestCli.ts index 7c386760b7a7d..c01157c4d94a5 100644 --- a/packages/source-control-gitlab/src/server/GitLabPullRequestCli.ts +++ b/packages/source-control-gitlab/src/server/GitLabPullRequestCli.ts @@ -466,6 +466,7 @@ export class GitLabPullRequestCli extends Context.Service< { readonly reactions: ReadonlyArray; readonly reactionsByNoteId: ReadonlyMap>; + readonly editedAtByNoteId: ReadonlyMap; readonly botLogins: ReadonlySet; }, GitLabPullRequestCliError @@ -1092,12 +1093,14 @@ export const make = Effect.gen(function* () { readonly collected: { readonly reactions: ReadonlyArray; readonly reactionsByNoteId: Map>; + readonly editedAtByNoteId: Map; readonly botLogins: Set; } | null; }): Effect.Effect< { readonly reactions: ReadonlyArray; readonly reactionsByNoteId: ReadonlyMap>; + readonly editedAtByNoteId: ReadonlyMap; readonly botLogins: ReadonlySet; }, GitLabPullRequestCliError @@ -1131,10 +1134,13 @@ export const make = Effect.gen(function* () { const collected = input.collected ?? { reactions: decoded.success.reactions, reactionsByNoteId: new Map>(), + editedAtByNoteId: new Map(), botLogins: new Set(), }; for (const [id, reactions] of decoded.success.reactionsByNoteId) collected.reactionsByNoteId.set(id, reactions); + for (const [id, editedAt] of decoded.success.editedAtByNoteId) + collected.editedAtByNoteId.set(id, editedAt); for (const login of decoded.success.botLogins) collected.botLogins.add(login); return decoded.success.nextCursor === null || input.page >= CONVERSATION_PAGES ? Effect.succeed(collected) diff --git a/packages/source-control-gitlab/src/server/GitLabPullRequestProvider.test.ts b/packages/source-control-gitlab/src/server/GitLabPullRequestProvider.test.ts index 7e68761996321..3eeef5645953d 100644 --- a/packages/source-control-gitlab/src/server/GitLabPullRequestProvider.test.ts +++ b/packages/source-control-gitlab/src/server/GitLabPullRequestProvider.test.ts @@ -485,7 +485,12 @@ describe("optional activity reads", () => { listCommits: () => Effect.succeed([commit]), listDiscussions: () => Effect.succeed({ threads: [], truncated: false }), listReactions: () => - Effect.succeed({ reactions: [], reactionsByNoteId: new Map(), botLogins: new Set() }), + Effect.succeed({ + reactions: [], + reactionsByNoteId: new Map(), + editedAtByNoteId: new Map(), + botLogins: new Set(), + }), }; const context = { operation: "execute" as const, @@ -532,6 +537,46 @@ describe("optional activity reads", () => { }), ); + it.effect("dates edited comments in the conversation and in threads", () => + Effect.gen(function* () { + const editedAt = "2026-07-02T00:00:00Z"; + const provider = yield* make.pipe( + Effect.provide( + Layer.mock(GitLabPullRequestCli.GitLabPullRequestCli)({ + ...reads, + listDiscussions: () => + Effect.succeed({ + threads: [ + { + id: "d1", + path: "src/a.ts", + line: 1, + side: "right" as const, + isResolved: false, + isOutdated: false, + comments: [comment], + }, + ], + truncated: false, + }), + listReactions: () => + Effect.succeed({ + reactions: [], + reactionsByNoteId: new Map(), + editedAtByNoteId: new Map([[comment.id, editedAt]]), + botLogins: new Set(), + }), + }), + ), + ); + + const activity = yield* provider.getChangeRequestActivity(reference); + + expect(activity.comments[0]?.editedAt).toBe(editedAt); + expect(activity.reviewThreads[0]?.comments[0]?.editedAt).toBe(editedAt); + }), + ); + it.effect("keeps comments and commits when optional discussions and reactions fail", () => Effect.gen(function* () { const provider = yield* make.pipe( @@ -600,6 +645,7 @@ describe("optional activity reads", () => { Effect.succeed({ reactions: [], reactionsByNoteId: new Map(), + editedAtByNoteId: new Map(), botLogins: new Set([bot.login.toUpperCase()]), }), }), diff --git a/packages/source-control-gitlab/src/server/GitLabPullRequestProvider.ts b/packages/source-control-gitlab/src/server/GitLabPullRequestProvider.ts index 82712a2c5ef3a..6db9b8545fefb 100644 --- a/packages/source-control-gitlab/src/server/GitLabPullRequestProvider.ts +++ b/packages/source-control-gitlab/src/server/GitLabPullRequestProvider.ts @@ -331,6 +331,7 @@ export const make = Effect.gen(function* () { recoverRead(cli.listReactions(input), { reactions: [] as ReadonlyArray, reactionsByNoteId: new Map>(), + editedAtByNoteId: new Map(), botLogins: new Set(), }), ], @@ -338,6 +339,10 @@ export const make = Effect.gen(function* () { ).pipe( Effect.mapError(fail("getChangeRequestActivity")), Effect.map(([notes, commits, discussions, awards]): ProviderChangeRequestActivity => { + const edited = (id: string) => { + const editedAt = awards.editedAtByNoteId.get(id); + return editedAt === undefined ? {} : { editedAt }; + }; const botLogins = new Set([...awards.botLogins].map((login) => login.toLowerCase())); const withBotFlag = (author: PullRequestActor | null) => author !== null && botLogins.has(author.login.toLowerCase()) @@ -347,6 +352,7 @@ export const make = Effect.gen(function* () { reactions: awards.reactions, comments: notes.comments.map((comment) => ({ ...comment, + ...edited(comment.id), author: withBotFlag(comment.author), reactions: awards.reactionsByNoteId.get(comment.id) ?? [], })), @@ -356,6 +362,7 @@ export const make = Effect.gen(function* () { ...thread, comments: thread.comments.map((comment) => ({ ...comment, + ...edited(comment.id), author: withBotFlag(comment.author), reactions: awards.reactionsByNoteId.get(comment.id) ?? [], })), diff --git a/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.test.ts b/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.test.ts index 7bec611ce077c..2b3ff84330ccc 100644 --- a/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.test.ts +++ b/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.test.ts @@ -544,6 +544,48 @@ describe("decodeAwardEmojiJson", () => { expect(result.nextCursor).toBeNull(); }); + it("dates a note's edit only when someone edited its text, not when its thread was resolved", () => { + const result = expectSuccess( + decodeAwardEmojiJson( + JSON.stringify({ + data: { + currentUser: { username: "bilal" }, + project: { + mergeRequest: { + awardEmoji: { nodes: [] }, + notes: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + { + id: "gid://gitlab/Note/1", + lastEditedAt: "2026-07-02T00:00:00Z", + lastEditedBy: { username: "julius" }, + resolved: false, + }, + { + id: "gid://gitlab/Note/2", + lastEditedAt: "2026-07-01T00:00:00Z", + lastEditedBy: null, + resolved: false, + }, + { + id: "gid://gitlab/DiscussionNote/3", + lastEditedAt: "2026-07-03T00:00:00Z", + lastEditedBy: { username: "julius" }, + resolved: true, + }, + ], + }, + }, + }, + }, + }), + ), + ); + + expect([...result.editedAtByNoteId]).toEqual([["1", "2026-07-02T00:00:00Z"]]); + }); + it("matches the viewer's username case-insensitively, since GitLab is not consistent about case", () => { const result = expectSuccess( decodeAwardEmojiJson( diff --git a/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts b/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts index 3f492bf20eee4..e3221f956ddb5 100644 --- a/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts +++ b/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts @@ -780,7 +780,7 @@ export const AWARD_EMOJI_GRAPHQL_QUERY = `query($fullPath: ID!, $iid: String!, $ awardEmoji { nodes { name user { username } } } notes(first: 100, after: $cursor) { pageInfo { hasNextPage endCursor } - nodes { id author { username bot } awardEmoji { nodes { name user { username } } } } + nodes { id author { username bot } lastEditedAt lastEditedBy { username } resolved awardEmoji { nodes { name user { username } } } } } } } @@ -832,6 +832,15 @@ const RawAwardEmojiPageSchema = Schema.Struct({ Schema.NullOr( Schema.Struct({ id: Schema.optional(Schema.NullOr(Schema.String)), + lastEditedAt: Schema.optional(Schema.NullOr(Schema.String)), + lastEditedBy: Schema.optional( + Schema.NullOr( + Schema.Struct({ + username: Schema.optional(Schema.NullOr(Schema.String)), + }), + ), + ), + resolved: Schema.optional(Schema.NullOr(Schema.Boolean)), // REST note authors carry no bot flag; GraphQL's does. author: Schema.optional( Schema.NullOr( @@ -895,6 +904,7 @@ function noteIdOf(gid: string | null | undefined): string | null { export interface GitLabAwardEmojiPage { readonly reactions: ReadonlyArray; readonly reactionsByNoteId: ReadonlyMap>; + readonly editedAtByNoteId: ReadonlyMap; readonly botLogins: ReadonlySet; readonly nextCursor: string | null; } @@ -910,6 +920,7 @@ export function decodeAwardEmojiJson( const viewer = trimmed(data.currentUser?.username); const mergeRequest = data.project?.mergeRequest; const reactionsByNoteId = new Map>(); + const editedAtByNoteId = new Map(); const botLogins = new Set(); for (const node of mergeRequest?.notes?.nodes ?? []) { const author = trimmed(node?.author?.username); @@ -918,11 +929,22 @@ export function decodeAwardEmojiJson( if (id === null) continue; const reactions = toReactions(node?.awardEmoji, viewer); if (reactions.length > 0) reactionsByNoteId.set(id, reactions); + // Resolving a thread moves lastEditedAt on every note in it; only lastEditedBy marks a + // text edit, and a resolved note's stamp may be the resolve rather than that edit. + const editedAt = trimmed(node?.lastEditedAt); + if ( + editedAt !== null && + trimmed(node?.lastEditedBy?.username) !== null && + node?.resolved !== true + ) { + editedAtByNoteId.set(id, editedAt); + } } const pageInfo = mergeRequest?.notes?.pageInfo; return Result.succeed({ reactions: toReactions(mergeRequest?.awardEmoji, viewer), reactionsByNoteId, + editedAtByNoteId, botLogins, nextCursor: pageInfo?.hasNextPage === true ? (trimmed(pageInfo.endCursor) ?? null) : null, });