diff --git a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala index f77fb8e..885f931 100644 --- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala +++ b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala @@ -125,10 +125,10 @@ (for{ issueId <- params("id").toIntOpt (issue, pullRequest) <- getPullRequest(repository.owner, repository.name, issueId) - users = getAccountsByUserNames(Set(repository.owner, pullRequest.requestUserName, issue.userName), Set()) + users = getAccountsByUserNames(Set(repository.owner, pullRequest.requestUserName, issue.openedUserName), Set()) baseOwner <- users.get(repository.owner) headOwner <- users.get(pullRequest.requestUserName) - issueUser <- users.get(issue.userName) + issueUser <- users.get(issue.openedUserName) headRepo <- getRepository(pullRequest.requestUserName, pullRequest.requestRepositoryName, baseUrl) } yield { JsonFormat(ApiPullRequest( diff --git a/src/main/scala/gitbucket/core/service/IssuesService.scala b/src/main/scala/gitbucket/core/service/IssuesService.scala index bf6d0b2..251abbd 100644 --- a/src/main/scala/gitbucket/core/service/IssuesService.scala +++ b/src/main/scala/gitbucket/core/service/IssuesService.scala @@ -22,10 +22,11 @@ def getComments(owner: String, repository: String, issueId: Int)(implicit s: Session) = IssueComments filter (_.byIssue(owner, repository, issueId)) list - def getCommentsForApi(owner: String, repository: String, issueId: Int)(implicit s: Session) = + /** @return IssueComment and commentedUser */ + def getCommentsForApi(owner: String, repository: String, issueId: Int)(implicit s: Session): List[(IssueComment, Account)] = IssueComments.filter(_.byIssue(owner, repository, issueId)) .filter(_.action inSetBind Set("comment" , "close_comment", "reopen_comment")) - .innerJoin(Accounts).on( (t1, t2) => t1.userName === t2.userName ) + .innerJoin(Accounts).on( (t1, t2) => t1.commentedUserName === t2.userName ) .list def getComment(owner: String, repository: String, commentId: String)(implicit s: Session) = @@ -168,7 +169,7 @@ } /** for api - * @return (issue, commentCount, pullRequest, headRepository, headOwner) + * @return (issue, issueUser, commentCount, pullRequest, headRepo, headOwner) */ def searchPullRequestByApi(condition: IssueSearchCondition, offset: Int, limit: Int, repos: (String, String)*) (implicit s: Session): List[(Issue, Account, Int, PullRequest, Repository, Account)] = { @@ -176,7 +177,7 @@ searchIssueQueryBase(condition, true, offset, limit, repos) .innerJoin(PullRequests).on { case ((t1, t2), t3) => t3.byPrimaryKey(t1.userName, t1.repositoryName, t1.issueId) } .innerJoin(Repositories).on { case (((t1, t2), t3), t4) => t4.byRepository(t1.userName, t1.repositoryName) } - .innerJoin(Accounts).on { case ((((t1, t2), t3), t4), t5) => t5.userName === t1.userName } + .innerJoin(Accounts).on { case ((((t1, t2), t3), t4), t5) => t5.userName === t1.openedUserName } .innerJoin(Accounts).on { case (((((t1, t2), t3), t4), t5), t6) => t6.userName === t4.userName } .map { case (((((t1, t2), t3), t4), t5), t6) => (t1, t5, t2.commentCount, t3, t4, t6) diff --git a/src/main/scala/gitbucket/core/service/WebHookService.scala b/src/main/scala/gitbucket/core/service/WebHookService.scala index 2e8f786..6e7bd63 100644 --- a/src/main/scala/gitbucket/core/service/WebHookService.scala +++ b/src/main/scala/gitbucket/core/service/WebHookService.scala @@ -50,6 +50,7 @@ val f = Future { logger.debug(s"start web hook invocation for ${webHookUrl}") val httpPost = new HttpPost(webHookUrl.url) + httpPost.addHeader("Content-Type", "application/x-www-form-urlencoded") httpPost.addHeader("X-Github-Event", eventName) val params: java.util.List[NameValuePair] = new java.util.ArrayList() @@ -80,10 +81,10 @@ // https://developer.github.com/v3/activity/events/types/#issuesevent def callIssuesWebHook(action: String, repository: RepositoryService.RepositoryInfo, issue: Issue, baseUrl: String, sender: Account)(implicit s: Session, context:JsonFormat.Context): Unit = { callWebHookOf(repository.owner, repository.name, "issues"){ - val users = getAccountsByUserNames(Set(repository.owner, issue.userName), Set(sender)) + val users = getAccountsByUserNames(Set(repository.owner, issue.openedUserName), Set(sender)) for{ repoOwner <- users.get(repository.owner) - issueUser <- users.get(issue.userName) + issueUser <- users.get(issue.openedUserName) } yield { WebHookIssuesPayload( action = action, @@ -100,14 +101,16 @@ callWebHookOf(repository.owner, repository.name, "pull_request"){ for{ (issue, pullRequest) <- getPullRequest(repository.owner, repository.name, issueId) - users = getAccountsByUserNames(Set(repository.owner, pullRequest.requestUserName), Set(sender)) + users = getAccountsByUserNames(Set(repository.owner, pullRequest.requestUserName, issue.openedUserName), Set(sender)) baseOwner <- users.get(repository.owner) headOwner <- users.get(pullRequest.requestUserName) + issueUser <- users.get(issue.openedUserName) headRepo <- getRepository(pullRequest.requestUserName, pullRequest.requestRepositoryName, baseUrl) } yield { WebHookPullRequestPayload( action = action, issue = issue, + issueUser = issueUser, pullRequest = pullRequest, headRepository = headRepo, headOwner = headOwner, @@ -118,8 +121,9 @@ } } + /** @return Map[(issue, issueUser, pullRequest, baseOwner, headOwner), webHooks] */ def getPullRequestsByRequestForWebhook(userName:String, repositoryName:String, branch:String) - (implicit s: Session): Map[(Issue, PullRequest, Account, Account), List[WebHook]] = + (implicit s: Session): Map[(Issue, Account, PullRequest, Account, Account), List[WebHook]] = (for{ is <- Issues if is.closed === false.bind pr <- PullRequests if pr.byPrimaryKey(is.userName, is.repositoryName, is.issueId) @@ -128,20 +132,22 @@ if pr.requestBranch === branch.bind bu <- Accounts if bu.userName === pr.userName ru <- Accounts if ru.userName === pr.requestUserName + iu <- Accounts if iu.userName === is.openedUserName wh <- WebHooks if wh.byRepository(is.userName , is.repositoryName) } yield { - ((is, pr, bu, ru), wh) + ((is, iu, pr, bu, ru), wh) }).list.groupBy(_._1).mapValues(_.map(_._2)) def callPullRequestWebHookByRequestBranch(action: String, requestRepository: RepositoryService.RepositoryInfo, requestBranch: String, baseUrl: String, sender: Account)(implicit s: Session, context:JsonFormat.Context): Unit = { import WebHookService._ for{ - ((issue, pullRequest, baseOwner, headOwner), webHooks) <- getPullRequestsByRequestForWebhook(requestRepository.owner, requestRepository.name, requestBranch) + ((issue, issueUser, pullRequest, baseOwner, headOwner), webHooks) <- getPullRequestsByRequestForWebhook(requestRepository.owner, requestRepository.name, requestBranch) baseRepo <- getRepository(pullRequest.userName, pullRequest.repositoryName, baseUrl) } yield { val payload = WebHookPullRequestPayload( action = action, issue = issue, + issueUser = issueUser, pullRequest = pullRequest, headRepository = requestRepository, headOwner = headOwner, @@ -161,10 +167,10 @@ callWebHookOf(repository.owner, repository.name, "issue_comment"){ for{ issueComment <- getComment(repository.owner, repository.name, issueCommentId.toString()) - users = getAccountsByUserNames(Set(issue.userName, repository.owner, issueComment.userName), Set(sender)) - issueUser <- users.get(issue.userName) + users = getAccountsByUserNames(Set(issue.openedUserName, repository.owner, issueComment.commentedUserName), Set(sender)) + issueUser <- users.get(issue.openedUserName) repoOwner <- users.get(repository.owner) - commenter <- users.get(issueComment.userName) + commenter <- users.get(issueComment.commentedUserName) } yield { WebHookIssueCommentPayload( issue = issue, @@ -224,6 +230,7 @@ object WebHookPullRequestPayload{ def apply(action: String, issue: Issue, + issueUser: Account, pullRequest: PullRequest, headRepository: RepositoryInfo, headOwner: Account, @@ -233,7 +240,7 @@ val headRepoPayload = ApiRepository(headRepository, headOwner) val baseRepoPayload = ApiRepository(baseRepository, baseOwner) val senderPayload = ApiUser(sender) - val pr = ApiPullRequest(issue, pullRequest, headRepoPayload, baseRepoPayload, senderPayload) + val pr = ApiPullRequest(issue, pullRequest, headRepoPayload, baseRepoPayload, ApiUser(issueUser)) WebHookPullRequestPayload( action = action, number = issue.issueId, diff --git a/src/test/scala/gitbucket/core/service/ServiceSpecBase.scala b/src/test/scala/gitbucket/core/service/ServiceSpecBase.scala index 195f7be..f5fd9ea 100644 --- a/src/test/scala/gitbucket/core/service/ServiceSpecBase.scala +++ b/src/test/scala/gitbucket/core/service/ServiceSpecBase.scala @@ -32,9 +32,11 @@ def generateNewAccount(name:String)(implicit s:Session):Account = { AccountService.createAccount(name, name, name, s"${name}@example.com", false, None) - AccountService.getAccountByUserName(name).get + user(name) } + def user(name:String)(implicit s:Session):Account = AccountService.getAccountByUserName(name).get + lazy val dummyService = new RepositoryService with AccountService with IssuesService with PullRequestService with CommitStatusService (){} @@ -44,11 +46,11 @@ ac } - def generateNewIssue(userName:String, repositoryName:String, requestUserName:String="root")(implicit s:Session): Int = { + def generateNewIssue(userName:String, repositoryName:String, loginUser:String="root")(implicit s:Session): Int = { dummyService.createIssue( owner = userName, repository = repositoryName, - loginUser = requestUserName, + loginUser = loginUser, title = "issue title", content = None, assignedUserName = None, @@ -56,10 +58,10 @@ isPullRequest = true) } - def generateNewPullRequest(base:String, request:String)(implicit s:Session):(Issue, PullRequest) = { + def generateNewPullRequest(base:String, request:String, loginUser:String=null)(implicit s:Session):(Issue, PullRequest) = { val Array(baseUserName, baseRepositoryName, baesBranch)=base.split("/") val Array(requestUserName, requestRepositoryName, requestBranch)=request.split("/") - val issueId = generateNewIssue(baseUserName, baseRepositoryName, requestUserName) + val issueId = generateNewIssue(baseUserName, baseRepositoryName, Option(loginUser).getOrElse(requestUserName)) dummyService.createPullRequest( originUserName = baseUserName, originRepositoryName = baseRepositoryName, diff --git a/src/test/scala/gitbucket/core/service/WebHookServiceSpec.scala b/src/test/scala/gitbucket/core/service/WebHookServiceSpec.scala index a3d3d34..1adf8d3 100644 --- a/src/test/scala/gitbucket/core/service/WebHookServiceSpec.scala +++ b/src/test/scala/gitbucket/core/service/WebHookServiceSpec.scala @@ -11,9 +11,10 @@ val user1 = generateNewUserWithDBRepository("user1","repo1") val user2 = generateNewUserWithDBRepository("user2","repo2") val user3 = generateNewUserWithDBRepository("user3","repo3") - val (issue1, pullreq1) = generateNewPullRequest("user1/repo1/master1", "user2/repo2/master2") - val (issue3, pullreq3) = generateNewPullRequest("user3/repo3/master3", "user2/repo2/master2") - val (issue32, pullreq32) = generateNewPullRequest("user3/repo3/master32", "user2/repo2/master2") + val issueUser = user("root") + val (issue1, pullreq1) = generateNewPullRequest("user1/repo1/master1", "user2/repo2/master2", loginUser="root") + val (issue3, pullreq3) = generateNewPullRequest("user3/repo3/master3", "user2/repo2/master2", loginUser="root") + val (issue32, pullreq32) = generateNewPullRequest("user3/repo3/master32", "user2/repo2/master2", loginUser="root") generateNewPullRequest("user2/repo2/master2", "user1/repo1/master2") service.addWebHookURL("user1", "repo1", "webhook1-1") service.addWebHookURL("user1", "repo1", "webhook1-2") @@ -25,18 +26,19 @@ service.getPullRequestsByRequestForWebhook("user1","repo1","master1") must_== Map.empty var r = service.getPullRequestsByRequestForWebhook("user2","repo2","master2").mapValues(_.map(_.url).toSet) + r.size must_== 3 - r((issue1, pullreq1, user1, user2)) must_== Set("webhook1-1","webhook1-2") - r((issue3, pullreq3, user3, user2)) must_== Set("webhook3-1","webhook3-2") - r((issue32, pullreq32, user3, user2)) must_== Set("webhook3-1","webhook3-2") + r((issue1, issueUser, pullreq1, user1, user2)) must_== Set("webhook1-1","webhook1-2") + r((issue3, issueUser, pullreq3, user3, user2)) must_== Set("webhook3-1","webhook3-2") + r((issue32, issueUser, pullreq32, user3, user2)) must_== Set("webhook3-1","webhook3-2") // when closed, it not founds. service.updateClosed("user1","repo1",issue1.issueId, true) var r2 = service.getPullRequestsByRequestForWebhook("user2","repo2","master2").mapValues(_.map(_.url).toSet) r2.size must_== 2 - r2((issue3, pullreq3, user3, user2)) must_== Set("webhook3-1","webhook3-2") - r2((issue32, pullreq32, user3, user2)) must_== Set("webhook3-1","webhook3-2") + r2((issue3, issueUser, pullreq3, user3, user2)) must_== Set("webhook3-1","webhook3-2") + r2((issue32, issueUser, pullreq32, user3, user2)) must_== Set("webhook3-1","webhook3-2") } } } } \ No newline at end of file