Skip to content

Commit 35d178c

Browse files
gerkaigerkai
committedOct 30, 2017
[WIP] check if commit author is PR author
1 parent 368646f commit 35d178c

File tree

6 files changed

+192
-11
lines changed

6 files changed

+192
-11
lines changed
 

‎bin/metadata.js

+15-10
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ function loadQuery(file) {
1111
const PR_QUERY = loadQuery('PR');
1212
const REVIEWS_QUERY = loadQuery('Reviews');
1313
const COMMENTS_QUERY = loadQuery('PRComments');
14-
// const COMMITS_QUERY = loadQuery('PRCommits');
14+
const COMMITS_QUERY = loadQuery('PRCommits');
1515

1616
const { request, requestAll } = require('../lib/request');
1717
const { getCollaborators } = require('../lib/collaborators');
@@ -22,10 +22,9 @@ const MetadataGenerator = require('../lib/metadata_gen');
2222

2323
// const REFERENCE_RE = /referenced this pull request in/
2424

25-
const OWNER = 'nodejs';
26-
const REPO = 'node';
27-
2825
const PR_ID = parsePRId(process.argv[2]);
26+
const OWNER = process.argv[3] || 'nodejs';
27+
const REPO = process.argv[4] || 'node';
2928

3029
async function main(prid, owner, repo) {
3130
logger.trace(`Getting collaborator contacts from README of ${owner}/${repo}`);
@@ -45,23 +44,29 @@ async function main(prid, owner, repo) {
4544
'repository', 'pullRequest', 'comments'
4645
]);
4746

48-
// logger.trace(`Getting commits from ${owner}/${repo}/pull/${prid}`);
49-
// const commits = await requestAll(COMMITS_QUERY, vars, [
50-
// 'repository', 'pullRequest', 'commits'
51-
// ]);
47+
logger.trace(`Getting commits from ${owner}/${repo}/pull/${prid}`);
48+
const commits = await requestAll(COMMITS_QUERY, vars, [
49+
'repository', 'pullRequest', 'commits'
50+
]);
5251

5352
const analyzer = new ReviewAnalyzer(reviews, comments, collaborators);
5453
const reviewers = analyzer.getReviewers();
5554
const metadata = new MetadataGenerator(repo, pr, reviewers).getMetadata();
5655
logger.info({ raw: metadata }, 'Generated metadata:');
5756

58-
const checker = new PRChecker(pr, reviewers, comments, reviews);
57+
const checker = new PRChecker(pr, reviewers, comments, reviews,
58+
commits, collaborators);
5959
checker.checkReviewers();
6060
checker.checkReviews();
6161
checker.checkPRWait();
6262
checker.checkCI();
63-
// TODO: check committers against authors
63+
64+
if (checker.authorIsNew()) {
65+
checker.checkAuthor();
66+
}
6467
// TODO: maybe invalidate review after new commits?
68+
// TODO: check for pre-backport, Github API v4
69+
// does not support reading files changed
6570
}
6671

