Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a utils function for GitHub links #174

Merged
merged 1 commit into from
Mar 18, 2017
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
20 changes: 16 additions & 4 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,22 @@
functions you use, we'd love to add them. Ideally you shouldn't need to use anything but Danger + utils
to write your Dangerfiles.

```js
danger.utils.href("http://danger.systems", "Danger") // <a href="http://danger.systems">Danger</a>
danger.utils.sentence(["A", "B", "C"]) // "A, B and C"
```
```js
danger.utils.href("http://danger.systems", "Danger") // <a href="http://danger.systems">Danger</a>
danger.utils.sentence(["A", "B", "C"]) // "A, B and C"
```

* Adds `danger.github.utils` - which currently has only one function: `fileLinks` - orta

Most of the time people are working with a list of files (e.g. modified, or created) and then
want to present clickable links to those. As the logic to figure the URLs is very GitHub specific,
we've moved that into it's own object with space to grow.

```js
const files = danger.git.modified_files // ["lib/component/a.ts", "lib/component/b.ts"]
const links = danger.github.utils.fileLinks(files) // "<a href='...'>a</a> and <a href='...'>b</a>"
warn(`These files have changes: ${links}`)
```

### 0.12.1

Expand Down
2 changes: 1 addition & 1 deletion source/dsl/DangerUtilsDSL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
export interface DangerUtilsDSL {

/**
* Creates an HTML link.
* Creates a link using HTML.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous one is not wrong, but if I already tried fixing it, I bet we'd get people in the future doing the same 👯‍♂️

*
* If `href` and `text` are falsy, null is returned.
* If `href` is falsy and `text` is truthy, `text` is returned.
Expand Down
28 changes: 27 additions & 1 deletion source/dsl/GitHubDSL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,24 @@ export interface GitHubDSL {
reviews: Array<GitHubReview>
/** The people requested to review this PR */
requested_reviewers: Array<GitHubUser>
/** A scope for useful functions related to GitHub */
utils: GitHubUtilsDSL
}

/** Useful functions for GitHub related work */
export interface GitHubUtilsDSL {
/**
* Creates HTML for a sentence of clickable links for an array of paths.
* This uses the source of the PR as the target, not the destination repo.
* You can manually set the target repo and branch however, to make it work how you want.
*
* @param {string} paths A list of strings representing file paths
* @param {string} useBasename Show either the file name, or the full path - defaults to just file name e.g. true.
* @param {string} repoSlug An optional override for the repo slug, ex: "orta/ORStackView"
* @param {string} branch An optional override for the branch, ex: "v3"
* @returns {string} A HTML string of <a>'s built as a sentence.
*/
fileLinks(paths: string[], useBasename?: boolean, repoSlug?: string, branch?: string): string
}

/**
Expand Down Expand Up @@ -292,6 +310,11 @@ export interface GitHubRepo {
* @type {Array<GitHubUser>}
*/
assignees: Array<GitHubUser>
/**
* The root web URL for the repo, e.g. https://github.com/artsy/emission
* @type {string}
*/
html_url: string
}

export interface GitHubMergeRef {
Expand All @@ -315,9 +338,12 @@ export interface GitHubMergeRef {

/**
* The user that owns the merge reference e.g. "artsy"
* @type {string}
*/
user: GitHubUser
/**
* The repo from whch the reference comes from
*/
repo: GitHubRepo
}

/**
Expand Down
4 changes: 3 additions & 1 deletion source/platforms/GitHub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { GitDSL } from "../dsl/GitDSL"
import { GitCommit } from "../dsl/Commit"
import { GitHubPRDSL, GitHubCommit, GitHubDSL, GitHubIssue, GitHubIssueLabel } from "../dsl/GitHubDSL"
import { GitHubAPI } from "./github/GitHubAPI"
import GitHubUtils from "./github/GitHubUtils"

import * as parseDiff from "parse-diff"
import * as includes from "lodash.includes"
Expand Down Expand Up @@ -99,7 +100,8 @@ export class GitHub {
pr,
commits,
reviews,
requested_reviewers
requested_reviewers,
utils: GitHubUtils(pr)
}
}

Expand Down
28 changes: 28 additions & 0 deletions source/platforms/github/GitHubUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import {basename} from "path"
import {sentence, href} from "../../runner/DangerUtils"
import {GitHubPRDSL, GitHubUtilsDSL} from "./../../dsl/GitHubDSL"

// We need to curry in access to the GitHub PR metadata

const utils = (pr: GitHubPRDSL) : GitHubUtilsDSL => {

const fileLinks = (paths: string[], useBasename: boolean = true, repoSlug?: string, branch?: string): string => {
// To support enterprise github, we need to handle custom github domains
// this can be pulled out of the repo url metadata

const githubRoot = pr.head.repo.html_url.split(pr.head.repo.owner.login)[0]
const slug = repoSlug || pr.head.repo.full_name
const ref = branch || pr.head.ref

const toHref = (path: string) => `${githubRoot}${slug}/blob/${ref}/${path}`
// As we should only be getting paths we can ignore the nullability
const hrefs = paths.map(p => href(toHref(p), useBasename ? basename(p) : p )) as string[]
return sentence(hrefs)
}

return {
fileLinks
}
}

export default utils
28 changes: 28 additions & 0 deletions source/platforms/github/_tests/_GitHubUtils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import utils from "../GitHubUtils"

import { readFileSync } from "fs"
import { resolve } from "path"

const fixtures = resolve(__dirname, "..", "..", "_tests", "fixtures")
const fixuredData = (path) => JSON.parse(readFileSync(`${fixtures}/${path}`, {}).toString())
const pr = fixuredData("github_pr.json")

describe("fileLinks", () => {
it("Should convert a few paths into links", () => {
const sut = utils(pr)
const links = sut.fileLinks(["a/b/c", "d/e/f"])
expect(links).toEqual('<a href="https://github.com/artsy/emission/blob/genevc/a/b/c">c</a> and <a href="https://github.com/artsy/emission/blob/genevc/d/e/f">f</a>') //tslint:disable-line:max-line-length
})

it("Should convert a few paths into links showing full links", () => {
const sut = utils(pr)
const links = sut.fileLinks(["a/b/c", "d/e/f"], false)
expect(links).toEqual('<a href="https://github.com/artsy/emission/blob/genevc/a/b/c">a/b/c</a> and <a href="https://github.com/artsy/emission/blob/genevc/d/e/f">d/e/f</a>') //tslint:disable-line:max-line-length
})

it("Should convert a few paths into links showing full link on a custom fork/branch", () => {
const sut = utils(pr)
const links = sut.fileLinks(["a/b/c", "d/e/f"], false, "orta/emission", "new")
expect(links).toEqual('<a href="https://github.com/orta/emission/blob/new/a/b/c">a/b/c</a> and <a href="https://github.com/orta/emission/blob/new/d/e/f">d/e/f</a>') //tslint:disable-line:max-line-length
})
})
5 changes: 4 additions & 1 deletion source/runner/DangerUtils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// The documentation for these are provided inline
// inside DangerUtilsDSL.ts

export function sentence(array: Array<string>): string {
if ((array || []).length === 0) {
return ""
Expand All @@ -8,7 +11,7 @@ export function sentence(array: Array<string>): string {
return array.slice(0, array.length - 1).join(", ") + " and " + array.pop()
}

export function href(href: string, text: string): string | null {
export function href(href?: string, text?: string): string | null {
if (!href && !text) {
return null
}
Expand Down