diff --git a/src/main/scala/app/PullRequestsController.scala b/src/main/scala/app/PullRequestsController.scala index 0d42d92..8219b9c 100644 --- a/src/main/scala/app/PullRequestsController.scala +++ b/src/main/scala/app/PullRequestsController.scala @@ -76,19 +76,24 @@ getMilestonesWithIssueCount(owner, name), commits, diffs, - if(issue.closed){ - false - } else { - checkConflict(owner, name, pullreq.branch, owner, name, pullreq.requestBranch) - }, hasWritePermission(owner, name, context.loginAccount), - repository, - s"${baseUrl}${context.path}/git/${pullreq.requestUserName}/${pullreq.requestRepositoryName}.git") + repository) } } getOrElse NotFound } }) + ajaxGet("/:owner/:repository/pull/:id/mergeguide")(collaboratorsOnly { repository => + defining(repository.owner, repository.name, params("id").toInt){ case (owner, name, issueId) => + getPullRequest(owner, name, issueId) map { case(issue, pullreq) => + pulls.html.mergeguide( + checkConflict(owner, name, pullreq.branch, owner, name, pullreq.requestBranch), + pullreq, + s"${baseUrl}${context.path}/git/${pullreq.requestUserName}/${pullreq.requestRepositoryName}.git") + } getOrElse NotFound() + } + }) + post("/:owner/:repository/pull/:id/merge", mergeForm)(collaboratorsOnly { (form, repository) => defining(repository.owner, repository.name, params("id").toInt){ case (owner, name, issueId) => LockUtil.lock(s"${owner}/${name}/merge"){ @@ -164,41 +169,6 @@ } }) - /** - * Checks whether conflict will be caused in merging. - * Returns true if conflict will be caused. - */ - private def checkConflict(userName: String, repositoryName: String, branch: String, - requestUserName: String, requestRepositoryName: String, requestBranch: String): Boolean = { - // TODO Are there more quick way? - LockUtil.lock(s"${userName}/${repositoryName}/merge-check"){ - val remote = getRepositoryDir(userName, repositoryName) - val tmpdir = new java.io.File(getTemporaryDir(userName, repositoryName), "merge-check") - if(tmpdir.exists()){ - FileUtils.deleteDirectory(tmpdir) - } - - val git = Git.cloneRepository.setDirectory(tmpdir).setURI(remote.toURI.toString).setBranch(branch).call - try { - git.checkout.setName(branch).call - - git.fetch - .setRemote(getRepositoryDir(requestUserName, requestRepositoryName).toURI.toString) - .setRefSpecs(new RefSpec(s"refs/heads/${branch}:refs/heads/${requestBranch}")).call - - val result = git.merge - .include(git.getRepository.resolve("FETCH_HEAD")) - .setCommit(false).call - - result.getConflicts != null - - } finally { - git.getRepository.close - FileUtils.deleteDirectory(tmpdir) - } - } - } - get("/:owner/:repository/compare")(referrersOnly { forkedRepository => (forkedRepository.repository.originUserName, forkedRepository.repository.originRepositoryName) match { case (Some(originUserName), Some(originRepositoryName)) => { @@ -229,11 +199,11 @@ val (forkedOwner, tmpForkedBranch) = parseCompareIdentifie(forked, repository.owner) (getRepository(originOwner, repository.name, baseUrl), - getRepository(forkedOwner, repository.name, baseUrl)) match { + getRepository(forkedOwner, repository.name, baseUrl)) match { case (Some(originRepository), Some(forkedRepository)) => { using( Git.open(getRepositoryDir(originOwner, repository.name)), - Git.open(getRepositoryDir(forkedOwner, repository.name)) + Git.open(getRepositoryDir(forkedOwner, repository.name)) ){ case (oldGit, newGit) => val originBranch = JGitUtil.getDefaultBranch(oldGit, originRepository, tmpOriginBranch).get._2 val forkedBranch = JGitUtil.getDefaultBranch(newGit, forkedRepository, tmpForkedBranch).get._2 @@ -259,7 +229,6 @@ forkedBranch, oldId.getName, newId.getName, - checkConflict(originOwner, repository.name, originBranch, forkedOwner, repository.name, forkedBranch), repository, originRepository, forkedRepository, @@ -270,6 +239,29 @@ } }) + ajaxGet("/:owner/:repository/compare/*...*/mergecheck")(collaboratorsOnly { repository => + val Seq(origin, forked) = multiParams("splat") + val (originOwner, tmpOriginBranch) = parseCompareIdentifie(origin, repository.owner) + val (forkedOwner, tmpForkedBranch) = parseCompareIdentifie(forked, repository.owner) + + (getRepository(originOwner, repository.name, baseUrl), + getRepository(forkedOwner, repository.name, baseUrl)) match { + case (Some(originRepository), Some(forkedRepository)) => { + using( + Git.open(getRepositoryDir(originOwner, repository.name)), + Git.open(getRepositoryDir(forkedOwner, repository.name)) + ){ case (oldGit, newGit) => + val originBranch = JGitUtil.getDefaultBranch(oldGit, originRepository, tmpOriginBranch).get._2 + val forkedBranch = JGitUtil.getDefaultBranch(newGit, forkedRepository, tmpForkedBranch).get._2 + + pulls.html.mergecheck( + checkConflict(originOwner, repository.name, originBranch, forkedOwner, repository.name, forkedBranch)) + } + } + case _ => NotFound() + } + }) + post("/:owner/:repository/pulls/new", pullRequestForm)(referrersOnly { (form, repository) => val loginUserName = context.loginAccount.get.userName @@ -314,6 +306,40 @@ }) /** + * Checks whether conflict will be caused in merging. Returns true if conflict will be caused. + */ + private def checkConflict(userName: String, repositoryName: String, branch: String, + requestUserName: String, requestRepositoryName: String, requestBranch: String): Boolean = { + // TODO Are there more quick way? + LockUtil.lock(s"${userName}/${repositoryName}/merge-check"){ + val remote = getRepositoryDir(userName, repositoryName) + val tmpdir = new java.io.File(getTemporaryDir(userName, repositoryName), "merge-check") + if(tmpdir.exists()){ + FileUtils.deleteDirectory(tmpdir) + } + + val git = Git.cloneRepository.setDirectory(tmpdir).setURI(remote.toURI.toString).setBranch(branch).call + try { + git.checkout.setName(branch).call + + git.fetch + .setRemote(getRepositoryDir(requestUserName, requestRepositoryName).toURI.toString) + .setRefSpecs(new RefSpec(s"refs/heads/${branch}:refs/heads/${requestBranch}")).call + + val result = git.merge + .include(git.getRepository.resolve("FETCH_HEAD")) + .setCommit(false).call + + result.getConflicts != null + + } finally { + git.getRepository.close + FileUtils.deleteDirectory(tmpdir) + } + } + } + + /** * Parses branch identifier and extracts owner and branch name as tuple. * * - "owner:branch" to ("owner", "branch") @@ -382,7 +408,7 @@ getPullRequestCountGroupByUser(condition.state == "closed", owner, Some(repoName)), userName, page, - countIssue(condition.copy(state = "open"), filterUser, true, owner -> repoName), + countIssue(condition.copy(state = "open" ), filterUser, true, owner -> repoName), countIssue(condition.copy(state = "closed"), filterUser, true, owner -> repoName), countIssue(condition, Map.empty, true, owner -> repoName), condition, diff --git a/src/main/twirl/pulls/compare.scala.html b/src/main/twirl/pulls/compare.scala.html index d523d2d..3c893a2 100644 --- a/src/main/twirl/pulls/compare.scala.html +++ b/src/main/twirl/pulls/compare.scala.html @@ -5,7 +5,6 @@ forkedId: String, sourceId: String, commitId: String, - hasConflict: Boolean, repository: service.RepositoryService.RepositoryInfo, originRepository: service.RepositoryService.RepositoryInfo, forkedRepository: service.RepositoryService.RepositoryInfo, @@ -52,14 +51,9 @@
- @if(hasConflict){ -

We can’t automatically merge these branches

-

Don't worry, you can still submit the pull request.

- } else { -

Able to merge

-

These branches can be automatically merged.

- } - +
@@ -86,29 +80,6 @@ } else { - @* -
- - @commits.map { day => - - - - @day.map { commit => - - - - - - } - } -
@date(day.head.time)
- @avatar(commit.committer, 20) - @commit.committer - @commit.shortMessage - @commit.id.substring(0, 7) -
-
- *@ @pulls.html.commits(commits, repository) @helper.html.diff(diffs, repository, Some(commitId), Some(sourceId), true) } @@ -148,5 +119,27 @@ $(this).hide(); $('#pull-request-form').show(); }); + + @if(hasWritePermission){ + function checkConflict(from, to, noConflictHandler, hasConflictHandler){ + $('.check-conflict').show(); + $.get('@url(repository)/compare/' + from + '...' + to + '/mergecheck', + function(data){ $('.check-conflict').html(data); }); + } + + @if(members.isEmpty){ + checkConflict( + $.trim($('i.icon-ok').parents('a.origin-branch').data('name')), + $.trim($('i.icon-ok').parents('a.forked-branch').data('name')) + ); + } else { + checkConflict( + $.trim($('i.icon-ok').parents('a.origin-owner' ).data('name')) + ":" + + $.trim($('i.icon-ok').parents('a.origin-branch').data('name')), + $.trim($('i.icon-ok').parents('a.forked-owner' ).data('name')) + ":" + + $.trim($('i.icon-ok').parents('a.forked-branch').data('name')) + ); + } + } }); diff --git a/src/main/twirl/pulls/discussion.scala.html b/src/main/twirl/pulls/discussion.scala.html index 770a2d6..91b90c0 100644 --- a/src/main/twirl/pulls/discussion.scala.html +++ b/src/main/twirl/pulls/discussion.scala.html @@ -3,10 +3,8 @@ comments: List[model.IssueComment], collaborators: List[String], milestones: List[(model.Milestone, Int, Int)], - hasConflict: Boolean, hasWritePermission: Boolean, - repository: service.RepositoryService.RepositoryInfo, - requestRepositoryUrl: String)(implicit context: app.Context) + repository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context) @import context._ @import view.helpers._
@@ -17,71 +15,8 @@
-
- -
-
- @if(hasConflict){ - We can’t automatically merge this pull request. - } else { - This pull request can be automatically merged. - } -
-
- @if(hasConflict){ - Use the command line to resolve conflicts before continuing. - } else { - You can also merge branches on the command line. - } -
- \ No newline at end of file diff --git a/src/main/twirl/pulls/mergecheck.scala.html b/src/main/twirl/pulls/mergecheck.scala.html new file mode 100644 index 0000000..52c12ee --- /dev/null +++ b/src/main/twirl/pulls/mergecheck.scala.html @@ -0,0 +1,9 @@ +@(hasConflict: Boolean) +@if(hasConflict){ +

We can’t automatically merge these branches

+

Don't worry, you can still submit the pull request.

+} else { +

Able to merge

+

These branches can be automatically merged.

+} + diff --git a/src/main/twirl/pulls/mergeguide.scala.html b/src/main/twirl/pulls/mergeguide.scala.html new file mode 100644 index 0000000..b2b84c5 --- /dev/null +++ b/src/main/twirl/pulls/mergeguide.scala.html @@ -0,0 +1,84 @@ +@(hasConflict: Boolean, + pullreq: model.PullRequest, + requestRepositoryUrl: String)(implicit context: app.Context) +@import context._ +@import view.helpers._ +
+ +
+
+ @if(hasConflict){ + We can’t automatically merge this pull request. + } else { + This pull request can be automatically merged. + } +
+
+ @if(hasConflict){ + Use the command line to resolve conflicts before continuing. + } else { + You can also merge branches on the command line. + } +
+ + \ No newline at end of file diff --git a/src/main/twirl/pulls/pullreq.scala.html b/src/main/twirl/pulls/pullreq.scala.html index 1713ddf..025c5c0 100644 --- a/src/main/twirl/pulls/pullreq.scala.html +++ b/src/main/twirl/pulls/pullreq.scala.html @@ -5,10 +5,8 @@ milestones: List[(model.Milestone, Int, Int)], dayByDayCommits: Seq[Seq[util.JGitUtil.CommitInfo]], diffs: Seq[util.JGitUtil.DiffInfo], - hasConflict: Boolean, hasWritePermission: Boolean, - repository: service.RepositoryService.RepositoryInfo, - requestRepositoryUrl: String)(implicit context: app.Context) + repository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context) @import context._ @import view.helpers._ @html.main("%s - Pull Request #%d - %s/%s".format(issue.title, issue.issueId, repository.owner, repository.name)){ @@ -39,7 +37,7 @@
- @pulls.html.discussion(issue, pullreq, comments, collaborators, milestones, hasConflict, hasWritePermission, repository, requestRepositoryUrl) + @pulls.html.discussion(issue, pullreq, comments, collaborators, milestones, hasWritePermission, repository)
@pulls.html.commits(dayByDayCommits, repository)