6772
main(PR_ID, OWNER, REPO).catch((err) => {

‎lib/pr_checker.js

+62-1
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,27 @@ const logger = require('./logger');
1414
const {
1515
REVIEW_SOURCES: { FROM_COMMENT }
1616
} = require('./reviews');
17+
const {
18+
FIRST_TIME_CONTRIBUTOR, FIRST_TIMER
19+
} = require('./user_status');
20+
1721
const CIParser = require('./ci');
1822
const CI_TYPES = CIParser.TYPES;
1923
const { FULL } = CIParser.constants;
2024

25+
/**
26+
* TODO: do not use logger here, put the summaries in arrays
27+
*/
2128
class PRChecker {
22-
constructor(pr, reviewers, comments, reviews) {
29+
constructor(pr, reviewers, comments, reviews, commits, collaborators) {
2330
this.reviewers = reviewers;
2431
this.pr = pr;
2532
this.comments = comments;
2633
this.reviews = reviews;
34+
this.commits = commits;
35+
this.collaboratorEmails = new Set(
36+
Array.from(collaborators).map((c) => c[1].email)
37+
);
2738
}
2839

2940
checkReviews() {
@@ -131,6 +142,56 @@ class PRChecker {
131142
logger.info(`Last ${name} CI on ${ci.date}: ${ci.link}`);
132143
}
133144
}
145+
146+
authorIsNew() {
147+
const assoc = this.pr.authorAssociation;
148+
return assoc === FIRST_TIME_CONTRIBUTOR || assoc === FIRST_TIMER;
149+
}
150+
151+
checkAuthor() {
152+
const oddCommits = this.filterOddCommits(this.commits);
153+
if (!oddCommits.length) {
154+
return;
155+
}
156+
157+
const prAuthor = this.pr.author.login;
158+
logger.warn(`PR is opened by @${prAuthor}`);
159+
for (const c of oddCommits) {
160+
const { oid, author } = c.commit;
161+
const hash = oid.slice(0, 7);
162+
logger.warn(`Author ${author.email} of commit ${hash} ` +
163+
`does not match committer or PR author`);
164+
}
165+
}
166+
167+
filterOddCommits(commits) {
168+
return commits.filter((c) => this.isOddAuthor(c.commit));
169+
}
170+
171+
isOddAuthor(commit) {
172+
// If they have added the alternative email to their account,
173+
// commit.authoredByCommitter should be set to true by Github
174+
if (commit.authoredByCommitter) {
175+
return false;
176+
}
177+
178+
// The commit author is one of the collaborators, they should know
179+
// what they are doing anyway
180+
if (this.collaboratorEmails.has(commit.author.email)) {
181+
return false;
182+
}
183+
184+
if (commit.author.email === this.pr.author.email) {
185+
return false;
186+
}
187+
188+
// At this point, the commit:
189+
// 1. is not authored by the commiter i.e. author email is not in the
190+
// committer's Github account
191+
// 2. is not authored by a collaborator
192+
// 3. is not authored by the people opening the PR
193+
return true;
194+
}
134195
}
135196

136197
module.exports = PRChecker;

‎lib/user_status.js

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
'use strict';
2+
3+
module.exports = {
4+
FIRST_TIME_CONTRIBUTOR: 'FIRST_TIME_CONTRIBUTOR',
5+
NONE: 'NONE',
6+
OWNER: 'OWNER', // TSC
7+
FIRST_TIMER: 'FIRST_TIMER', // New to Github
8+
COLLABORATOR: 'COLLABORATOR'
9+
};

‎queries/PR.gql

+4
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ query PR($prid: Int!, $owner: String!, $repo: String!) {
22
repository(owner: $owner, name: $repo) {
33
pullRequest(number: $prid) {
44
createdAt,
5+
authorAssociation,
6+
author {
7+
login
8+
},
59
url,
610
bodyHTML,
711
bodyText,

‎queries/PRCommits.gql

+4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ query Commits($prid: Int!, $owner: String!, $repo: String!, $after: String) {
33
pullRequest(number: $prid) {
44
commits(first: 100, after: $after) {
55
totalCount
6+
pageInfo {
7+
hasNextPage
8+
endCursor
9+
}
610
nodes {
711
commit {
812
committedDate

‎test/fixtures/oddCommits.json

+98
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
[
2+
{
3+
"commit": {
4+
"committedDate": "2017-10-26T12:35:26Z",
5+
"author": {
6+
"email": "pr_author@example.com",
7+
"name": "Their Github Account email"
8+
},
9+
"committer": {
10+
"email": "pr_author@example.com",
11+
"name": "Their Github Account email"
12+
},
13+
"oid": "ffdef335209c77f66d933bd873950747bfe42264",
14+
"message": "Normal commit, same email for everything",
15+
"authoredByCommitter": true
16+
}
17+
},
18+
{
19+
"commit": {
20+
"committedDate": "2017-09-26T12:35:14Z",
21+
"author": {
22+
"email": "test@example.com",
23+
"name": "Their git config user.email"
24+
},
25+
"committer": {
26+
"email": "pr_author@example.com",
27+
"name": "Their Github Account email"
28+
},
29+
"oid": "e3ad7c72e88c83b7184563e8fdb63df02e2fbacb",
30+
"message": "Either they have not configured git user.email, or have not add the email to their account",
31+
"authoredByCommitter": false
32+
}
33+
},
34+
{
35+
"commit": {
36+
"committedDate": "2017-10-16T12:35:26Z",
37+
"author": {
38+
"email": "test@example.com",
39+
"name": "Their git config user.email"
40+
},
41+
"committer": {
42+
"email": "pr_author@example.com",
43+
"name": "Their Github Account email"
44+
},
45+
"oid": "ff30f335209c77f66d933bd873950747bfe42264",
46+
"message": "They have added the config email to their Github Account, so Github will set authoredByCommitter to true",
47+
"authoredByCommitter": true
48+
}
49+
},
50+
{
51+
"commit": {
52+
"committedDate": "2017-10-17T12:35:26Z",
53+
"author": {
54+
"email": "pr_author@example.com",
55+
"name": "Their git config user.email"
56+
},
57+
"committer": {
58+
"email": "noreply@github.com",
59+
"name": "GitHub"
60+
},
61+
"oid": "ff88f335209c77f66d933bd873950747bfe42264",
62+
"message": "They probably used the Github interface to update PR, the author email should their account email because...it's done on Github",
63+
"authoredByCommitter": false
64+
}
65+
},
66+
{
67+
"commit": {
68+
"committedDate": "2017-10-17T12:35:26Z",
69+
"author": {
70+
"email": "pr_author@example.com",
71+
"name": "Their Github Account email"
72+
},
73+
"committer": {
74+
"email": "baz@gmail.com",
75+
"name": "Baz User"
76+
},
77+
"oid": "da39a3ee5e6b4b0d3255bfef95601890afd80709",
78+
"message": "Edited by a collaborator",
79+
"authoredByCommitter": false
80+
}
81+
},
82+
{
83+
"commit": {
84+
"committedDate": "2017-10-17T12:35:26Z",
85+
"author": {
86+
"email": "test@example.com",
87+
"name": "Their git config user.email"
88+
},
89+
"committer": {
90+
"email": "baz@gmail.com",
91+
"name": "Baz User"
92+
},
93+
"oid": "da39a3ee5e6b4b0d3255bfef95601890afd80709",
94+
"message": "Edited by a collaborator, but the author is not their Github Account",
95+
"authoredByCommitter": false
96+
}
97+
}
98+
]

0 commit comments

Comments
 (0)
Please sign in to comment.