Merge pull request #252 from shapehq/enhancement/filter-filenames

Ignores files starting with dot and residing in directory when posting PR

__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
@@ -314,7 +314,7 @@ test("It updates comment to remove file list  when all relevant file changes wer
   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("<table>")
-  expect(commentBody).not.toContain(".demo-docs.yml")
+  expect(commentBody).toContain("<table>")
+  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("<table>")
+  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("<table>")
+  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()
 })

__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("")
+})

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(".")
+}

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"

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}/${ref}`
+        if (!this.isProjectConfigurationFile(file.filename)) {
+          link += `/${file.filename}`
+        }
         button = ` <a href="${link}">Preview</a>`
       }
       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)
+  }
+}