Skip to content

Make bot check contributors before sending heart-warming greeting #2384

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

Merged
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
36 changes: 29 additions & 7 deletions bot/src/dotty/tools/bot/PullRequestService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ trait PullRequestService {
def reviewUrl(issueNbr: Int): String =
withGithubSecret(s"$githubUrl/repos/lampepfl/dotty/pulls/$issueNbr/reviews")

def contributorsUrl: String =
withGithubSecret("https://api.github.com/repos/lampepfl/dotty/contributors")

sealed trait CommitStatus {
def commit: Commit
def isValid: Boolean
Expand Down Expand Up @@ -183,6 +186,13 @@ trait PullRequestService {
.headOption
}

/** Get all contributors from GitHub */
def getContributors(implicit client: Client): Task[Set[String]] =
for {
authors <- client.expect(get(contributorsUrl))(jsonOf[List[Author]])
logins = authors.map(_.login).flatten
} yield logins.toSet

/** Ordered from earliest to latest */
def getCommits(issueNbr: Int)(implicit httpClient: Client): Task[List[Commit]] = {
def makeRequest(url: String): Task[List[Commit]] =
Expand Down Expand Up @@ -226,8 +236,11 @@ trait PullRequestService {
wrongTense || firstLine.last == '.' || firstLine.length > 80
}

def sendInitialComment(issueNbr: Int, invalidUsers: List[String], commits: List[Commit], client: Client): Task[ReviewResponse] = {

/** Returns the body of a `ReviewResponse` */
def sendInitialComment(issueNbr: Int,
invalidUsers: List[String],
commits: List[Commit],
newContributors: Boolean)(implicit client: Client): Task[String] = {
val cla = if (invalidUsers.nonEmpty) {
s"""|## CLA ##
|In order for us to be able to accept your contribution, all users
Expand Down Expand Up @@ -272,9 +285,16 @@ trait PullRequestService {

val review = Review.comment(body)

val shouldPost = newContributors || commitRules.nonEmpty || invalidUsers.nonEmpty

for {
req <- post(reviewUrl(issueNbr)).withAuth(githubUser, githubToken)
res <- client.expect(req.withBody(review.asJson))(jsonOf[ReviewResponse])
res <- {
if (shouldPost)
client.expect(req.withBody(review.asJson))(jsonOf[ReviewResponse]).map(_.body)
else
Task.now("")
}
} yield res
}

Expand All @@ -299,10 +319,12 @@ trait PullRequestService {
setStatus(statuses.last, httpClient)
}

// Send positive comment:
_ <- sendInitialComment(issue.number, invalidUsers, commits, httpClient)
_ <- httpClient.shutdown
resp <- Ok("Fresh PR checked")
authors = commits.map(_.author.login).flatten.toSet
contribs <- getContributors
newContr = !authors.forall(contribs.contains)
_ <- sendInitialComment(issue.number, invalidUsers, commits, newContr)
_ <- httpClient.shutdown
resp <- Ok("Fresh PR checked")
} yield resp

}
Expand Down
16 changes: 11 additions & 5 deletions bot/test/PRServiceTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,14 @@ class PRServiceTests extends PullRequestService {
@Test def canPostReview = {
val invalidUsers = "felixmulder" :: "smarter" :: Nil
val commit = Commit("", Author(Some("smarter")), Author(Some("smarter")), CommitInfo("Added stuff"))
val res = withClient(sendInitialComment(2281, invalidUsers, commit :: Nil, _))
val resBody =
withClient(sendInitialComment(2281, invalidUsers, commit :: Nil, false)(_))

assert(
res.body.contains("We want to keep history") &&
res.body.contains("Could you folks please sign the CLA?") &&
res.body.contains("Have an awesome day!"),
s"Body of review was not as expected:\n${res.body}"
resBody.contains("We want to keep history") &&
resBody.contains("Could you folks please sign the CLA?") &&
resBody.contains("Have an awesome day!"),
s"Body of review was not as expected:\n${resBody}"
)
}

Expand Down Expand Up @@ -159,4 +160,9 @@ class PRServiceTests extends PullRequestService {

assert(respondToComment(issueComment).run.status.code == 200)
}

@Test def canGetContributors = {
val contributors = withClient(getContributors(_))
assert(contributors.contains("felixmulder"))
}
}