From 1f9fc7b9fe94f5a8b938cce32dfed4869ec172da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Thu, 18 Jul 2024 15:31:13 +0200 Subject: [PATCH 1/4] Fixes test name --- __test__/hooks/PullRequestCommenter.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__test__/hooks/PullRequestCommenter.test.ts b/__test__/hooks/PullRequestCommenter.test.ts index 0091b8d4..83c3c5b8 100644 --- a/__test__/hooks/PullRequestCommenter.test.ts +++ b/__test__/hooks/PullRequestCommenter.test.ts @@ -238,7 +238,7 @@ test("It skips updating comment when the body has not changed", async () => { expect(didUpdateComment).toBeFalsy() }) -test("It updates comment to remove file list when all relevant file changes were removed from the PR", async () => { +test("It updates comment to remove file list when all relevant file changes were removed from the PR", async () => { let didAddComment = false let didUpdateComment = false let addedCommentBody: string | undefined From 974d2235d21508240311afb21938acdf5c96520f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Thu, 18 Jul 2024 15:58:06 +0200 Subject: [PATCH 2/4] Adds getBaseFilename --- __test__/utils/getBaseFilename.test.ts | 31 ++++++++++++++++++++++++++ src/common/utils/getBaseFilename.ts | 7 ++++++ src/common/utils/index.ts | 1 + 3 files changed, 39 insertions(+) create mode 100644 __test__/utils/getBaseFilename.test.ts create mode 100644 src/common/utils/getBaseFilename.ts diff --git a/__test__/utils/getBaseFilename.test.ts b/__test__/utils/getBaseFilename.test.ts new file mode 100644 index 00000000..cc110965 --- /dev/null +++ b/__test__/utils/getBaseFilename.test.ts @@ -0,0 +1,31 @@ +import { getBaseFilename } from "@/common" + +test("It returns base filename for file in root", async () => { + const result = getBaseFilename("foo.yml") + expect(result).toEqual("foo") +}) + +test("It returns base filename for file path including dot", async () => { + const result = getBaseFilename("foo.bar.yml") + expect(result).toEqual("foo.bar") +}) + +test("It returns base filename for file in folder", async () => { + const result = getBaseFilename("foo/bar.yml") + expect(result).toEqual("bar") +}) + +test("It returns base filename when file path starts with a slash", async () => { + const result = getBaseFilename("/foo/bar.yml") + expect(result).toEqual("bar") +}) + +test("It returns base filename when file path does not contain an extension", async () => { + const result = getBaseFilename("foo") + expect(result).toEqual("foo") +}) + +test("It returns empty string for the empty string", async () => { + const result = getBaseFilename("") + expect(result).toEqual("") +}) diff --git a/src/common/utils/getBaseFilename.ts b/src/common/utils/getBaseFilename.ts new file mode 100644 index 00000000..d5beb955 --- /dev/null +++ b/src/common/utils/getBaseFilename.ts @@ -0,0 +1,7 @@ +export default function getBaseFilename(filePath: string): string { + const filename = filePath.split("/").pop() || "" + if (!filename.includes(".")) { + return filename + } + return filename.split(".").slice(0, -1).join(".") +} diff --git a/src/common/utils/index.ts b/src/common/utils/index.ts index 4b6ec76d..d8b888cd 100644 --- a/src/common/utils/index.ts +++ b/src/common/utils/index.ts @@ -3,3 +3,4 @@ export { default as fetcher } from "./fetcher" export { default as ZodJSONCoder } from "./ZodJSONCoder" export { default as listFromCommaSeparatedString } from "./listFromCommaSeparatedString" export { default as env } from "./env" +export { default as getBaseFilename } from "./getBaseFilename" From 02400ce4586f03416d2e3bb178d653ac99064e3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Thu, 18 Jul 2024 15:58:29 +0200 Subject: [PATCH 3/4] Ignores files beginning with dot or residing in folders --- __test__/hooks/PullRequestCommenter.test.ts | 145 +++++++++++++++++- .../hooks/domain/PullRequestCommenter.ts | 39 +++-- 2 files changed, 169 insertions(+), 15 deletions(-) diff --git a/__test__/hooks/PullRequestCommenter.test.ts b/__test__/hooks/PullRequestCommenter.test.ts index 83c3c5b8..358c5af5 100644 --- a/__test__/hooks/PullRequestCommenter.test.ts +++ b/__test__/hooks/PullRequestCommenter.test.ts @@ -314,7 +314,7 @@ test("It updates comment to remove file list when all relevant file changes were expect(updatedCommentBody).not.toContain("openapi.yml") }) -test("It adds comment without file table if only project configuration was edited", async () => { +test("It adds comment if only project configuration was edited", async () => { let didAddComment = false let commentBody: string | undefined const sut = new PullRequestCommenter({ @@ -364,6 +364,145 @@ test("It adds comment without file table if only project configuration was edite ref: "main" }) expect(didAddComment).toBeTruthy() - expect(commentBody).not.toContain("") - expect(commentBody).not.toContain(".demo-docs.yml") + expect(commentBody).toContain("
") + expect(commentBody).toContain(".demo-docs.yml") +}) + +test("It ignores files starting with a dot", async () => { + let didAddComment = false + let commentBody: string | undefined + const sut = new PullRequestCommenter({ + domain: "https://example.com", + siteName: "Demo Docs", + repositoryNameSuffix: "-openapi", + projectConfigurationFilename: ".demo-docs.yml", + gitHubAppId: "appid1234", + gitHubClient: { + async graphql() { + return {} + }, + async getRepositoryContent() { + return { downloadURL: "https://example.com" } + }, + async getPullRequestFiles() { + return [{ + filename: "openapi.yml", + status: "changed" + }, { + filename: ".foo,yml", + status: "changed" + }] + }, + async getPullRequestComments() { + return [] + }, + async addCommentToPullRequest(request) { + didAddComment = true + commentBody = request.body + }, + async updatePullRequestComment() {} + } + }) + await sut.commentPullRequest({ + appInstallationId: 1234, + pullRequestNumber: 42, + repositoryOwner: "acme", + repositoryName: "demo-openapi", + ref: "main" + }) + expect(didAddComment).toBeTruthy() + expect(commentBody).toContain("
") + expect(commentBody).toContain("openapi.yml") + expect(commentBody).not.toContain(".foo.yml") +}) + +test("It ignores files in directories", async () => { + let didAddComment = false + let commentBody: string | undefined + const sut = new PullRequestCommenter({ + domain: "https://example.com", + siteName: "Demo Docs", + repositoryNameSuffix: "-openapi", + projectConfigurationFilename: ".demo-docs.yml", + gitHubAppId: "appid1234", + gitHubClient: { + async graphql() { + return {} + }, + async getRepositoryContent() { + return { downloadURL: "https://example.com" } + }, + async getPullRequestFiles() { + return [{ + filename: "openapi.yml", + status: "changed" + }, { + filename: "foo/bar,yml", + status: "changed" + }] + }, + async getPullRequestComments() { + return [] + }, + async addCommentToPullRequest(request) { + didAddComment = true + commentBody = request.body + }, + async updatePullRequestComment() {} + } + }) + await sut.commentPullRequest({ + appInstallationId: 1234, + pullRequestNumber: 42, + repositoryOwner: "acme", + repositoryName: "demo-openapi", + ref: "main" + }) + expect(didAddComment).toBeTruthy() + expect(commentBody).toContain("
") + expect(commentBody).toContain("openapi.yml") + expect(commentBody).not.toContain("foo/bar.yml") +}) + +test("It does not post comment if changes only include ignored filenames", async () => { + let didAddComment = false + const sut = new PullRequestCommenter({ + domain: "https://example.com", + siteName: "Demo Docs", + repositoryNameSuffix: "-openapi", + projectConfigurationFilename: ".demo-docs.yml", + gitHubAppId: "appid1234", + gitHubClient: { + async graphql() { + return {} + }, + async getRepositoryContent() { + return { downloadURL: "https://example.com" } + }, + async getPullRequestFiles() { + return [{ + filename: ".foo.yml", + status: "changed" + }, { + filename: ".github/workflows/bar.yml", + status: "changed" + }] + }, + async getPullRequestComments() { + return [] + }, + async addCommentToPullRequest(_request) { + didAddComment = true + }, + async updatePullRequestComment() {} + } + }) + await sut.commentPullRequest({ + appInstallationId: 1234, + pullRequestNumber: 42, + repositoryOwner: "acme", + repositoryName: "demo-openapi", + ref: "main" + }) + expect(didAddComment).toBeFalsy() }) diff --git a/src/features/hooks/domain/PullRequestCommenter.ts b/src/features/hooks/domain/PullRequestCommenter.ts index d1d0ca90..3a23d5a3 100644 --- a/src/features/hooks/domain/PullRequestCommenter.ts +++ b/src/features/hooks/domain/PullRequestCommenter.ts @@ -1,4 +1,5 @@ import { IGitHubClient, PullRequestFile } from "@/common" +import { getBaseFilename } from "@/common/utils" export default class PullRequestCommenter { private readonly domain: string @@ -7,7 +8,6 @@ export default class PullRequestCommenter { private readonly projectConfigurationFilename: string private readonly gitHubAppId: string private readonly gitHubClient: IGitHubClient - private readonly fileExtensionRegex = /\.ya?ml$/ constructor(config: { domain: string @@ -32,7 +32,7 @@ export default class PullRequestCommenter { ref: string pullRequestNumber: number }) { - const files = await this.getChangedYamlFiles(request) + const files = this.getChangedFiles(await this.getYamlFiles(request)) const commentBody = this.makeCommentBody({ files, owner: request.repositoryOwner, @@ -59,7 +59,20 @@ export default class PullRequestCommenter { } } - private async getChangedYamlFiles(request: { + private getChangedFiles(files: PullRequestFile[]) { + return files + .filter(file => file.status != "unchanged") + .filter(file => { + // Do not include files that begins with a dot (.) unless it's the project configuration. + return !file.filename.startsWith(".") || this.isProjectConfigurationFile(file.filename) + }) + .filter(file => { + // Do not include files in folders. + return file.filename.split("/").length === 1 + }) + } + + private async getYamlFiles(request: { appInstallationId: number, repositoryOwner: string repositoryName: string @@ -71,9 +84,7 @@ export default class PullRequestCommenter { repositoryName: request.repositoryName, pullRequestNumber: request.pullRequestNumber }) - return files - .filter(file => file.filename.match(this.fileExtensionRegex)) - .filter(file => file.status != "unchanged") + return files.filter(file => file.filename.match(/\.ya?ml$/)) } private async getExistingComment(request: { @@ -120,15 +131,15 @@ export default class PullRequestCommenter { const { files, owner, repositoryName, ref } = params const rows: { filename: string, status: string, button: string }[] = [] const projectId = this.getProjectId({ repositoryName }) - // Make sure we don't include the project configuration file. - const baseConfigFilename = this.projectConfigurationFilename.replace(this.fileExtensionRegex, "") - const changedFiles = files.filter(file => file.filename.replace(this.fileExtensionRegex, "") != baseConfigFilename) // Create rows for each file - for (const file of changedFiles) { + for (const file of files) { const status = this.getStatusText(file) let button = "" if (file.status != "removed") { - const link = `${this.domain}/${owner}/${projectId}/${ref}/${file.filename}` + let link = `${this.domain}/${owner}/${projectId}` + if (!this.isProjectConfigurationFile(file.filename)) { + link += `/${ref}/${file.filename}` + } button = ` Preview` } rows.push({ filename: file.filename, status, button }) @@ -159,4 +170,8 @@ export default class PullRequestCommenter { private getProjectId({ repositoryName }: { repositoryName: string }): string { return repositoryName.replace(new RegExp(this.repositoryNameSuffix + "$"), "") } -} + + private isProjectConfigurationFile(filename: string) { + return getBaseFilename(filename) === getBaseFilename(this.projectConfigurationFilename) + } +} \ No newline at end of file From 308ef94c09af10457278cc19901cb099c7a330f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Thu, 18 Jul 2024 15:59:09 +0200 Subject: [PATCH 4/4] Always includes ref --- src/features/hooks/domain/PullRequestCommenter.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/features/hooks/domain/PullRequestCommenter.ts b/src/features/hooks/domain/PullRequestCommenter.ts index 3a23d5a3..315962d7 100644 --- a/src/features/hooks/domain/PullRequestCommenter.ts +++ b/src/features/hooks/domain/PullRequestCommenter.ts @@ -136,9 +136,9 @@ export default class PullRequestCommenter { const status = this.getStatusText(file) let button = "" if (file.status != "removed") { - let link = `${this.domain}/${owner}/${projectId}` + let link = `${this.domain}/${owner}/${projectId}/${ref}` if (!this.isProjectConfigurationFile(file.filename)) { - link += `/${ref}/${file.filename}` + link += `/${file.filename}` } button = ` Preview` }