Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 143 additions & 4 deletions __test__/hooks/PullRequestCommenter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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()
})
31 changes: 31 additions & 0 deletions __test__/utils/getBaseFilename.test.ts
Original file line number Diff line number Diff line change
@@ -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("")
})
7 changes: 7 additions & 0 deletions src/common/utils/getBaseFilename.ts
Original file line number Diff line number Diff line change
@@ -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(".")
}
1 change: 1 addition & 0 deletions src/common/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
39 changes: 27 additions & 12 deletions src/features/hooks/domain/PullRequestCommenter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { IGitHubClient, PullRequestFile } from "@/common"
import { getBaseFilename } from "@/common/utils"

export default class PullRequestCommenter {
private readonly domain: string
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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: {
Expand Down Expand Up @@ -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 })
Expand Down Expand Up @@ -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)
}
}