Fix bug on showing inline comments in Pull Request.

This commit is contained in:
Shintaro Murakami
2014-12-28 20:04:11 +09:00
parent 837b1e44a7
commit 6e2b67ec0b
14 changed files with 47 additions and 30 deletions

View File

@@ -9,7 +9,8 @@ CREATE TABLE COMMIT_COMMENT (
OLD_LINE_NUMBER INT, OLD_LINE_NUMBER INT,
NEW_LINE_NUMBER INT, NEW_LINE_NUMBER INT,
REGISTERED_DATE TIMESTAMP NOT NULL, REGISTERED_DATE TIMESTAMP NOT NULL,
UPDATED_DATE TIMESTAMP NOT NULL UPDATED_DATE TIMESTAMP NOT NULL,
IS_IN_PR BOOLEAN NOT NULL
); );
ALTER TABLE COMMIT_COMMENT ADD CONSTRAINT IDX_COMMIT_COMMENT_PK PRIMARY KEY (COMMENT_ID); ALTER TABLE COMMIT_COMMENT ADD CONSTRAINT IDX_COMMIT_COMMENT_PK PRIMARY KEY (COMMENT_ID);

View File

@@ -79,7 +79,7 @@ trait PullRequestsControllerBase extends ControllerBase {
pulls.html.pullreq( pulls.html.pullreq(
issue, pullreq, issue, pullreq,
(commits.flatten.map(commit => getCommitComments(owner, name, commit.id)).flatten.toList ::: getComments(owner, name, issueId)) (commits.flatten.map(commit => getCommitComments(owner, name, commit.id, true)).flatten.toList ::: getComments(owner, name, issueId))
.sortWith((a, b) => a.registeredDate before b.registeredDate), .sortWith((a, b) => a.registeredDate before b.registeredDate),
getIssueLabels(owner, name, issueId), getIssueLabels(owner, name, issueId),
(getCollaborators(owner, name) ::: (if(getAccountByUserName(owner).get.isGroupAccount) Nil else List(owner))).sorted, (getCollaborators(owner, name) ::: (if(getAccountByUserName(owner).get.isGroupAccount) Nil else List(owner))).sorted,
@@ -280,7 +280,7 @@ trait PullRequestsControllerBase extends ControllerBase {
case (Some(userName), Some(repositoryName)) => (userName, repositoryName) :: getForkedRepositories(userName, repositoryName) case (Some(userName), Some(repositoryName)) => (userName, repositoryName) :: getForkedRepositories(userName, repositoryName)
case _ => (forkedRepository.owner, forkedRepository.name) :: getForkedRepositories(forkedRepository.owner, forkedRepository.name) case _ => (forkedRepository.owner, forkedRepository.name) :: getForkedRepositories(forkedRepository.owner, forkedRepository.name)
}, },
commits.flatten.map(commit => getCommitComments(forkedRepository.owner, forkedRepository.name, commit.id)).flatten.toList, commits.flatten.map(commit => getCommitComments(forkedRepository.owner, forkedRepository.name, commit.id, false)).flatten.toList,
originBranch, originBranch,
forkedBranch, forkedBranch,
oldId.getName, oldId.getName,

View File

@@ -56,7 +56,8 @@ trait RepositoryViewerControllerBase extends ControllerBase {
fileName: Option[String], fileName: Option[String],
oldLineNumber: Option[Int], oldLineNumber: Option[Int],
newLineNumber: Option[Int], newLineNumber: Option[Int],
content: String content: String,
isInPR: Boolean
) )
val editorForm = mapping( val editorForm = mapping(
@@ -81,7 +82,8 @@ trait RepositoryViewerControllerBase extends ControllerBase {
"fileName" -> trim(label("Filename", optional(text()))), "fileName" -> trim(label("Filename", optional(text()))),
"oldLineNumber" -> trim(label("Old line number", optional(number()))), "oldLineNumber" -> trim(label("Old line number", optional(number()))),
"newLineNumber" -> trim(label("New line number", optional(number()))), "newLineNumber" -> trim(label("New line number", optional(number()))),
"content" -> trim(label("Content", text(required))) "content" -> trim(label("Content", text(required))),
"isInPR" -> trim(label("Is in PR", boolean()))
)(CommentForm.apply) )(CommentForm.apply)
/** /**
@@ -235,7 +237,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
repo.html.commit(id, new JGitUtil.CommitInfo(revCommit), repo.html.commit(id, new JGitUtil.CommitInfo(revCommit),
JGitUtil.getBranchesOfCommit(git, revCommit.getName), JGitUtil.getBranchesOfCommit(git, revCommit.getName),
JGitUtil.getTagsOfCommit(git, revCommit.getName), JGitUtil.getTagsOfCommit(git, revCommit.getName),
getCommitComments(repository.owner, repository.name, id), getCommitComments(repository.owner, repository.name, id, false),
repository, diffs, oldCommitId, hasWritePermission(repository.owner, repository.name, context.loginAccount)) repository, diffs, oldCommitId, hasWritePermission(repository.owner, repository.name, context.loginAccount))
} }
} }
@@ -245,7 +247,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
post("/:owner/:repository/commit/:id/comment/new", commentForm)(readableUsersOnly { (form, repository) => post("/:owner/:repository/commit/:id/comment/new", commentForm)(readableUsersOnly { (form, repository) =>
val id = params("id") val id = params("id")
createCommitComment(repository.owner, repository.name, id, context.loginAccount.get.userName, form.content, createCommitComment(repository.owner, repository.name, id, context.loginAccount.get.userName, form.content,
form.fileName, form.oldLineNumber, form.newLineNumber) form.fileName, form.oldLineNumber, form.newLineNumber, form.isInPR)
recordCommentCommitActivity(repository.owner, repository.name, context.loginAccount.get.userName, id, form.content) recordCommentCommitActivity(repository.owner, repository.name, context.loginAccount.get.userName, id, form.content)
redirect(s"/${repository.owner}/${repository.name}/commit/${id}") redirect(s"/${repository.owner}/${repository.name}/commit/${id}")
}) })
@@ -255,9 +257,10 @@ trait RepositoryViewerControllerBase extends ControllerBase {
val fileName = params.get("fileName") val fileName = params.get("fileName")
val oldLineNumber = params.get("oldLineNumber") flatMap {b => Some(b.toInt)} val oldLineNumber = params.get("oldLineNumber") flatMap {b => Some(b.toInt)}
val newLineNumber = params.get("newLineNumber") flatMap {b => Some(b.toInt)} val newLineNumber = params.get("newLineNumber") flatMap {b => Some(b.toInt)}
val isInPR = params.get("isInPR")
repo.html.commentform( repo.html.commentform(
commitId = id, commitId = id,
fileName, oldLineNumber, newLineNumber, fileName, oldLineNumber, newLineNumber, isInPR.map(_.toBoolean).getOrElse(false),
hasWritePermission = hasWritePermission(repository.owner, repository.name, context.loginAccount), hasWritePermission = hasWritePermission(repository.owner, repository.name, context.loginAccount),
repository = repository repository = repository
) )
@@ -266,7 +269,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
ajaxPost("/:owner/:repository/commit/:id/comment/_data/new", commentForm)(readableUsersOnly { (form, repository) => ajaxPost("/:owner/:repository/commit/:id/comment/_data/new", commentForm)(readableUsersOnly { (form, repository) =>
val id = params("id") val id = params("id")
val commentId = createCommitComment(repository.owner, repository.name, id, context.loginAccount.get.userName, val commentId = createCommitComment(repository.owner, repository.name, id, context.loginAccount.get.userName,
form.content, form.fileName, form.oldLineNumber, form.newLineNumber) form.content, form.fileName, form.oldLineNumber, form.newLineNumber, form.isInPR)
recordCommentCommitActivity(repository.owner, repository.name, context.loginAccount.get.userName, id, form.content) recordCommentCommitActivity(repository.owner, repository.name, context.loginAccount.get.userName, id, form.content)
helper.html.commitcomment(getCommitComment(repository.owner, repository.name, commentId.toString).get, helper.html.commitcomment(getCommitComment(repository.owner, repository.name, commentId.toString).get,
hasWritePermission(repository.owner, repository.name, context.loginAccount), repository) hasWritePermission(repository.owner, repository.name, context.loginAccount), repository)

View File

@@ -55,7 +55,8 @@ trait CommitCommentComponent extends TemplateComponent { self: Profile =>
val newLine = column[Option[Int]]("NEW_LINE_NUMBER") val newLine = column[Option[Int]]("NEW_LINE_NUMBER")
val registeredDate = column[java.util.Date]("REGISTERED_DATE") val registeredDate = column[java.util.Date]("REGISTERED_DATE")
val updatedDate = column[java.util.Date]("UPDATED_DATE") val updatedDate = column[java.util.Date]("UPDATED_DATE")
def * = (userName, repositoryName, commitId, commentId, commentedUserName, content, fileName, oldLine, newLine, registeredDate, updatedDate) <> (CommitComment.tupled, CommitComment.unapply) val isInPR = column[Boolean]("IS_IN_PR")
def * = (userName, repositoryName, commitId, commentId, commentedUserName, content, fileName, oldLine, newLine, registeredDate, updatedDate, isInPR) <> (CommitComment.tupled, CommitComment.unapply)
def byPrimaryKey(commentId: Int) = this.commentId === commentId.bind def byPrimaryKey(commentId: Int) = this.commentId === commentId.bind
} }
@@ -72,5 +73,6 @@ case class CommitComment(
oldLine: Option[Int], oldLine: Option[Int],
newLine: Option[Int], newLine: Option[Int],
registeredDate: java.util.Date, registeredDate: java.util.Date,
updatedDate: java.util.Date updatedDate: java.util.Date,
isInPR: Boolean
) extends Comment ) extends Comment

View File

@@ -12,8 +12,10 @@ import util.StringUtil._
trait CommitsService { trait CommitsService {
def getCommitComments(owner: String, repository: String, commitId: String)(implicit s: Session) = def getCommitComments(owner: String, repository: String, commitId: String, isInPR: Boolean)(implicit s: Session) =
CommitComments filter (_.byCommit(owner, repository, commitId)) list CommitComments filter {
t => t.byCommit(owner, repository, commitId) && (t.isInPR === isInPR || isInPR)
} list
def getCommitComment(owner: String, repository: String, commentId: String)(implicit s: Session) = def getCommitComment(owner: String, repository: String, commentId: String)(implicit s: Session) =
if (commentId forall (_.isDigit)) if (commentId forall (_.isDigit))
@@ -24,7 +26,7 @@ trait CommitsService {
None None
def createCommitComment(owner: String, repository: String, commitId: String, loginUser: String, def createCommitComment(owner: String, repository: String, commitId: String, loginUser: String,
content: String, fileName: Option[String], oldLine: Option[Int], newLine: Option[Int])(implicit s: Session): Int = content: String, fileName: Option[String], oldLine: Option[Int], newLine: Option[Int], isInPR: Boolean)(implicit s: Session): Int =
CommitComments.autoInc insert CommitComment( CommitComments.autoInc insert CommitComment(
userName = owner, userName = owner,
repositoryName = repository, repositoryName = repository,
@@ -35,7 +37,8 @@ trait CommitsService {
oldLine = oldLine, oldLine = oldLine,
newLine = newLine, newLine = newLine,
registeredDate = currentDate, registeredDate = currentDate,
updatedDate = currentDate) updatedDate = currentDate,
isInPR = isInPR)
def updateCommitComment(commentId: Int, content: String)(implicit s: Session) = def updateCommitComment(commentId: Int, content: String)(implicit s: Session) =
CommitComments CommitComments

View File

@@ -1,19 +1,24 @@
@(comment: model.CommitComment, @(comment: model.CommitComment,
hasWritePermission: Boolean, hasWritePermission: Boolean,
repository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context) repository: service.RepositoryService.RepositoryInfo,
latestCommitId: Option[String] = None)(implicit context: app.Context)
@import context._ @import context._
@import view.helpers._ @import view.helpers._
<div class="@if(comment.fileName.isDefined){inline-comment}" @if(comment.fileName.isDefined){filename=@comment.fileName.get} @if(comment.newLine.isDefined){newline=@comment.newLine.get} @if(comment.oldLine.isDefined){oldline=@comment.oldLine.get}> <div class="@if(comment.fileName.isDefined && (!latestCommitId.isDefined || latestCommitId.get == comment.commitId)){inline-comment}" @if(comment.fileName.isDefined){filename=@comment.fileName.get} @if(comment.newLine.isDefined){newline=@comment.newLine.get} @if(comment.oldLine.isDefined){oldline=@comment.oldLine.get}>
<div class="issue-avatar-image">@avatar(comment.commentedUserName, 48)</div> <div class="issue-avatar-image">@avatar(comment.commentedUserName, 48)</div>
<div class="box commit-comment-box commit-comment-@comment.commentId"> <div class="box commit-comment-box commit-comment-@comment.commentId">
<div class="box-header-small"> <div class="box-header-small">
@user(comment.commentedUserName, styleClass="username strong") @user(comment.commentedUserName, styleClass="username strong")
<span class="muted"> <span class="muted">
commented commented
@if(comment.isInPR){
on this Pull Request
}else{
@if(comment.fileName.isDefined){ @if(comment.fileName.isDefined){
on @comment.fileName.get on @comment.fileName.get
} }
in <a href="@path/@repository.owner/@repository.name/commit/@comment.commitId">@comment.commitId.substring(0, 7)</a> in <a href="@path/@repository.owner/@repository.name/commit/@comment.commitId">@comment.commitId.substring(0, 7)</a>
}
@helper.html.datetimeago(comment.registeredDate) @helper.html.datetimeago(comment.registeredDate)
</span> </span>
<span class="pull-right"> <span class="pull-right">

View File

@@ -3,6 +3,7 @@
newCommitId: Option[String], newCommitId: Option[String],
oldCommitId: Option[String], oldCommitId: Option[String],
showIndex: Boolean, showIndex: Boolean,
isInPR: Boolean,
hasWritePermission: Boolean, hasWritePermission: Boolean,
showLineNotes: Boolean)(implicit context: app.Context) showLineNotes: Boolean)(implicit context: app.Context)
@import context._ @import context._
@@ -156,7 +157,7 @@ $(function(){
fileName = $(this).closest('.table-bordered').attr('fileName'), fileName = $(this).closest('.table-bordered').attr('fileName'),
oldLineNumber = $(this).closest('.newline').prev('.oldline').text(), oldLineNumber = $(this).closest('.newline').prev('.oldline').text(),
newLineNumber = $(this).closest('.newline').clone().children().remove().end().text(), newLineNumber = $(this).closest('.newline').clone().children().remove().end().text(),
url = '@url(repository)/commit/' + commitId + '/comment/_form?fileName=' + fileName; url = '@url(repository)/commit/' + commitId + '/comment/_form?fileName=' + fileName + '&isInPR=@isInPR';
if (!isNaN(oldLineNumber) && oldLineNumber != null && oldLineNumber !== '') { if (!isNaN(oldLineNumber) && oldLineNumber != null && oldLineNumber !== '') {
url += ('&oldLineNumber=' + oldLineNumber) url += ('&oldLineNumber=' + oldLineNumber)
} }

View File

@@ -103,7 +103,7 @@
} }
} }
case comment: model.CommitComment => { case comment: model.CommitComment => {
@helper.html.commitcomment(comment, hasWritePermission, repository) @helper.html.commitcomment(comment, hasWritePermission, repository, pullreq.map(_.commitIdTo))
} }
} }
<script> <script>

View File

@@ -21,7 +21,7 @@
@comments.get.flatMap @{ @comments.get.flatMap @{
case comment: model.CommitComment => Some(comment) case comment: model.CommitComment => Some(comment)
case other => None case other => None
}.count(_.commitId == commit.id) }.count(t => t.commitId == commit.id && !t.isInPR)
}</span> }</span>
</td> </td>
<td style="width: 10%; text-align: right;"> <td style="width: 10%; text-align: right;">

View File

@@ -83,7 +83,7 @@
</table> </table>
} else { } else {
@pulls.html.commits(commits, Some(comments), repository) @pulls.html.commits(commits, Some(comments), repository)
@helper.html.diff(diffs, repository, Some(commitId), Some(sourceId), true, hasWritePermission, false) @helper.html.diff(diffs, repository, Some(commitId), Some(sourceId), true, false, hasWritePermission, false)
<p>Showing you all comments on commits in this comparison.</p> <p>Showing you all comments on commits in this comparison.</p>
@issues.html.commentlist(None, comments, hasWritePermission, repository, None) @issues.html.commentlist(None, comments, hasWritePermission, repository, None)
} }

View File

@@ -77,7 +77,7 @@
@pulls.html.commits(dayByDayCommits, Some(comments), repository) @pulls.html.commits(dayByDayCommits, Some(comments), repository)
</div> </div>
<div class="tab-pane" id="files"> <div class="tab-pane" id="files">
@helper.html.diff(diffs, repository, Some(commits.head.id), Some(commits.last.id), true, hasWritePermission, true) @helper.html.diff(diffs, repository, Some(commits.head.id), Some(commits.last.id), true, true, hasWritePermission, true)
</div> </div>
</div> </div>
} }

View File

@@ -2,6 +2,7 @@
fileName: Option[String] = None, fileName: Option[String] = None,
oldLineNumber: Option[Int] = None, oldLineNumber: Option[Int] = None,
newLineNumber: Option[Int] = None, newLineNumber: Option[Int] = None,
isInPR: Boolean,
hasWritePermission: Boolean, hasWritePermission: Boolean,
repository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context) repository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context)
@import context._ @import context._
@@ -28,6 +29,7 @@
<input type="submit" class="btn btn-success" formaction="@url(repository)/commit/@commitId/comment/new" value="Comment on this commit"/> <input type="submit" class="btn btn-success" formaction="@url(repository)/commit/@commitId/comment/new" value="Comment on this commit"/>
</div> </div>
} }
<input type="hidden" name="isInPR" value="@isInPR">
@if(fileName.isDefined){<input type="hidden" name="fileName" value="@fileName.get">} @if(fileName.isDefined){<input type="hidden" name="fileName" value="@fileName.get">}
@if(oldLineNumber.isDefined){<input type="hidden" name="oldLineNumber" value="@oldLineNumber.get">} @if(oldLineNumber.isDefined){<input type="hidden" name="oldLineNumber" value="@oldLineNumber.get">}
@if(newLineNumber.isDefined){<input type="hidden" name="newLineNumber" value="@newLineNumber.get">} @if(newLineNumber.isDefined){<input type="hidden" name="newLineNumber" value="@newLineNumber.get">}

View File

@@ -83,14 +83,14 @@
</td> </td>
</tr> </tr>
</table> </table>
@helper.html.diff(diffs, repository, Some(commit.id), oldCommitId, true, hasWritePermission, true) @helper.html.diff(diffs, repository, Some(commit.id), oldCommitId, true, false, hasWritePermission, true)
<label class="checkbox"> <label class="checkbox">
<input type="checkbox" id="show-notes"> Show line notes below <input type="checkbox" id="show-notes"> Show line notes below
</label> </label>
<div id="comment-list"> <div id="comment-list">
@issues.html.commentlist(None, comments, hasWritePermission, repository, None) @issues.html.commentlist(None, comments, hasWritePermission, repository, None)
</div> </div>
@commentform(commitId = commitId, hasWritePermission = hasWritePermission, repository = repository) @commentform(commitId = commitId, isInPR = false, hasWritePermission = hasWritePermission, repository = repository)
} }
} }
<script> <script>

View File

@@ -26,7 +26,7 @@
</div> </div>
</li> </li>
</ul> </ul>
@helper.html.diff(diffs, repository, None, None, false, false, false) @helper.html.diff(diffs, repository, None, None, false, false, false, false)
@if(hasWritePermission){ @if(hasWritePermission){
<div> <div>
@if(pageName.isDefined){ @if(pageName.isDefined){