Deleted users no longer break GitHub parser (#228)

When a GitHub user delete their account, all of their comments remain,
but with a `null` author. Previously, our code did not account for this
possibility. Now it does (by simply not adding an AUTHORSHIP edge).
Conveniently, our porcelain API already represents authors as a list, so
this doesn't require any change in porcelain API usage.

Test plan:
I did not add any unit tests, simply because
creating-and-deleting a GitHub user to make a repro seemed like a bit of
a pain. However, it is very unlikely that this bug will re-occur,
because the nullability of AuthorJSON is now enforced at the type level
inside graphql.js.

Also, running `node bin/sourcecred.js src-d go-git` now succeedsinstead
of failing.
This commit is contained in:
Dandelion Mané 2018-05-07 17:06:26 -07:00 committed by GitHub
parent 0cae9d742d
commit 9d4ae8b901
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 15 additions and 7 deletions

View File

@ -631,6 +631,11 @@ function mergeDirect<T>(destination: T, source: any): T {
} }
} }
// If a user deletes their account, then the author is null
// (the UI will show that the message was written by @ghost).
// Therefore, NullableAuthorJSON is preferred to AuthorJSON
// for most actual usage.
export type NullableAuthorJSON = AuthorJSON | null;
export type AuthorJSON = {| export type AuthorJSON = {|
+__typename: "User" | "Bot" | "Organization", +__typename: "User" | "Bot" | "Organization",
+id: string, +id: string,
@ -667,7 +672,7 @@ export type IssueJSON = {|
+title: string, +title: string,
+body: string, +body: string,
+number: number, +number: number,
+author: AuthorJSON, +author: NullableAuthorJSON,
+comments: ConnectionJSON<CommentJSON>, +comments: ConnectionJSON<CommentJSON>,
|}; |};
@ -695,7 +700,7 @@ export type PullRequestJSON = {|
+title: string, +title: string,
+body: string, +body: string,
+number: number, +number: number,
+author: AuthorJSON, +author: NullableAuthorJSON,
+comments: ConnectionJSON<CommentJSON>, +comments: ConnectionJSON<CommentJSON>,
+reviews: ConnectionJSON<PullRequestReviewJSON>, +reviews: ConnectionJSON<PullRequestReviewJSON>,
// If present, oid is the commit SHA of the merged commit. // If present, oid is the commit SHA of the merged commit.
@ -727,7 +732,7 @@ export type CommentJSON = {|
+id: string, +id: string,
+url: string, +url: string,
+body: string, +body: string,
+author: AuthorJSON, +author: NullableAuthorJSON,
|}; |};
function commentsFragment(): FragmentDefinition { function commentsFragment(): FragmentDefinition {
const b = build; const b = build;
@ -754,7 +759,7 @@ export type PullRequestReviewJSON = {|
+id: string, +id: string,
+url: string, +url: string,
+body: string, +body: string,
+author: AuthorJSON, +author: NullableAuthorJSON,
+state: PullRequestReviewState, +state: PullRequestReviewState,
+comments: ConnectionJSON<PullRequestReviewCommentJSON>, +comments: ConnectionJSON<PullRequestReviewCommentJSON>,
|}; |};
@ -779,7 +784,7 @@ export type PullRequestReviewCommentJSON = {|
+id: string, +id: string,
+url: string, +url: string,
+body: string, +body: string,
+author: AuthorJSON, +author: NullableAuthorJSON,
|}; |};
function reviewCommentsFragment(): FragmentDefinition { function reviewCommentsFragment(): FragmentDefinition {
const b = build; const b = build;

View File

@ -27,7 +27,7 @@ import type {
PullRequestJSON, PullRequestJSON,
IssueJSON, IssueJSON,
CommentJSON, CommentJSON,
AuthorJSON, NullableAuthorJSON,
} from "./graphql"; } from "./graphql";
import type {Address} from "../../core/address"; import type {Address} from "../../core/address";
@ -76,8 +76,11 @@ class GithubParser {
| PullRequestReviewCommentNodePayload | PullRequestReviewCommentNodePayload
| PullRequestReviewNodePayload | PullRequestReviewNodePayload
>, >,
authorJson: AuthorJSON authorJson: NullableAuthorJSON
) { ) {
if (authorJson == null) {
return;
}
let authorType: AuthorSubtype; let authorType: AuthorSubtype;
switch (authorJson.__typename) { switch (authorJson.__typename) {
case "User": case "User":