(refs #2)Add tabs to the pull request page.

2025-11-01 11:06:06 +01:00 · 2013-07-14 14:06:48 +09:00
parent 2fb9f83227
commit b68977597b
6 changed files with 215 additions and 61 deletions
--- a/src/main/scala/app/PullRequestsController.scala
+++ b/src/main/scala/app/PullRequestsController.scala
@@ -35,6 +35,60 @@ trait PullRequestsControllerBase extends ControllerBase {
    pulls.html.list(repository)
  })

+  get("/:owner/:repository/pulls/:id")(referrersOnly { repository =>
+    val owner   = repository.owner
+    val name    = repository.name
+    val issueId = params("id").toInt
+
+    getPullRequest(owner, name, issueId) map { case(issue, pullreq) =>
+      pulls.html.pullreq(
+        issue, pullreq,
+        getComments(owner, name, issueId.toInt),
+        (getCollaborators(owner, name) :+ owner).sorted,
+        getMilestones(owner, name),
+        hasWritePermission(owner, name, context.loginAccount),
+        repository)
+    } getOrElse NotFound
+  })
+
+  get("/:owner/:repository/pulls/:id/commits")(referrersOnly { repository =>
+    val owner   = repository.owner
+    val name    = repository.name
+    val issueId = params("id").toInt
+
+    getPullRequest(owner, name, issueId) map { case(issue, pullreq) =>
+      pulls.html.commits(
+        issue, pullreq,
+        getCompareInfo(owner, name, pullreq.branch, pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.requestBranch)._1,
+        hasWritePermission(owner, name, context.loginAccount),
+        repository)
+    } getOrElse NotFound
+  })
+
+  get("/:owner/:repository/pulls/:id/files")(referrersOnly { repository =>
+    val owner   = repository.owner
+    val name    = repository.name
+    val issueId = params("id").toInt
+
+    getPullRequest(owner, name, issueId) map { case(issue, pullreq) =>
+      JGitUtil.withGit(getRepositoryDir(owner, name)){ git =>
+        val newId = git.getRepository.resolve(pullreq.requestBranch)
+
+        pulls.html.files(
+          issue, pullreq,
+          getCompareInfo(owner, name, pullreq.branch, pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.requestBranch)._2,
+          newId.getName,
+          hasWritePermission(owner, name, context.loginAccount),
+          repository)
+      }
+    } getOrElse NotFound
+  })
+
+
+  post("/:owner/:repository/pulls/:id/merge")(collaboratorsOnly { repository =>
+    // TODO Not implemented yet.
+  })
+
  // TODO Replace correct authenticator
  get("/:owner/:repository/pulls/compare")(collaboratorsOnly { newRepo =>
    (newRepo.repository.originUserName, newRepo.repository.originRepositoryName) match {
@@ -64,47 +118,18 @@ trait PullRequestsControllerBase extends ControllerBase {
        repository.repository.originUserName.get,
        repository.repository.originRepositoryName.get, baseUrl
      ).map{ originRepository =>
-
        val Seq(origin, originId, forkedId) = multiParams("splat")
+        val userName       = params("owner")
+        val repositoryName = params("repository")

-        withGit(
-          getRepositoryDir(origin, repository.repository.originRepositoryName.get),
-          getRepositoryDir(params("owner"), params("repository"))
-        ){ (oldGit, newGit) =>
-          val oldReader = oldGit.getRepository.newObjectReader
-          val oldTreeIter = new CanonicalTreeParser
-          oldTreeIter.reset(oldReader, oldGit.getRepository.resolve(s"${originId}^{tree}"))
+        JGitUtil.withGit(getRepositoryDir(userName, repositoryName)){ git =>
+          val newId = git.getRepository.resolve(forkedId)

-          val newReader = newGit.getRepository.newObjectReader
-          val newTreeIter = new CanonicalTreeParser
-          newTreeIter.reset(newReader, newGit.getRepository.resolve(s"${forkedId}^{tree}"))
+          val pullreq = getCompareInfo(
+            origin, repository.repository.originRepositoryName.get, originId,
+            params("owner"), params("repository"), forkedId)

-          import scala.collection.JavaConverters._
-          import util.Implicits._
-
-          val oldId = oldGit.getRepository.resolve(originId)
-          val newId = newGit.getRepository.resolve(forkedId)
-          val i = newGit.log.addRange(oldId, newId).call.iterator
-
-          val commits = new ArrayBuffer[CommitInfo]
-          while(i.hasNext){
-            val revCommit = i.next
-            commits += new CommitInfo(revCommit)
-          }
-
-          val diffs = newGit.diff.setOldTree(oldTreeIter).setNewTree(newTreeIter).call.asScala.map { diff =>
-            if(FileUtil.isImage(diff.getOldPath) || FileUtil.isImage(diff.getNewPath)){
-              DiffInfo(diff.getChangeType, diff.getOldPath, diff.getNewPath, None, None)
-            } else {
-              DiffInfo(diff.getChangeType, diff.getOldPath, diff.getNewPath,
-                JGitUtil.getContent(oldGit, diff.getOldId.toObjectId, false).filter(FileUtil.isText).map(new String(_, "UTF-8")),
-                JGitUtil.getContent(newGit, diff.getNewId.toObjectId, false).filter(FileUtil.isText).map(new String(_, "UTF-8")))
-            }
-          }
-
-          pulls.html.compare(commits.toList.splitWith{ (commit1, commit2) =>
-            view.helpers.date(commit1.time) == view.helpers.date(commit2.time)
-          }, diffs.toList, origin, originId, forkedId, newId.getName, repository, originRepository)
+          pulls.html.compare(pullreq._1, pullreq._2, origin, originId, forkedId, newId.getName, repository, originRepository)
        }
      } getOrElse NotFound
    }
@@ -135,26 +160,6 @@ trait PullRequestsControllerBase extends ControllerBase {
    redirect(s"/${repository.owner}/${repository.name}/pulls/${issueId}")
  })

-  get("/:owner/:repository/pulls/:id")(referrersOnly { repository =>
-    val owner   = repository.owner
-    val name    = repository.name
-    val issueId = params("id").toInt
-
-    getPullRequest(owner, name, issueId) map { case(issue, pullreq) =>
-      pulls.html.pullreq(
-        issue, pullreq,
-        getComments(owner, name, issueId.toInt),
-        (getCollaborators(owner, name) :+ owner).sorted,
-        getMilestones(owner, name),
-        hasWritePermission(owner, name, context.loginAccount),
-        repository)
-    } getOrElse NotFound
-  })
-
-  post("/:owner/:repository/pulls/:id/merge")(collaboratorsOnly { repository =>
-    // TODO Not implemented yet.
-  })
-
  private def withGit[T](oldDir: java.io.File, newDir: java.io.File)(action: (Git, Git) => T): T = {
    val oldGit = Git.open(oldDir)
    val newGit = Git.open(newDir)
@@ -166,4 +171,50 @@ trait PullRequestsControllerBase extends ControllerBase {
    }
  }

+  /**
+   * Returns the commits and diffs between specified repository and revision.
+   */
+  private def getCompareInfo(userName: String, repositoryName: String, branch: String,
+      requestUserName: String, requestRepositoryName: String, requestBranch: String): (Seq[Seq[CommitInfo]], Seq[DiffInfo]) = {
+    withGit(
+      getRepositoryDir(userName, repositoryName),
+      getRepositoryDir(requestUserName, requestRepositoryName)
+    ){ (oldGit, newGit) =>
+      val oldReader = oldGit.getRepository.newObjectReader
+      val oldTreeIter = new CanonicalTreeParser
+      oldTreeIter.reset(oldReader, oldGit.getRepository.resolve(s"${branch}^{tree}"))
+
+      val newReader = newGit.getRepository.newObjectReader
+      val newTreeIter = new CanonicalTreeParser
+      newTreeIter.reset(newReader, newGit.getRepository.resolve(s"${requestBranch}^{tree}"))
+
+      import scala.collection.JavaConverters._
+      import util.Implicits._
+
+      val oldId = oldGit.getRepository.resolve(branch)
+      val newId = newGit.getRepository.resolve(requestBranch)
+      val i = newGit.log.addRange(oldId, newId).call.iterator
+
+      val commits = new ArrayBuffer[CommitInfo]
+      while(i.hasNext){
+        val revCommit = i.next
+        commits += new CommitInfo(revCommit)
+      }
+
+      val diffs = newGit.diff.setOldTree(oldTreeIter).setNewTree(newTreeIter).call.asScala.map { diff =>
+        if(FileUtil.isImage(diff.getOldPath) || FileUtil.isImage(diff.getNewPath)){
+          DiffInfo(diff.getChangeType, diff.getOldPath, diff.getNewPath, None, None)
+        } else {
+          DiffInfo(diff.getChangeType, diff.getOldPath, diff.getNewPath,
+            JGitUtil.getContent(oldGit, diff.getOldId.toObjectId, false).filter(FileUtil.isText).map(new String(_, "UTF-8")),
+            JGitUtil.getContent(newGit, diff.getNewId.toObjectId, false).filter(FileUtil.isText).map(new String(_, "UTF-8")))
+        }
+      }
+
+      (commits.toList.splitWith{ (commit1, commit2) =>
+        view.helpers.date(commit1.time) == view.helpers.date(commit2.time)
+      }, diffs.toSeq)
+    }
+  }
+
 }
--- a/src/main/twirl/pulls/commits.scala.html
+++ b/src/main/twirl/pulls/commits.scala.html
@@ -0,0 +1,32 @@
+@(issue: model.Issue,
+  pullreq: model.PullRequest,
+  commits: Seq[Seq[util.JGitUtil.CommitInfo]],
+  hasWritePermission: Boolean,
+  repository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context)
+@import context._
+@import view.helpers._
+@html.main("%s - Issue #%d - %s/%s".format(issue.title, issue.issueId, repository.owner, repository.name)){
+  @html.header("issues", repository)
+  @tab("commits", issue.issueId, repository)
+  <div class="box">
+    <table class="table table-file-list" style="border: 1px solid silver;">
+      @commits.map { day =>
+        <tr>
+          <th colspan="3" class="box-header" style="font-weight: normal;">@date(day.head.time)</th>
+        </tr>
+        @day.map { commit =>
+          <tr>
+            <td style="width: 20%;">
+              @avatar(commit.committer, 20)
+              <a href="@url(commit.committer)" class="username">@commit.committer</a>
+            </td>
+            <td>@commit.shortMessage</td>
+            <td style="width: 10%; text-align: right;">
+              <a href="@url(repository)/commit/@commit.id" class="monospace">@commit.id.substring(0, 7)</a>
+            </td>
+          </tr>
+        }
+      }
+    </table>
+  </div>
+}
--- a/src/main/twirl/pulls/compare.scala.html
+++ b/src/main/twirl/pulls/compare.scala.html
@@ -1,4 +1,4 @@
-@(commits: Seq[Seq[util.JGitUtil.CommitInfo]], diffs: List[util.JGitUtil.DiffInfo],
+@(commits: Seq[Seq[util.JGitUtil.CommitInfo]], diffs: Seq[util.JGitUtil.DiffInfo],
  origin: String, originId: String, forkedId: String, commitId: String,
  repository: service.RepositoryService.RepositoryInfo,
  originRepository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context)
@@ -33,7 +33,7 @@
    <div style="margin-bottom: 10px;" id="create-pull-request">
      <a href="#" class="btn" id="show-form">Click to create a pull request for this comparison</a>
    </div>
-    <div id="pull-request-form" style="display: none; width: 600px;">
+    <div id="pull-request-form" style="display: none; width: 620px;">
      <form method="POST" action="@path/@origin/@repository.name/pulls/new" validate="true">
        <span class="error" id="error-title"></span>
        <input type="text" name="title" style="width: 600px" placeholder="Title"/>
--- a/src/main/twirl/pulls/files.scala.html
+++ b/src/main/twirl/pulls/files.scala.html
@@ -0,0 +1,52 @@
+@(issue: model.Issue,
+  pullreq: model.PullRequest,
+  diffs: Seq[util.JGitUtil.DiffInfo],
+  commitId: String,
+  hasWritePermission: Boolean,
+  repository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context)
+@import context._
+@import view.helpers._
+@import org.eclipse.jgit.diff.DiffEntry.ChangeType
+@html.main("%s - Issue #%d - %s/%s".format(issue.title, issue.issueId, repository.owner, repository.name)){
+  @html.header("issues", repository)
+  @tab("files", issue.issueId, repository)
+  <div>
+    <div class="pull-right" style="margin-bottom: 10px;">
+      <input id="toggle-file-list" type="button" class="btn" value="Show file list"/>
+    </div>
+    Showing @diffs.size changed @plural(diffs.size, "file")
+  </div>
+  <ul id="commit-file-list" style="display: none;">
+  @diffs.zipWithIndex.map { case (diff, i) =>
+    <li@if(i > 0){ class="border"}>
+      <a href="#diff-@i">
+        @if(diff.changeType == ChangeType.COPY || diff.changeType == ChangeType.RENAME){
+          <img src="@assets/common/images/diff_move.png"/> @diff.oldPath -> @diff.newPath
+        }
+        @if(diff.changeType == ChangeType.ADD){
+          <img src="@assets/common/images/diff_add.png"/> @diff.newPath
+        }
+        @if(diff.changeType == ChangeType.MODIFY){
+          <img src="@assets/common/images/diff_edit.png"/> @diff.newPath
+        }
+        @if(diff.changeType == ChangeType.DELETE){
+          <img src="@assets/common/images/diff_delete.png"/> @diff.oldPath
+        }
+      </a>
+    </li>
+  }
+  </ul>
+  @helper.html.diff(diffs, repository, Some(commitId))
+}
+<script>
+$(function(){
+  $('#toggle-file-list').click(function(){
+    $('#commit-file-list').toggle();
+    if($(this).val() == 'Show file list'){
+      $(this).val('Hide file list');
+    } else {
+      $(this).val('Show file list');
+    }
+  });
+});
+</script>
--- a/src/main/twirl/pulls/pullreq.scala.html
+++ b/src/main/twirl/pulls/pullreq.scala.html
@@ -9,11 +9,13 @@
@import view.helpers._
@html.main("%s - Issue #%d - %s/%s".format(issue.title, issue.issueId, repository.owner, repository.name)){
  @html.header("issues", repository)
-  @issues.html.tab("issues", repository)
+  @tab("discussion", issue.issueId, repository)
+  @*
  <ul class="nav nav-tabs">
    <li class="pull-left"><a href="@url(repository)/issues"><i class="icon-arrow-left"></i> Back to issue list</a></li>
    <li class="pull-right">Issue #@issue.issueId</li>
  </ul>
+  *@
  <div class="row-fluid">
    <div class="span10">
      <div class="issue-avatar-image">@avatar(issue.openedUserName, 48)</div>
@@ -140,7 +142,7 @@ git push origin @{pullreq.branch}</pre>
            <div>
              <strong>Merge pull request #@issue.issueId from @{pullreq.requestUserName}/@{pullreq.requestBranch}</strong>
            </div>
-            <textarea name="content" style="width: 680px; height: 100px;">@issue.title</textarea>
+            <textarea name="content" style="width: 680px; height: 80px;">@issue.title</textarea>
            <div>
              <input type="button" class="btn" value="Cancel" id="cancel-merge-pull-request"/>
              <input type="submit" class="btn btn-success" value="Confirm merge"/>
--- a/src/main/twirl/pulls/tab.scala.html
+++ b/src/main/twirl/pulls/tab.scala.html
@@ -0,0 +1,17 @@
+@(active: String, issueId: Int, repository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context)
+@import context._
+@import view.helpers._
+<ul class="nav nav-tabs">
+  <li@if(active == "discussion"){ class="active"}><a href="@url(repository)/pulls/@issueId">Discussion</a></li>
+  <li@if(active == "commits"){ class="active"}><a href="@url(repository)/pulls/@issueId/commits">Commits</a></li>
+  <li@if(active == "files"){ class="active"}><a href="@url(repository)/pulls/@issueId/files">Files Changed</a></li>
+  @*
+  @if(loginAccount.isDefined){
+  <li class="pull-right">
+    <div class="btn-group">
+      <a class="btn btn-success" href="@url(repository)/issues/new">New Issue</a>
+    </div>
+  </li>
+  }
+  *@
+</ul>