status-im
/
sourcecred
mirror of https://github.com/status-im/sourcecred.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fix a crash when a review comment has >5 reactions (#855) 

Our continuation-fetching code failed to properly get continuations for
pull request review comments, because it was only asking for more
reactions on `"IssueComment"` fragments. This caused the
`ensureNoMorePages` function to properly throw an error rather than
proceding with incomplete data.

This commit fixes the root cause by splitting
`continuationsFromComment`into `continuationsFromReviewComment` and
`continuationsFromIssueComment`. (Pull and issue comments are both
considered 'IssueComment's.) The example-github repository has been
updated to include 10 reactions to a single review comment; the
example-data was updated in this commit, and all reactions have been
loaded.

I've also added a `console.error` statement in `ensureNoMorePages`. This
only triggers when the program is about to fail, and it's useful for
debugging.

Test plan: `yarn test --full` passes.

Paired with @wchargin
This commit is contained in:
Dandelion Mané 2018-09-18 12:52:05 -07:00 committed by GitHub
parent fa81c4eaa9
commit f03e3c83f2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 524 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

156
src/plugins/github/__snapshots__/createGraph.test.js.snap
View File

@ -1635,6 +1635,58 @@ Array [
"dstIndex": 23, "dstIndex": 23,
"srcIndex": 42, "srcIndex": 42,
}, },
Object {
"address": Array [
"sourcecred",
"github",
"REACTS",
"HEART",
"5",
"sourcecred",
"github",
"USERLIKE",
"USER",
"decentralion",
"9",
"sourcecred",
"github",
"COMMENT",
"REVIEW",
"sourcecred",
"example-github",
"5",
"100313899",
"171460198",
],
"dstIndex": 24,
"srcIndex": 42,
},
Object {
"address": Array [
"sourcecred",
"github",
"REACTS",
"HEART",
"5",
"sourcecred",
"github",
"USERLIKE",
"USER",
"wchargin",
"9",
"sourcecred",
"github",
"COMMENT",
"REVIEW",
"sourcecred",
"example-github",
"5",
"100313899",
"171460198",
],
"dstIndex": 24,
"srcIndex": 43,
},
Object { Object {
"address": Array [ "address": Array [
"sourcecred", "sourcecred",
@ -1683,6 +1735,58 @@ Array [
"dstIndex": 8, "dstIndex": 8,
"srcIndex": 42, "srcIndex": 42,
}, },
Object {
"address": Array [
"sourcecred",
"github",
"REACTS",
"HOORAY",
"5",
"sourcecred",
"github",
"USERLIKE",
"USER",
"decentralion",
"9",
"sourcecred",
"github",
"COMMENT",
"REVIEW",
"sourcecred",
"example-github",
"5",
"100313899",
"171460198",
],
"dstIndex": 24,
"srcIndex": 42,
},
Object {
"address": Array [
"sourcecred",
"github",
"REACTS",
"HOORAY",
"5",
"sourcecred",
"github",
"USERLIKE",
"USER",
"wchargin",
"9",
"sourcecred",
"github",
"COMMENT",
"REVIEW",
"sourcecred",
"example-github",
"5",
"100313899",
"171460198",
],
"dstIndex": 24,
"srcIndex": 43,
},
Object { Object {
"address": Array [ "address": Array [
"sourcecred", "sourcecred",
@ -1777,6 +1881,58 @@ Array [
"dstIndex": 8, "dstIndex": 8,
"srcIndex": 42, "srcIndex": 42,
}, },
Object {
"address": Array [
"sourcecred",
"github",
"REACTS",
"THUMBS_UP",
"5",
"sourcecred",
"github",
"USERLIKE",
"USER",
"decentralion",
"9",
"sourcecred",
"github",
"COMMENT",
"REVIEW",
"sourcecred",
"example-github",
"5",
"100313899",
"171460198",
],
"dstIndex": 24,
"srcIndex": 42,
},
Object {
"address": Array [
"sourcecred",
"github",
"REACTS",
"THUMBS_UP",
"5",
"sourcecred",
"github",
"USERLIKE",
"USER",
"wchargin",
"9",
"sourcecred",
"github",
"COMMENT",
"REVIEW",
"sourcecred",
"example-github",
"5",
"100313899",
"171460198",
],
"dstIndex": 24,
"srcIndex": 43,
},
Object { Object {
"address": Array [ "address": Array [
"sourcecred", "sourcecred",

199
src/plugins/github/__snapshots__/relationalView.test.js.snap
View File

@ -16,7 +16,106 @@ Object {
} }
`; `;
exports[`plugins/github/relationalView Comment has reactions 1`] = `Array []`; exports[`plugins/github/relationalView Comment has reactions 1`] = `
Array [
Object {
"content": "THUMBS_UP",
"user": Object {
"login": "wchargin",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "THUMBS_DOWN",
"user": Object {
"login": "wchargin",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "THUMBS_UP",
"user": Object {
"login": "decentralion",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "LAUGH",
"user": Object {
"login": "wchargin",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "THUMBS_DOWN",
"user": Object {
"login": "decentralion",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "HOORAY",
"user": Object {
"login": "wchargin",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "LAUGH",
"user": Object {
"login": "decentralion",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "CONFUSED",
"user": Object {
"login": "wchargin",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "HOORAY",
"user": Object {
"login": "decentralion",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "HEART",
"user": Object {
"login": "wchargin",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "CONFUSED",
"user": Object {
"login": "decentralion",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "HEART",
"user": Object {
"login": "decentralion",
"subtype": "USER",
"type": "USERLIKE",
},
},
]
`;
exports[`plugins/github/relationalView Comment has url 1`] = `"https://github.com/sourcecred/example-github/pull/5#discussion_r171460198"`; exports[`plugins/github/relationalView Comment has url 1`] = `"https://github.com/sourcecred/example-github/pull/5#discussion_r171460198"`;
@ -405,6 +504,104 @@ Object {
}, },
}, },
], ],
"https://github.com/sourcecred/example-github/pull/5#discussion_r171460198": Array [
Object {
"content": "THUMBS_UP",
"user": Object {
"login": "wchargin",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "THUMBS_DOWN",
"user": Object {
"login": "wchargin",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "THUMBS_UP",
"user": Object {
"login": "decentralion",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "LAUGH",
"user": Object {
"login": "wchargin",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "THUMBS_DOWN",
"user": Object {
"login": "decentralion",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "HOORAY",
"user": Object {
"login": "wchargin",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "LAUGH",
"user": Object {
"login": "decentralion",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "CONFUSED",
"user": Object {
"login": "wchargin",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "HOORAY",
"user": Object {
"login": "decentralion",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "HEART",
"user": Object {
"login": "wchargin",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "CONFUSED",
"user": Object {
"login": "decentralion",
"subtype": "USER",
"type": "USERLIKE",
},
},
Object {
"content": "HEART",
"user": Object {
"login": "decentralion",
"subtype": "USER",
"type": "USERLIKE",
},
},
],
"https://github.com/sourcecred/example-github/pull/5#issuecomment-396430464": Array [ "https://github.com/sourcecred/example-github/pull/5#issuecomment-396430464": Array [
Object { Object {
"content": "HEART", "content": "HEART",

122
src/plugins/github/example/example-github.json
View File

@ -1000,9 +1000,129 @@
"id": "MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDE3MTQ2MDE5OA==", "id": "MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDE3MTQ2MDE5OA==",
"reactions": { "reactions": {
"nodes": [ "nodes": [
{
"content": "THUMBS_UP",
"id": "MDg6UmVhY3Rpb24yOTU5NTI1MQ==",
"user": {
"__typename": "User",
"id": "MDQ6VXNlcjQzMTc4MDY=",
"login": "wchargin",
"url": "https://github.com/wchargin"
}
},
{
"content": "THUMBS_DOWN",
"id": "MDg6UmVhY3Rpb24yOTU5NTI1NA==",
"user": {
"__typename": "User",
"id": "MDQ6VXNlcjQzMTc4MDY=",
"login": "wchargin",
"url": "https://github.com/wchargin"
}
},
{
"content": "THUMBS_UP",
"id": "MDg6UmVhY3Rpb24yOTU5NTI1NQ==",
"user": {
"__typename": "User",
"id": "MDQ6VXNlcjE0MDAwMjM=",
"login": "decentralion",
"url": "https://github.com/decentralion"
}
},
{
"content": "LAUGH",
"id": "MDg6UmVhY3Rpb24yOTU5NTI1Nw==",
"user": {
"__typename": "User",
"id": "MDQ6VXNlcjQzMTc4MDY=",
"login": "wchargin",
"url": "https://github.com/wchargin"
}
},
{
"content": "THUMBS_DOWN",
"id": "MDg6UmVhY3Rpb24yOTU5NTI1OA==",
"user": {
"__typename": "User",
"id": "MDQ6VXNlcjE0MDAwMjM=",
"login": "decentralion",
"url": "https://github.com/decentralion"
}
},
{
"content": "HOORAY",
"id": "MDg6UmVhY3Rpb24yOTU5NTI1OQ==",
"user": {
"__typename": "User",
"id": "MDQ6VXNlcjQzMTc4MDY=",
"login": "wchargin",
"url": "https://github.com/wchargin"
}
},
{
"content": "LAUGH",
"id": "MDg6UmVhY3Rpb24yOTU5NTI2MQ==",
"user": {
"__typename": "User",
"id": "MDQ6VXNlcjE0MDAwMjM=",
"login": "decentralion",
"url": "https://github.com/decentralion"
}
},
{
"content": "CONFUSED",
"id": "MDg6UmVhY3Rpb24yOTU5NTI2NQ==",
"user": {
"__typename": "User",
"id": "MDQ6VXNlcjQzMTc4MDY=",
"login": "wchargin",
"url": "https://github.com/wchargin"
}
},
{
"content": "HOORAY",
"id": "MDg6UmVhY3Rpb24yOTU5NTI2Ng==",
"user": {
"__typename": "User",
"id": "MDQ6VXNlcjE0MDAwMjM=",
"login": "decentralion",
"url": "https://github.com/decentralion"
}
},
{
"content": "HEART",
"id": "MDg6UmVhY3Rpb24yOTU5NTI2OA==",
"user": {
"__typename": "User",
"id": "MDQ6VXNlcjQzMTc4MDY=",
"login": "wchargin",
"url": "https://github.com/wchargin"
}
},
{
"content": "CONFUSED",
"id": "MDg6UmVhY3Rpb24yOTU5NTI2OQ==",
"user": {
"__typename": "User",
"id": "MDQ6VXNlcjE0MDAwMjM=",
"login": "decentralion",
"url": "https://github.com/decentralion"
}
},
{
"content": "HEART",
"id": "MDg6UmVhY3Rpb24yOTU5NTI3NA==",
"user": {
"__typename": "User",
"id": "MDQ6VXNlcjE0MDAwMjM=",
"login": "decentralion",
"url": "https://github.com/decentralion"
}
}
], ],
"pageInfo": { "pageInfo": {
"endCursor": null, "endCursor": "Y3Vyc29yOnYyOpHOAcOWig==",
"hasNextPage": false "hasNextPage": false
} }
}, },

1
src/plugins/github/fetchGithubRepo.js
View File

@ -188,6 +188,7 @@ function ensureNoMorePages(result: any, path = []) {
} }
if (result.pageInfo) { if (result.pageInfo) {
if (result.pageInfo.hasNextPage) { if (result.pageInfo.hasNextPage) {
console.error(result);
throw new Error(`More pages at: ${path.join()}`); throw new Error(`More pages at: ${path.join()}`);
} }
} }

54
src/plugins/github/graphql.js
View File

@ -87,7 +87,13 @@ const PAGE_SIZE_REACTIONS = 5;
* the continuation into a query, and not of the continuation itself. * the continuation into a query, and not of the continuation itself.
*/ */
export type Continuation = {| export type Continuation = {|
+enclosingNodeType: "REPOSITORY" | "ISSUE" | "PULL" | "REVIEW" | "COMMENT", +enclosingNodeType:
| "REPOSITORY"
| "ISSUE"
| "PULL"
| "REVIEW"
| "ISSUE_COMMENT"
| "REVIEW_COMMENT",
+enclosingNodeId: string, +enclosingNodeId: string,
+selections: $ReadOnlyArray<Selection>, +selections: $ReadOnlyArray<Selection>,
+destinationPath: $ReadOnlyArray<string | number>, +destinationPath: $ReadOnlyArray<string | number>,
@ -217,7 +223,8 @@ export function continuationsFromContinuation(
ISSUE: continuationsFromIssue, ISSUE: continuationsFromIssue,
PULL: continuationsFromPR, PULL: continuationsFromPR,
REVIEW: continuationsFromReview, REVIEW: continuationsFromReview,
COMMENT: continuationsFromComment, ISSUE_COMMENT: continuationsFromIssueComment,
REVIEW_COMMENT: continuationsFromReviewComment,
}[source.enclosingNodeType]; }[source.enclosingNodeType];
return continuationsFromEnclosingType( return continuationsFromEnclosingType(
result, result,
@ -368,7 +375,7 @@ function* continuationsFromIssue(
for (let i = 0; i < result.comments.nodes.length; i++) { for (let i = 0; i < result.comments.nodes.length; i++) {
const comment = result.comments.nodes[i]; const comment = result.comments.nodes[i];
const subpath = [...path, "comments", "nodes", i]; const subpath = [...path, "comments", "nodes", i];
yield* continuationsFromComment(comment, comment.id, subpath); yield* continuationsFromIssueComment(comment, comment.id, subpath);
} }
} }
} }
@ -447,7 +454,7 @@ function* continuationsFromPR(
for (let i = 0; i < result.comments.nodes.length; i++) { for (let i = 0; i < result.comments.nodes.length; i++) {
const comment = result.comments.nodes[i]; const comment = result.comments.nodes[i];
const subpath = [...path, "comments", "nodes", i]; const subpath = [...path, "comments", "nodes", i];
yield* continuationsFromComment(comment, comment.id, subpath); yield* continuationsFromIssueComment(comment, comment.id, subpath);
} }
} }
} }
@ -477,9 +484,17 @@ function* continuationsFromReview(
destinationPath: path, destinationPath: path,
}; };
} }
if (result.comments) {
for (let i = 0; i < result.comments.nodes.length; i++) {
const comment = result.comments.nodes[i];
const subpath = [...path, "comments", "nodes", i];
yield* continuationsFromReviewComment(comment, comment.id, subpath);
}
}
} }
function* continuationsFromComment( // Pull Request comments are also issue comments.
function* continuationsFromIssueComment(
result: any, result: any,
nodeId: string, nodeId: string,
path: $ReadOnlyArray<string | number> path: $ReadOnlyArray<string | number>
@ -487,7 +502,7 @@ function* continuationsFromComment(
const b = build; const b = build;
if (result.reactions && result.reactions.pageInfo.hasNextPage) { if (result.reactions && result.reactions.pageInfo.hasNextPage) {
yield { yield {
enclosingNodeType: "COMMENT", enclosingNodeType: "ISSUE_COMMENT",
enclosingNodeId: nodeId, enclosingNodeId: nodeId,
selections: [ selections: [
b.inlineFragment("IssueComment", [ b.inlineFragment("IssueComment", [
@ -506,6 +521,33 @@ function* continuationsFromComment(
} }
} }
function* continuationsFromReviewComment(
result: any,
nodeId: string,
path: $ReadOnlyArray<string | number>
): Iterator<Continuation> {
const b = build;
if (result.reactions && result.reactions.pageInfo.hasNextPage) {
yield {
enclosingNodeType: "REVIEW_COMMENT",
enclosingNodeId: nodeId,
selections: [
b.inlineFragment("PullRequestReviewComment", [
b.field(
"reactions",
{
first: b.literal(PAGE_LIMIT),
after: b.literal(result.reactions.pageInfo.endCursor),
},
[b.fragmentSpread("reactions")]
),
]),
],
destinationPath: path,
};
}
}
/** /**
* Execute the given query, returning all pages of data throughout the * Execute the given query, returning all pages of data throughout the
* query. That is: post the query, then find any entities that require * query. That is: post the query, then find any entities that require