diff --git a/src/actions.ts b/src/actions.ts index 938a1da..ca0dcf0 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -4,7 +4,7 @@ import { EmitterWebhookEventName as WebhookEventName } from "@octokit/webhooks"; import { Value } from "@sinclair/typebox/value"; import { LogReturn, Logs } from "@ubiquity-os/ubiquity-os-logger"; import { config } from "dotenv"; -import { postComment } from "./comment"; +import { CommentHandler } from "./comment"; import { Context } from "./context"; import { customOctokit } from "./octokit"; import { verifySignature } from "./signature"; @@ -30,7 +30,7 @@ export async function createActionsPlugin o.message).join(", ")}`); return; } const signature = body.signature; @@ -46,6 +46,7 @@ export async function createActionsPlugin = T & { issueNumber: number; }; -export type PostComment = { - ( +interface IssueContext { + issueNumber: number; + commentId?: number; + owner: string; + repo: string; +} + +export class CommentHandler { + public static readonly HEADER_NAME = "UbiquityOS"; + private _lastCommentId = { reviewCommentId: null as number | null, issueCommentId: null as number | null }; + + async _updateIssueComment( context: Context, - message: LogReturn | Error, - options?: CommentOptions - ): Promise | null>; - lastCommentId?: number; -}; + params: { owner: string; repo: string; body: string; issueNumber: number } + ): Promise> { + if (!this._lastCommentId.issueCommentId) { + throw context.logger.error("issueCommentId is missing"); + } + const commentData = await context.octokit.rest.issues.updateComment({ + owner: params.owner, + repo: params.repo, + comment_id: this._lastCommentId.issueCommentId, + body: params.body, + }); + return { ...commentData.data, issueNumber: params.issueNumber }; + } -/** - * Posts a comment on a GitHub issue if the issue exists in the context payload, embedding structured metadata to it. - */ -export const postComment: PostComment = async function ( - context: Context, - message: LogReturn | Error, - options: CommentOptions = { updateComment: true, raw: false } -) { - let issueNumber; - - if ("issue" in context.payload) { - issueNumber = context.payload.issue.number; - } else if ("pull_request" in context.payload) { - issueNumber = context.payload.pull_request.number; - } else if ("discussion" in context.payload) { - issueNumber = context.payload.discussion.number; - } else { - context.logger.info("Cannot post comment because issue is not found in the payload."); - return null; + async _updateReviewComment( + context: Context, + params: { owner: string; repo: string; body: string; issueNumber: number } + ): Promise> { + if (!this._lastCommentId.reviewCommentId) { + throw context.logger.error("reviewCommentId is missing"); + } + const commentData = await context.octokit.rest.pulls.updateReviewComment({ + owner: params.owner, + repo: params.repo, + comment_id: this._lastCommentId.reviewCommentId, + body: params.body, + }); + return { ...commentData.data, issueNumber: params.issueNumber }; } - if ("repository" in context.payload && context.payload.repository?.owner?.login) { - const body = await createStructuredMetadataWithMessage(context, message, options); - if (options.updateComment && postComment.lastCommentId) { - const commentData = await context.octokit.rest.issues.updateComment({ - owner: context.payload.repository.owner.login, - repo: context.payload.repository.name, - comment_id: postComment.lastCommentId, - body: body, - }); - return { ...commentData.data, issueNumber }; - } else { - const commentData = await context.octokit.rest.issues.createComment({ - owner: context.payload.repository.owner.login, - repo: context.payload.repository.name, - issue_number: issueNumber, - body: body, + async _createNewComment( + context: Context, + params: { owner: string; repo: string; body: string; issueNumber: number; commentId?: number } + ): Promise> { + if (params.commentId) { + const commentData = await context.octokit.rest.pulls.createReplyForReviewComment({ + owner: params.owner, + repo: params.repo, + pull_number: params.issueNumber, + comment_id: params.commentId, + body: params.body, }); - postComment.lastCommentId = commentData.data.id; - return { ...commentData.data, issueNumber }; + this._lastCommentId.reviewCommentId = commentData.data.id; + return { ...commentData.data, issueNumber: params.issueNumber }; } - } else { - context.logger.info("Cannot post comment because repository is not found in the payload.", { payload: context.payload }); + + const commentData = await context.octokit.rest.issues.createComment({ + owner: params.owner, + repo: params.repo, + issue_number: params.issueNumber, + body: params.body, + }); + this._lastCommentId.issueCommentId = commentData.data.id; + return { ...commentData.data, issueNumber: params.issueNumber }; } - return null; -}; -async function createStructuredMetadataWithMessage(context: Context, message: LogReturn | Error, options: CommentOptions) { - let logMessage; - let callingFnName; - let instigatorName; - let metadata: Metadata; - - if (message instanceof Error) { - metadata = { - message: message.message, - name: message.name, - stack: message.stack, - }; - callingFnName = message.stack?.split("\n")[2]?.match(/at (\S+)/)?.[1] ?? "anonymous"; - logMessage = context.logger.error(message.message).logMessage; - } else if (message.metadata) { - metadata = { - message: message.metadata.message, - stack: message.metadata.stack || message.metadata.error?.stack, - caller: message.metadata.caller || message.metadata.error?.stack?.split("\n")[2]?.match(/at (\S+)/)?.[1], + _getIssueNumber(context: Context): number | undefined { + if ("issue" in context.payload) return context.payload.issue.number; + if ("pull_request" in context.payload) return context.payload.pull_request.number; + if ("discussion" in context.payload) return context.payload.discussion.number; + return undefined; + } + + _getCommentId(context: Context): number | undefined { + return "pull_request" in context.payload && "comment" in context.payload ? context.payload.comment.id : undefined; + } + + _extractIssueContext(context: Context): IssueContext | null { + if (!("repository" in context.payload) || !context.payload.repository?.owner?.login) { + return null; + } + + const issueNumber = this._getIssueNumber(context); + if (!issueNumber) return null; + + return { + issueNumber, + commentId: this._getCommentId(context), + owner: context.payload.repository.owner.login, + repo: context.payload.repository.name, }; - logMessage = message.logMessage; - callingFnName = metadata.caller; - } else { - metadata = { ...message }; } - const jsonPretty = sanitizeMetadata(metadata); - if ("installation" in context.payload && context.payload.installation && "account" in context.payload.installation) { - instigatorName = context.payload.installation?.account?.name; - } else { - instigatorName = context.payload.sender?.login || HEADER_NAME; + async _processMessage(context: Context, message: LogReturn | Error) { + if (message instanceof Error) { + const metadata = { + message: message.message, + name: message.name, + stack: message.stack, + }; + return { metadata, logMessage: context.logger.error(message.message).logMessage }; + } + + const metadata = message.metadata + ? { + ...message.metadata, + message: message.metadata.message, + stack: message.metadata.stack || message.metadata.error?.stack, + caller: message.metadata.caller || message.metadata.error?.stack?.split("\n")[2]?.match(/at (\S+)/)?.[1], + } + : { ...message }; + + return { metadata, logMessage: message.logMessage }; + } + + _getInstigatorName(context: Context): string { + if ( + "installation" in context.payload && + context.payload.installation && + "account" in context.payload.installation && + context.payload.installation?.account?.name + ) { + return context.payload.installation?.account?.name; + } + return context.payload.sender?.login || CommentHandler.HEADER_NAME; + } + + async _createMetadataContent(context: Context, metadata: Metadata) { + const jsonPretty = sanitizeMetadata(metadata); + const instigatorName = this._getInstigatorName(context); + const runUrl = PluginRuntimeInfo.getInstance().runUrl; + const version = await PluginRuntimeInfo.getInstance().version; + const callingFnName = metadata.caller || "anonymous"; + + return { + header: `"].join("\n"); - let metadataSerialized: string; - const metadataSerializedVisible = ["```json", jsonPretty, "```"].join("\n"); - const metadataSerializedHidden = [ubiquityMetadataHeader, jsonPretty, "-->"].join("\n"); + return logMessage?.type === "fatal" ? [metadataVisible, metadataHidden].join("\n") : metadataHidden; + } + + async _createCommentBody(context: Context, message: LogReturn | Error, options: CommentOptions): Promise { + const { metadata, logMessage } = await this._processMessage(context, message); + const { header, jsonPretty } = await this._createMetadataContent(context, metadata); + const metadataContent = this._formatMetadataContent(logMessage, header, jsonPretty); - if (logMessage?.type === "fatal") { - // if the log message is fatal, then we want to show the metadata - metadataSerialized = [metadataSerializedVisible, metadataSerializedHidden].join("\n"); - } else { - // otherwise we want to hide it - metadataSerialized = metadataSerializedHidden; + return `${options.raw ? logMessage?.raw : logMessage?.diff}\n\n${metadataContent}\n`; } - // Add carriage returns to avoid any formatting issue - return `${options.raw ? logMessage?.raw : logMessage?.diff}\n\n${metadataSerialized}\n`; + async postComment( + context: Context, + message: LogReturn | Error, + options: CommentOptions = { updateComment: true, raw: false } + ): Promise | null> { + const issueContext = this._extractIssueContext(context); + if (!issueContext) { + context.logger.info("Cannot post comment: missing issue context in payload"); + return null; + } + + const body = await this._createCommentBody(context, message, options); + const { issueNumber, commentId, owner, repo } = issueContext; + const params = { owner, repo, body, issueNumber }; + + if (options.updateComment) { + if (this._lastCommentId.issueCommentId && !("pull_request" in context.payload && "comment" in context.payload)) { + return this._updateIssueComment(context, params); + } + + if (this._lastCommentId.reviewCommentId && "pull_request" in context.payload && "comment" in context.payload) { + return this._updateReviewComment(context, params); + } + } + + return this._createNewComment(context, { ...params, commentId }); + } } diff --git a/src/context.ts b/src/context.ts index 47b6933..f1436a0 100644 --- a/src/context.ts +++ b/src/context.ts @@ -1,5 +1,6 @@ import { EmitterWebhookEvent as WebhookEvent, EmitterWebhookEventName as WebhookEventName } from "@octokit/webhooks"; import { Logs } from "@ubiquity-os/ubiquity-os-logger"; +import { CommentHandler } from "./comment"; import { customOctokit } from "./octokit"; export interface Context { @@ -12,4 +13,5 @@ export interface Context { + app.post("/", async function appPost(ctx) { if (ctx.req.header("content-type") !== "application/json") { throw new HTTPException(400, { message: "Content-Type must be application/json" }); } @@ -92,6 +92,7 @@ export function createPlugin { it("Should reuse a message if the reuse option is true", async () => { @@ -15,7 +15,7 @@ describe("Post comment tests", () => { id: 1234, }, })); - jest.unstable_mockModule("@octokit/core", () => ({ + const c = jest.unstable_mockModule("@octokit/core", () => ({ Octokit: jest.fn(() => ({ rest: { issues: { @@ -41,8 +41,9 @@ describe("Post comment tests", () => { logger, octokit: new Octokit(), } as unknown as Context; - await postComment(ctx, logger.ok("test"), { updateComment: true }); - await postComment(ctx, logger.ok("test 2"), { updateComment: true }); + const commentHandler = new CommentHandler(); + await commentHandler.postComment(ctx, logger.ok("test"), { updateComment: true }); + await commentHandler.postComment(ctx, logger.ok("test 2"), { updateComment: true }); expect(createComment).toHaveBeenCalledWith({ owner: "ubiquity-os", repo: "plugin-sdk", @@ -55,5 +56,6 @@ describe("Post comment tests", () => { comment_id: 1234, body: expect.anything(), }); + c.clearAllMocks(); }); }); diff --git a/tests/pr.test.ts b/tests/pr.test.ts new file mode 100644 index 0000000..e1e9aa9 --- /dev/null +++ b/tests/pr.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, it, jest } from "@jest/globals"; +import { Logs } from "@ubiquity-os/ubiquity-os-logger"; +import { CommentHandler, Context } from "../src"; + +describe("Pull-request comment tests", () => { + it("Should be able to post to issues and pull-request reviews", async () => { + const logger = new Logs("debug"); + const createComment = jest.fn(() => ({ + data: { + id: 1234, + }, + })); + const updateComment = jest.fn(() => ({ + data: { + id: 1234, + }, + })); + const createReplyForReviewComment = jest.fn(() => ({ + data: { + id: 5678, + }, + })); + const updateReviewComment = jest.fn(() => ({ + data: { + id: 5678, + }, + })); + jest.unstable_mockModule("@octokit/core", () => ({ + Octokit: jest.fn(() => ({ + rest: { + issues: { + createComment, + updateComment, + }, + pulls: { + createReplyForReviewComment, + updateReviewComment, + }, + }, + })), + })); + const { Octokit } = await import("@octokit/core"); + const ctxIssue = { + payload: { + pull_request: { + number: 1, + }, + repository: { + owner: { + login: "ubiquity-os", + }, + name: "plugin-sdk", + }, + }, + logger, + octokit: new Octokit(), + } as unknown as Context; + const ctxReviewComment = { + payload: { + pull_request: { + number: 1, + }, + comment: { + id: 2, + }, + repository: { + owner: { + login: "ubiquity-os", + }, + name: "plugin-sdk", + }, + }, + logger, + octokit: new Octokit(), + } as unknown as Context; + const commentHandler = new CommentHandler(); + await commentHandler.postComment(ctxIssue, logger.ok("test"), { updateComment: true }); + await commentHandler.postComment(ctxIssue, logger.ok("test 2"), { updateComment: true }); + await commentHandler.postComment(ctxReviewComment, logger.ok("test 3"), { updateComment: true }); + await commentHandler.postComment(ctxReviewComment, logger.ok("test 4"), { updateComment: true }); + expect(createComment).toHaveBeenCalledWith({ + owner: "ubiquity-os", + repo: "plugin-sdk", + issue_number: 1, + body: expect.anything(), + }); + expect(updateComment).toHaveBeenCalledWith({ + owner: "ubiquity-os", + repo: "plugin-sdk", + comment_id: 1234, + body: expect.anything(), + }); + expect(createReplyForReviewComment).toHaveBeenCalledWith({ + owner: "ubiquity-os", + repo: "plugin-sdk", + pull_number: 1, + comment_id: 2, + body: expect.anything(), + }); + expect(updateReviewComment).toHaveBeenCalledWith({ + owner: "ubiquity-os", + repo: "plugin-sdk", + comment_id: 5678, + body: expect.anything(), + }); + }); +});