Separate GitHub issueByNumber/prByNumber (#284)

Summary:
Instead of having one function that returns a union, we present two
functions, each of which returns a more specific type.

Paired with @decentralion.

Test Plan:
Existing unit tests pass, with sufficient modifications.

wchargin-branch: separate-issueByNumber-prByNumber
This commit is contained in:
William Chargin 2018-05-14 18:15:45 -07:00 committed by GitHub
parent fb8da7fcdb
commit 9591792f59
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 52 additions and 29 deletions

View File

@ -173,19 +173,30 @@ export class Repository extends GithubEntity<RepositoryNodePayload> {
return (e: any);
}
issueOrPRByNumber(number: number): ?(Issue | PullRequest) {
let result: Issue | PullRequest;
this.graph
.neighborhood(this.nodeAddress, {
edgeType: CONTAINS_EDGE_TYPE,
})
.forEach(({neighbor}) => {
const payload = this.graph.node(neighbor).payload;
if (payload.number === number) {
result = (asEntity(this.graph, neighbor): any);
}
});
return result;
issueByNumber(number: number): ?Issue {
for (const {neighbor} of this.graph.neighborhood(this.nodeAddress, {
edgeType: CONTAINS_EDGE_TYPE,
direction: "OUT",
nodeType: ISSUE_NODE_TYPE,
})) {
const node = this.graph.node(neighbor);
if (node.payload.number === number) {
return new Issue(this.graph, neighbor);
}
}
}
pullRequestByNumber(number: number): ?PullRequest {
for (const {neighbor} of this.graph.neighborhood(this.nodeAddress, {
edgeType: CONTAINS_EDGE_TYPE,
direction: "OUT",
nodeType: PULL_REQUEST_NODE_TYPE,
})) {
const node = this.graph.node(neighbor);
if (node.payload.number === number) {
return new PullRequest(this.graph, neighbor);
}
}
}
owner(): string {

View File

@ -47,17 +47,33 @@ describe("GitHub porcelain", () => {
expect(urlToProperty).toMatchSnapshot();
}
function issueOrPRByNumber(n: number): Issue | PullRequest {
const result = repo.issueOrPRByNumber(n);
function issueByNumber(n: number): Issue {
const result = repo.issueByNumber(n);
if (result == null) {
throw new Error(`Expected Issue/PR ${n} to exist`);
throw new Error(`Expected Issue #${n} to exist`);
}
return result;
}
const issue = issueOrPRByNumber(2);
function prByNumber(n: number): PullRequest {
const result = repo.pullRequestByNumber(n);
if (result == null) {
throw new Error(`Expected PR #${n} to exist`);
}
return result;
}
function issueOrPrByNumber(n: number): Issue | PullRequest {
const result = repo.issueByNumber(n) || repo.pullRequestByNumber(n);
if (result == null) {
throw new Error(`Expected Issue/PR #${n} to exist`);
}
return result;
}
const issue = issueByNumber(2);
const comment = issue.comments()[0];
const pullRequest = PullRequest.from(issueOrPRByNumber(5));
const pullRequest = prByNumber(5);
const pullRequestReview = pullRequest.reviews()[0];
const pullRequestReviewComment = pullRequestReview.comments()[0];
const author = issue.authors()[0];
@ -157,7 +173,7 @@ describe("GitHub porcelain", () => {
describe("issues and pull requests", () => {
const issuesAndPRs = [1, 2, 3, 4, 5, 6, 7, 8, 9].map((n) =>
issueOrPRByNumber(n)
issueOrPrByNumber(n)
);
it("have numbers", () => {
expectPropertiesToMatchSnapshot(issuesAndPRs, (e) => e.number());
@ -173,11 +189,7 @@ describe("GitHub porcelain", () => {
});
describe("pull requests", () => {
const prs = [
PullRequest.from(issueOrPRByNumber(3)),
PullRequest.from(issueOrPRByNumber(5)),
PullRequest.from(issueOrPRByNumber(9)),
];
const prs = [prByNumber(3), prByNumber(5), prByNumber(9)];
it("have mergeCommitHashes", () => {
expectPropertiesToMatchSnapshot(prs, (e) => e.mergeCommitHash());
});
@ -239,7 +251,7 @@ describe("GitHub porcelain", () => {
describe("References", () => {
it("via #-number", () => {
const srcIssue = issueOrPRByNumber(2);
const srcIssue = issueByNumber(2);
const references = srcIssue.references();
expect(references).toHaveLength(1);
// Note: this verifies that we are not counting in-references, as
@ -252,7 +264,7 @@ describe("GitHub porcelain", () => {
describe("by exact url", () => {
function expectCommentToHaveSingleReference({commentNumber, type, url}) {
const comments = issueOrPRByNumber(2).comments();
const comments = issueByNumber(2).comments();
const references = comments[commentNumber].references();
expect(references).toHaveLength(1);
expect(references[0].url()).toBe(url);
@ -311,14 +323,14 @@ describe("GitHub porcelain", () => {
});
it("to multiple entities", () => {
const references = issueOrPRByNumber(2)
const references = issueByNumber(2)
.comments()[6]
.references();
expect(references).toHaveLength(5);
});
it("to no entities", () => {
const references = issueOrPRByNumber(2)
const references = issueByNumber(2)
.comments()[7]
.references();
expect(references).toHaveLength(0);
@ -326,7 +338,7 @@ describe("GitHub porcelain", () => {
});
it("References by @-author", () => {
const pr = issueOrPRByNumber(5);
const pr = prByNumber(5);
const references = pr.references();
expect(references).toHaveLength(1);
const referenced = Author.from(references[0]);