From 00eab5d584e764feaaa94362c619077d6cedea15 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Sun, 11 Dec 2016 14:16:35 +0900 Subject: [PATCH 1/6] (refs #1348)Keep pull request comment if new commits are pushed --- build.sbt | 2 + .../core/controller/ApiController.scala | 1 + .../core/controller/DashboardController.scala | 6 +- .../core/controller/IssuesController.scala | 29 ++++++- .../controller/PullRequestsController.scala | 39 ++++----- .../core/service/CommitsService.scala | 10 ++- .../core/service/PullRequestService.scala | 87 ++++++++++++++++++- .../core/servlet/GitRepositoryServlet.scala | 2 +- .../gitbucket/core/helper/diff.scala.html | 23 +++-- 9 files changed, 155 insertions(+), 44 deletions(-) diff --git a/build.sbt b/build.sbt index df90251db..e87fe2630 100644 --- a/build.sbt +++ b/build.sbt @@ -15,6 +15,7 @@ scalaVersion := "2.11.8" // dependency settings resolvers ++= Seq( Classpaths.typesafeReleases, + Resolver.jcenterRepo, "amateras" at "http://amateras.sourceforge.jp/mvn/", "sonatype-snapshot" at "https://oss.sonatype.org/content/repositories/snapshots/", "amateras-snapshot" at "http://amateras.sourceforge.jp/mvn-snapshot/" @@ -45,6 +46,7 @@ libraryDependencies ++= Seq( "com.typesafe" % "config" % "1.3.0", "com.typesafe.akka" %% "akka-actor" % "2.3.15", "fr.brouillard.oss.security.xhub" % "xhub4j-core" % "1.0.0", + "com.github.bkromhout" % "java-diff-utils" % "2.1.1", "com.enragedginger" %% "akka-quartz-scheduler" % "1.4.0-akka-2.3.x" exclude("c3p0","c3p0"), "org.eclipse.jetty" % "jetty-webapp" % JettyVersion % "provided", "javax.servlet" % "javax.servlet-api" % "3.1.0" % "provided", diff --git a/src/main/scala/gitbucket/core/controller/ApiController.scala b/src/main/scala/gitbucket/core/controller/ApiController.scala index ba32532e1..344969e9a 100644 --- a/src/main/scala/gitbucket/core/controller/ApiController.scala +++ b/src/main/scala/gitbucket/core/controller/ApiController.scala @@ -22,6 +22,7 @@ class ApiController extends ApiControllerBase with IssuesService with LabelsService with PullRequestService + with CommitsService with CommitStatusService with RepositoryCreationService with HandleCommentService diff --git a/src/main/scala/gitbucket/core/controller/DashboardController.scala b/src/main/scala/gitbucket/core/controller/DashboardController.scala index 1227f24f5..9aec00f16 100644 --- a/src/main/scala/gitbucket/core/controller/DashboardController.scala +++ b/src/main/scala/gitbucket/core/controller/DashboardController.scala @@ -1,13 +1,13 @@ package gitbucket.core.controller import gitbucket.core.dashboard.html -import gitbucket.core.service.{RepositoryService, PullRequestService, AccountService, IssuesService} -import gitbucket.core.util.{StringUtil, Keys, UsersAuthenticator} +import gitbucket.core.service._ +import gitbucket.core.util.{Keys, UsersAuthenticator} import gitbucket.core.util.Implicits._ import gitbucket.core.service.IssuesService._ class DashboardController extends DashboardControllerBase - with IssuesService with PullRequestService with RepositoryService with AccountService + with IssuesService with PullRequestService with RepositoryService with AccountService with CommitsService with UsersAuthenticator trait DashboardControllerBase extends ControllerBase { diff --git a/src/main/scala/gitbucket/core/controller/IssuesController.scala b/src/main/scala/gitbucket/core/controller/IssuesController.scala index f0afabb1a..4222dba92 100644 --- a/src/main/scala/gitbucket/core/controller/IssuesController.scala +++ b/src/main/scala/gitbucket/core/controller/IssuesController.scala @@ -14,12 +14,33 @@ import org.scalatra.Ok class IssuesController extends IssuesControllerBase - with IssuesService with RepositoryService with AccountService with LabelsService with MilestonesService with ActivityService with HandleCommentService - with ReadableUsersAuthenticator with ReferrerAuthenticator with WritableUsersAuthenticator with PullRequestService with WebHookIssueCommentService + with IssuesService + with RepositoryService + with AccountService + with LabelsService + with MilestonesService + with ActivityService + with HandleCommentService + with ReadableUsersAuthenticator + with ReferrerAuthenticator + with WritableUsersAuthenticator + with PullRequestService + with WebHookIssueCommentService + with CommitsService trait IssuesControllerBase extends ControllerBase { - self: IssuesService with RepositoryService with AccountService with LabelsService with MilestonesService with ActivityService with HandleCommentService - with ReadableUsersAuthenticator with ReferrerAuthenticator with WritableUsersAuthenticator with PullRequestService with WebHookIssueCommentService => + self: IssuesService + with RepositoryService + with AccountService + with LabelsService + with MilestonesService + with ActivityService + with HandleCommentService + with ReadableUsersAuthenticator + with ReferrerAuthenticator + with WritableUsersAuthenticator + with PullRequestService + with WebHookIssueCommentService => case class IssueCreateForm(title: String, content: Option[String], assignedUserName: Option[String], milestoneId: Option[Int], labelNames: Option[String]) diff --git a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala index ebcc95326..52f675b3d 100644 --- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala +++ b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala @@ -11,7 +11,6 @@ import gitbucket.core.service._ import gitbucket.core.util.ControlUtil._ import gitbucket.core.util.Directory._ import gitbucket.core.util.Implicits._ -import gitbucket.core.util.JGitUtil._ import gitbucket.core.util._ import gitbucket.core.view import gitbucket.core.view.helpers @@ -498,25 +497,25 @@ trait PullRequestsControllerBase extends ControllerBase { (defaultOwner, value) } - private def getRequestCompareInfo(userName: String, repositoryName: String, branch: String, - requestUserName: String, requestRepositoryName: String, requestCommitId: String): (Seq[Seq[CommitInfo]], Seq[DiffInfo]) = - using( - Git.open(getRepositoryDir(userName, repositoryName)), - Git.open(getRepositoryDir(requestUserName, requestRepositoryName)) - ){ (oldGit, newGit) => - val oldId = oldGit.getRepository.resolve(branch) - val newId = newGit.getRepository.resolve(requestCommitId) - - val commits = newGit.log.addRange(oldId, newId).call.iterator.asScala.map { revCommit => - new CommitInfo(revCommit) - }.toList.splitWith { (commit1, commit2) => - helpers.date(commit1.commitTime) == view.helpers.date(commit2.commitTime) - } - - val diffs = JGitUtil.getDiffs(newGit, oldId.getName, newId.getName, true) - - (commits, diffs) - } +// private def getRequestCompareInfo(userName: String, repositoryName: String, branch: String, +// requestUserName: String, requestRepositoryName: String, requestCommitId: String): (Seq[Seq[CommitInfo]], Seq[DiffInfo]) = +// using( +// Git.open(getRepositoryDir(userName, repositoryName)), +// Git.open(getRepositoryDir(requestUserName, requestRepositoryName)) +// ){ (oldGit, newGit) => +// val oldId = oldGit.getRepository.resolve(branch) +// val newId = newGit.getRepository.resolve(requestCommitId) +// +// val commits = newGit.log.addRange(oldId, newId).call.iterator.asScala.map { revCommit => +// new CommitInfo(revCommit) +// }.toList.splitWith { (commit1, commit2) => +// helpers.date(commit1.commitTime) == view.helpers.date(commit2.commitTime) +// } +// +// val diffs = JGitUtil.getDiffs(newGit, oldId.getName, newId.getName, true) +// +// (commits, diffs) +// } private def searchPullRequests(userName: Option[String], repository: RepositoryService.RepositoryInfo) = defining(repository.owner, repository.name){ case (owner, repoName) => diff --git a/src/main/scala/gitbucket/core/service/CommitsService.scala b/src/main/scala/gitbucket/core/service/CommitsService.scala index 5ccffd530..229519da9 100644 --- a/src/main/scala/gitbucket/core/service/CommitsService.scala +++ b/src/main/scala/gitbucket/core/service/CommitsService.scala @@ -42,12 +42,18 @@ trait CommitsService { updatedDate = currentDate, issueId = issueId) + def updateCommitCommentPosition(commentId: Int, commitId: String, oldLine: Option[Int], newLine: Option[Int])(implicit s: Session): Unit = + CommitComments.filter(_.byPrimaryKey(commentId)) + .map { t => + (t.commitId, t.oldLine, t.newLine) + }.update(commitId, oldLine, newLine) + def updateCommitComment(commentId: Int, content: String)(implicit s: Session) = CommitComments .filter (_.byPrimaryKey(commentId)) .map { t => - t.content -> t.updatedDate - }.update (content, currentDate) + t.content -> t.updatedDate + }.update (content, currentDate) def deleteCommitComment(commentId: Int)(implicit s: Session) = CommitComments filter (_.byPrimaryKey(commentId)) delete diff --git a/src/main/scala/gitbucket/core/service/PullRequestService.scala b/src/main/scala/gitbucket/core/service/PullRequestService.scala index e90ba3fcb..6f94ab3b5 100644 --- a/src/main/scala/gitbucket/core/service/PullRequestService.scala +++ b/src/main/scala/gitbucket/core/service/PullRequestService.scala @@ -1,12 +1,22 @@ package gitbucket.core.service -import gitbucket.core.model.{Account, Issue, PullRequest, WebHook, CommitStatus, CommitState} +import difflib.{Delta, DiffUtils} +import gitbucket.core.model.{Session => _, _} import gitbucket.core.model.Profile._ +import gitbucket.core.util.ControlUtil._ +import gitbucket.core.util.Directory._ +import gitbucket.core.util.Implicits._ import gitbucket.core.util.JGitUtil +import gitbucket.core.util.JGitUtil.{CommitInfo, DiffInfo} +import gitbucket.core.view +import gitbucket.core.view.helpers +import org.eclipse.jgit.api.Git import profile.simple._ +import scala.collection.JavaConverters._ -trait PullRequestService { self: IssuesService => + +trait PullRequestService { self: IssuesService with CommitsService => import PullRequestService._ def getPullRequest(owner: String, repository: String, issueId: Int) @@ -111,9 +121,26 @@ trait PullRequestService { self: IssuesService => def updatePullRequests(owner: String, repository: String, branch: String)(implicit s: Session): Unit = getPullRequestsByRequest(owner, repository, branch, false).foreach { pullreq => if(Repositories.filter(_.byRepository(pullreq.userName, pullreq.repositoryName)).exists.run){ + // Update the git repository val (commitIdTo, commitIdFrom) = JGitUtil.updatePullRequest( pullreq.userName, pullreq.repositoryName, pullreq.branch, pullreq.issueId, pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.requestBranch) + + // Update comments position + val comments = getCommitComments(pullreq.userName, pullreq.repositoryName, pullreq.commitIdTo, true) + println(comments) + comments.foreach { comment => + comment match { + case CommitComment(_, _, _, commitId, _, _, Some(fileName), _, Some(newLine), _, _, _) => { + getNewLineNumber(fileName, newLine, pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo, commitIdTo).foreach { lineNumber => + updateCommitCommentPosition(commitId, commitIdTo, None, Some(lineNumber)) + } + } + case _ => // Do nothing + } + } + + // Update commit id in the PULL_REQUEST table updateCommitId(pullreq.userName, pullreq.repositoryName, pullreq.issueId, commitIdTo, commitIdFrom) } } @@ -137,6 +164,62 @@ trait PullRequestService { self: IssuesService => .firstOption } } + + def getNewLineNumber(file: String, lineNumber: Int, userName: String, repositoryName: String, oldCommitId: String, newCommitId: String): Option[Int] = { + val (_, diffs) = getRequestCompareInfo(userName, repositoryName, oldCommitId, userName, repositoryName, newCommitId) + + var counter = lineNumber + + diffs.find(x => x.oldPath == "test").foreach { diff => + (diff.oldContent, diff.newContent) match { + case (Some(oldContent), Some(newContent)) => { + val oldLines = oldContent.replace("\r\n", "\n").split("\n") + val newLines = newContent.replace("\r\n", "\n").split("\n") + val deltas = DiffUtils.diff(oldLines.toList.asJava, newLines.toList.asJava) + + deltas.getDeltas.asScala.filter(_.getOriginal.getPosition < lineNumber).foreach { delta => + delta.getType match { + case Delta.TYPE.CHANGE => + if(delta.getOriginal.getPosition <= lineNumber - 1 && lineNumber <= delta.getOriginal.getPosition + delta.getRevised.getLines.size){ + counter = -1 + } else { + counter = counter + (delta.getRevised.getLines.size - delta.getOriginal.getLines.size) + } + case Delta.TYPE.INSERT => + counter = counter + delta.getRevised.getLines.size + case Delta.TYPE.DELETE => + counter = counter - delta.getOriginal.getLines.size + } + } + counter + } + case _ => counter = -1 + } + } + + if(counter >= 0) Some(counter) else None + } + + def getRequestCompareInfo(userName: String, repositoryName: String, branch: String, + requestUserName: String, requestRepositoryName: String, requestCommitId: String): (Seq[Seq[CommitInfo]], Seq[DiffInfo]) = + using( + Git.open(getRepositoryDir(userName, repositoryName)), + Git.open(getRepositoryDir(requestUserName, requestRepositoryName)) + ){ (oldGit, newGit) => + val oldId = oldGit.getRepository.resolve(branch) + val newId = newGit.getRepository.resolve(requestCommitId) + + val commits = newGit.log.addRange(oldId, newId).call.iterator.asScala.map { revCommit => + new CommitInfo(revCommit) + }.toList.splitWith { (commit1, commit2) => + helpers.date(commit1.commitTime) == view.helpers.date(commit2.commitTime) + } + + val diffs = JGitUtil.getDiffs(newGit, oldId.getName, newId.getName, true) + + (commits, diffs) + } + } object PullRequestService { diff --git a/src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala b/src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala index ad8bc03dc..76f1f0631 100644 --- a/src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala +++ b/src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala @@ -110,7 +110,7 @@ import scala.collection.JavaConverters._ class CommitLogHook(owner: String, repository: String, pusher: String, baseUrl: String)(implicit session: Session) extends PostReceiveHook with PreReceiveHook with RepositoryService with AccountService with IssuesService with ActivityService with PullRequestService with WebHookService - with WebHookPullRequestService { + with WebHookPullRequestService with CommitsService { private val logger = LoggerFactory.getLogger(classOf[CommitLogHook]) private var existIds: Seq[String] = Nil diff --git a/src/main/twirl/gitbucket/core/helper/diff.scala.html b/src/main/twirl/gitbucket/core/helper/diff.scala.html index cb56b0193..17fdea22b 100644 --- a/src/main/twirl/gitbucket/core/helper/diff.scala.html +++ b/src/main/twirl/gitbucket/core/helper/diff.scala.html @@ -240,7 +240,6 @@ $(function(){ var oldline = $v.attr('oldline'); var newline = $v.attr('newline'); var tmp; - var diff; if (typeof oldline !== 'undefined') { if (typeof newline !== 'undefined') { tmp = getInlineContainer(); @@ -248,13 +247,11 @@ $(function(){ tmp = getInlineContainer('old'); } tmp.children('td:first').html($v.clone().show()); - diff.find('table.diff').find('.oldline[line-number=' + oldline + ']') - .parent().nextAll(':not(.not-diff):first').before(tmp); + diff.find('table.diff').find('.oldline[line-number=' + oldline + ']').parent().nextAll(':not(.not-diff):first').before(tmp); } else { tmp = getInlineContainer('new'); tmp.children('td:last').html($v.clone().show()); - diff.find('table.diff').find('.newline[line-number=' + newline + ']') - .parent().nextAll(':not(.not-diff):first').before(tmp); + diff.find('table.diff').find('.newline[line-number=' + newline + ']').parent().nextAll(':not(.not-diff):first').before(tmp); } if (!diff.find('.toggle-notes').prop('checked')) { tmp.hide(); @@ -263,10 +260,10 @@ $(function(){ function renderStatBar(add, del){ if(add + del > 5){ if(add){ - if(add+'+add+'-'+del+'').append(renderStatBar(add,del).attr('title',(add+del)+" lines changed").tooltip()); + $('span.diffstat[data-diff-id="'+i+'"]') + .html('+' + add + '-' + del + '') + .append(renderStatBar(add, del).attr('title', (add + del) + " lines changed").tooltip()); @if(hasWritePermission) { diffText.find('.body').each(function(){ $('+').prependTo(this); }); @@ -301,7 +300,7 @@ $(function(){ @if(showLineNotes){ var fileName = table.attr('filename'); $('.inline-comment').each(function(i, v) { - if($(this).attr('filename')==fileName){ + if($(this).attr('filename') == fileName){ renderOneCommitCommentIntoDiff($(this), table); } }); From 0f6a433623126cc11b96bb5eb1bb798db8a8fe6c Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Sun, 11 Dec 2016 16:49:47 +0900 Subject: [PATCH 2/6] (refs #1348)Fix testcase --- .../scala/gitbucket/core/service/PullRequestServiceSpec.scala | 2 +- src/test/scala/gitbucket/core/service/ServiceSpecBase.scala | 2 +- src/test/scala/gitbucket/core/service/WebHookServiceSpec.scala | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/scala/gitbucket/core/service/PullRequestServiceSpec.scala b/src/test/scala/gitbucket/core/service/PullRequestServiceSpec.scala index d6ddff9e7..a7b8050ef 100644 --- a/src/test/scala/gitbucket/core/service/PullRequestServiceSpec.scala +++ b/src/test/scala/gitbucket/core/service/PullRequestServiceSpec.scala @@ -4,7 +4,7 @@ import gitbucket.core.model._ import org.scalatest.FunSpec class PullRequestServiceSpec extends FunSpec with ServiceSpecBase - with PullRequestService with IssuesService with AccountService with RepositoryService { + with PullRequestService with IssuesService with AccountService with RepositoryService with CommitsService { def swap(r: (Issue, PullRequest)) = (r._2 -> r._1) diff --git a/src/test/scala/gitbucket/core/service/ServiceSpecBase.scala b/src/test/scala/gitbucket/core/service/ServiceSpecBase.scala index 0bf208ba1..16fd952bc 100644 --- a/src/test/scala/gitbucket/core/service/ServiceSpecBase.scala +++ b/src/test/scala/gitbucket/core/service/ServiceSpecBase.scala @@ -44,7 +44,7 @@ trait ServiceSpecBase { 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 with LabelsService (){} + with CommitsService with CommitStatusService with LabelsService (){} def generateNewUserWithDBRepository(userName:String, repositoryName:String)(implicit s:Session):Account = { val ac = AccountService.getAccountByUserName(userName).getOrElse(generateNewAccount(userName)) diff --git a/src/test/scala/gitbucket/core/service/WebHookServiceSpec.scala b/src/test/scala/gitbucket/core/service/WebHookServiceSpec.scala index c4c2c7d3a..238cafb31 100644 --- a/src/test/scala/gitbucket/core/service/WebHookServiceSpec.scala +++ b/src/test/scala/gitbucket/core/service/WebHookServiceSpec.scala @@ -6,7 +6,7 @@ import gitbucket.core.model.WebHookContentType class WebHookServiceSpec extends FunSuite with ServiceSpecBase { - lazy val service = new WebHookPullRequestService with AccountService with RepositoryService with PullRequestService with IssuesService + lazy val service = new WebHookPullRequestService with AccountService with RepositoryService with PullRequestService with IssuesService with CommitsService test("WebHookPullRequestService.getPullRequestsByRequestForWebhook") { withTestDB { implicit session => val user1 = generateNewUserWithDBRepository("user1","repo1") From 907532fd13976771590084af5e2844f617d58585 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Sun, 11 Dec 2016 17:17:41 +0900 Subject: [PATCH 3/6] (refs #1348)Clean up --- .../controller/PullRequestsController.scala | 22 ------------------- .../core/service/PullRequestService.scala | 1 - 2 files changed, 23 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala index 52f675b3d..671d9f156 100644 --- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala +++ b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala @@ -12,8 +12,6 @@ import gitbucket.core.util.ControlUtil._ import gitbucket.core.util.Directory._ import gitbucket.core.util.Implicits._ import gitbucket.core.util._ -import gitbucket.core.view -import gitbucket.core.view.helpers import io.github.gitbucket.scalatra.forms._ import org.eclipse.jgit.api.Git import org.eclipse.jgit.lib.PersonIdent @@ -497,26 +495,6 @@ trait PullRequestsControllerBase extends ControllerBase { (defaultOwner, value) } -// private def getRequestCompareInfo(userName: String, repositoryName: String, branch: String, -// requestUserName: String, requestRepositoryName: String, requestCommitId: String): (Seq[Seq[CommitInfo]], Seq[DiffInfo]) = -// using( -// Git.open(getRepositoryDir(userName, repositoryName)), -// Git.open(getRepositoryDir(requestUserName, requestRepositoryName)) -// ){ (oldGit, newGit) => -// val oldId = oldGit.getRepository.resolve(branch) -// val newId = newGit.getRepository.resolve(requestCommitId) -// -// val commits = newGit.log.addRange(oldId, newId).call.iterator.asScala.map { revCommit => -// new CommitInfo(revCommit) -// }.toList.splitWith { (commit1, commit2) => -// helpers.date(commit1.commitTime) == view.helpers.date(commit2.commitTime) -// } -// -// val diffs = JGitUtil.getDiffs(newGit, oldId.getName, newId.getName, true) -// -// (commits, diffs) -// } - private def searchPullRequests(userName: Option[String], repository: RepositoryService.RepositoryInfo) = defining(repository.owner, repository.name){ case (owner, repoName) => val page = IssueSearchCondition.page(request) diff --git a/src/main/scala/gitbucket/core/service/PullRequestService.scala b/src/main/scala/gitbucket/core/service/PullRequestService.scala index 6f94ab3b5..6f3ab427d 100644 --- a/src/main/scala/gitbucket/core/service/PullRequestService.scala +++ b/src/main/scala/gitbucket/core/service/PullRequestService.scala @@ -128,7 +128,6 @@ trait PullRequestService { self: IssuesService with CommitsService => // Update comments position val comments = getCommitComments(pullreq.userName, pullreq.repositoryName, pullreq.commitIdTo, true) - println(comments) comments.foreach { comment => comment match { case CommitComment(_, _, _, commitId, _, _, Some(fileName), _, Some(newLine), _, _, _) => { From 158f799ca16a2865f85029c529ad877c0fd14c5f Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Sun, 11 Dec 2016 21:57:37 +0900 Subject: [PATCH 4/6] (refs #1348)Improve efficiency of updating comment positions --- .../core/service/PullRequestService.scala | 89 +++++++++++-------- 1 file changed, 52 insertions(+), 37 deletions(-) diff --git a/src/main/scala/gitbucket/core/service/PullRequestService.scala b/src/main/scala/gitbucket/core/service/PullRequestService.scala index 6f3ab427d..cd29357b4 100644 --- a/src/main/scala/gitbucket/core/service/PullRequestService.scala +++ b/src/main/scala/gitbucket/core/service/PullRequestService.scala @@ -126,18 +126,18 @@ trait PullRequestService { self: IssuesService with CommitsService => pullreq.userName, pullreq.repositoryName, pullreq.branch, pullreq.issueId, pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.requestBranch) - // Update comments position - val comments = getCommitComments(pullreq.userName, pullreq.repositoryName, pullreq.commitIdTo, true) - comments.foreach { comment => - comment match { - case CommitComment(_, _, _, commitId, _, _, Some(fileName), _, Some(newLine), _, _, _) => { - getNewLineNumber(fileName, newLine, pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo, commitIdTo).foreach { lineNumber => - updateCommitCommentPosition(commitId, commitIdTo, None, Some(lineNumber)) - } - } - case _ => // Do nothing + // Collect comment positions + val positions = getCommitComments(pullreq.userName, pullreq.repositoryName, pullreq.commitIdTo, true) + .collect { + case CommitComment(_, _, _, commentId, _, _, Some(file), _, Some(newLine), _, _, _) => (file, commentId, newLine) } - } + .groupBy { case (file, _, _) => file } + .map { case (file, comments) => file -> + comments.map { case (_, commentId, lineNumber) => (commentId, lineNumber) } + } + + // Update comments position + updatePullRequestCommentPositions(positions, pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo, commitIdTo) // Update commit id in the PULL_REQUEST table updateCommitId(pullreq.userName, pullreq.repositoryName, pullreq.issueId, commitIdTo, commitIdFrom) @@ -164,39 +164,54 @@ trait PullRequestService { self: IssuesService with CommitsService => } } - def getNewLineNumber(file: String, lineNumber: Int, userName: String, repositoryName: String, oldCommitId: String, newCommitId: String): Option[Int] = { + private def updatePullRequestCommentPositions(positions: Map[String, Seq[(Int, Int)]], userName: String, repositoryName: String, + oldCommitId: String, newCommitId: String)(implicit s: Session): Unit = { + val (_, diffs) = getRequestCompareInfo(userName, repositoryName, oldCommitId, userName, repositoryName, newCommitId) - var counter = lineNumber - - diffs.find(x => x.oldPath == "test").foreach { diff => - (diff.oldContent, diff.newContent) match { - case (Some(oldContent), Some(newContent)) => { - val oldLines = oldContent.replace("\r\n", "\n").split("\n") - val newLines = newContent.replace("\r\n", "\n").split("\n") - val deltas = DiffUtils.diff(oldLines.toList.asJava, newLines.toList.asJava) - - deltas.getDeltas.asScala.filter(_.getOriginal.getPosition < lineNumber).foreach { delta => - delta.getType match { - case Delta.TYPE.CHANGE => - if(delta.getOriginal.getPosition <= lineNumber - 1 && lineNumber <= delta.getOriginal.getPosition + delta.getRevised.getLines.size){ - counter = -1 - } else { - counter = counter + (delta.getRevised.getLines.size - delta.getOriginal.getLines.size) - } - case Delta.TYPE.INSERT => - counter = counter + delta.getRevised.getLines.size - case Delta.TYPE.DELETE => - counter = counter - delta.getOriginal.getLines.size - } + val patchs = positions.map { case (file, _) => + diffs.find(x => x.oldPath == file).map { diff => + (diff.oldContent, diff.newContent) match { + case (Some(oldContent), Some(newContent)) => { + val oldLines = oldContent.replace("\r\n", "\n").split("\n") + val newLines = newContent.replace("\r\n", "\n").split("\n") + file -> Option(DiffUtils.diff(oldLines.toList.asJava, newLines.toList.asJava)) } - counter + case _ => + file -> None } - case _ => counter = -1 + }.getOrElse { + file -> None } } - if(counter >= 0) Some(counter) else None + positions.foreach { case (file, comments) => + patchs(file) match { + case Some(patch) => { + file -> comments.foreach { case (commentId, lineNumber) => + var counter = lineNumber + patch.getDeltas.asScala.filter(_.getOriginal.getPosition < lineNumber).foreach { delta => + delta.getType match { + case Delta.TYPE.CHANGE => + if(delta.getOriginal.getPosition <= lineNumber - 1 && lineNumber <= delta.getOriginal.getPosition + delta.getRevised.getLines.size){ + counter = -1 + } else { + counter = counter + (delta.getRevised.getLines.size - delta.getOriginal.getLines.size) + } + case Delta.TYPE.INSERT => counter = counter + delta.getRevised.getLines.size + case Delta.TYPE.DELETE => counter = counter - delta.getOriginal.getLines.size + } + } + if(counter >= 0){ + updateCommitCommentPosition(commentId, newCommitId, None, Some(counter)) + } + } + } + case _ => comments.foreach { case (commentId, lineNumber) => + updateCommitCommentPosition(commentId, newCommitId, None, Some(lineNumber)) + } + } + } } def getRequestCompareInfo(userName: String, repositoryName: String, branch: String, From 1c660523727b648d87eb94f799d1081e7b7b6d65 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Mon, 12 Dec 2016 01:30:10 +0900 Subject: [PATCH 5/6] (refs #1348)Keep comments on the old line --- .../core/service/PullRequestService.scala | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/main/scala/gitbucket/core/service/PullRequestService.scala b/src/main/scala/gitbucket/core/service/PullRequestService.scala index cd29357b4..cbd3df097 100644 --- a/src/main/scala/gitbucket/core/service/PullRequestService.scala +++ b/src/main/scala/gitbucket/core/service/PullRequestService.scala @@ -129,7 +129,8 @@ trait PullRequestService { self: IssuesService with CommitsService => // Collect comment positions val positions = getCommitComments(pullreq.userName, pullreq.repositoryName, pullreq.commitIdTo, true) .collect { - case CommitComment(_, _, _, commentId, _, _, Some(file), _, Some(newLine), _, _, _) => (file, commentId, newLine) + case CommitComment(_, _, _, commentId, _, _, Some(file), None, Some(newLine), _, _, _) => (file, commentId, Right(newLine)) + case CommitComment(_, _, _, commentId, _, _, Some(file), Some(oldLine), None, _, _, _) => (file, commentId, Left(oldLine)) } .groupBy { case (file, _, _) => file } .map { case (file, comments) => file -> @@ -164,7 +165,7 @@ trait PullRequestService { self: IssuesService with CommitsService => } } - private def updatePullRequestCommentPositions(positions: Map[String, Seq[(Int, Int)]], userName: String, repositoryName: String, + private def updatePullRequestCommentPositions(positions: Map[String, Seq[(Int, Either[Int, Int])]], userName: String, repositoryName: String, oldCommitId: String, newCommitId: String)(implicit s: Session): Unit = { val (_, diffs) = getRequestCompareInfo(userName, repositoryName, oldCommitId, userName, repositoryName, newCommitId) @@ -187,13 +188,14 @@ trait PullRequestService { self: IssuesService with CommitsService => positions.foreach { case (file, comments) => patchs(file) match { - case Some(patch) => { - file -> comments.foreach { case (commentId, lineNumber) => - var counter = lineNumber - patch.getDeltas.asScala.filter(_.getOriginal.getPosition < lineNumber).foreach { delta => + case Some(patch) => file -> comments.foreach { case (commentId, lineNumber) => lineNumber match { + case Left(oldLine) => updateCommitCommentPosition(commentId, newCommitId, Some(oldLine), None) + case Right(newLine) => + var counter = newLine + patch.getDeltas.asScala.filter(_.getOriginal.getPosition < newLine).foreach { delta => delta.getType match { case Delta.TYPE.CHANGE => - if(delta.getOriginal.getPosition <= lineNumber - 1 && lineNumber <= delta.getOriginal.getPosition + delta.getRevised.getLines.size){ + if(delta.getOriginal.getPosition <= newLine - 1 && newLine <= delta.getOriginal.getPosition + delta.getRevised.getLines.size){ counter = -1 } else { counter = counter + (delta.getRevised.getLines.size - delta.getOriginal.getLines.size) @@ -205,11 +207,11 @@ trait PullRequestService { self: IssuesService with CommitsService => if(counter >= 0){ updateCommitCommentPosition(commentId, newCommitId, None, Some(counter)) } - } - } - case _ => comments.foreach { case (commentId, lineNumber) => - updateCommitCommentPosition(commentId, newCommitId, None, Some(lineNumber)) - } + }} + case _ => comments.foreach { case (commentId, lineNumber) => lineNumber match { + case Right(oldLine) => updateCommitCommentPosition(commentId, newCommitId, Some(oldLine), None) + case Left(newLine) => updateCommitCommentPosition(commentId, newCommitId, None, Some(newLine)) + }} } } } From f0e27758613290a25760e45e29df12cbedc0a5ed Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Mon, 12 Dec 2016 17:50:59 +0900 Subject: [PATCH 6/6] (refs #1348)Show commentted filename, commit id and pull request id on the header --- .../controller/RepositoryViewerController.scala | 2 +- .../gitbucket/core/helper/commitcomment.scala.html | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala index 92f9bbf13..701a66c23 100644 --- a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala +++ b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala @@ -328,7 +328,7 @@ trait RepositoryViewerControllerBase extends ControllerBase { html.commit(id, new JGitUtil.CommitInfo(revCommit), JGitUtil.getBranchesOfCommit(git, revCommit.getName), JGitUtil.getTagsOfCommit(git, revCommit.getName), - getCommitComments(repository.owner, repository.name, id, false), + getCommitComments(repository.owner, repository.name, id, true), repository, diffs, oldCommitId, hasDeveloperRole(repository.owner, repository.name, context.loginAccount)) } } diff --git a/src/main/twirl/gitbucket/core/helper/commitcomment.scala.html b/src/main/twirl/gitbucket/core/helper/commitcomment.scala.html index 3cb91cd7b..cd242f2ec 100644 --- a/src/main/twirl/gitbucket/core/helper/commitcomment.scala.html +++ b/src/main/twirl/gitbucket/core/helper/commitcomment.scala.html @@ -13,15 +13,14 @@ @helpers.avatar(comment.commentedUserName, 20) @helpers.user(comment.commentedUserName, styleClass="username strong") - commented + commented on @if(comment.issueId.isDefined){ - on this Pull request - } else { - @if(comment.fileName.isDefined){ - on @comment.fileName.get - } - in @comment.commitId.substring(0, 7) + #@comment.issueId } + @comment.fileName.map { fileName => + @fileName in + } + @comment.commitId.substring(0, 7) @gitbucket.core.helper.html.datetimeago(comment.registeredDate)