summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--models/issue_comment.go12
-rw-r--r--models/migrations/migrations.go2
-rw-r--r--models/migrations/v158.go110
-rw-r--r--services/pull/review.go83
4 files changed, 183 insertions, 24 deletions
diff --git a/models/issue_comment.go b/models/issue_comment.go
index a7e9c049bf..7bcea40b93 100644
--- a/models/issue_comment.go
+++ b/models/issue_comment.go
@@ -712,6 +712,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
RefAction: opts.RefAction,
RefIsPull: opts.RefIsPull,
IsForcePush: opts.IsForcePush,
+ Invalidated: opts.Invalidated,
}
if _, err = e.Insert(comment); err != nil {
return nil, err
@@ -878,6 +879,7 @@ type CreateCommentOptions struct {
RefAction references.XRefAction
RefIsPull bool
IsForcePush bool
+ Invalidated bool
}
// CreateComment creates comment of issue or commit.
@@ -953,6 +955,8 @@ type FindCommentsOptions struct {
ReviewID int64
Since int64
Before int64
+ Line int64
+ TreePath string
Type CommentType
}
@@ -976,6 +980,12 @@ func (opts *FindCommentsOptions) toConds() builder.Cond {
if opts.Type != CommentTypeUnknown {
cond = cond.And(builder.Eq{"comment.type": opts.Type})
}
+ if opts.Line > 0 {
+ cond = cond.And(builder.Eq{"comment.line": opts.Line})
+ }
+ if len(opts.TreePath) > 0 {
+ cond = cond.And(builder.Eq{"comment.tree_path": opts.TreePath})
+ }
return cond
}
@@ -990,6 +1000,8 @@ func findComments(e Engine, opts FindCommentsOptions) ([]*Comment, error) {
sess = opts.setSessionPagination(sess)
}
+ // WARNING: If you change this order you will need to fix createCodeComment
+
return comments, sess.
Asc("comment.created_unix").
Asc("comment.id").
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index cbf8ae8732..4715f192c1 100644
--- a/models/migrations/migrations.go
+++ b/models/migrations/migrations.go
@@ -250,6 +250,8 @@ var migrations = []Migration{
NewMigration("fix publisher ID for tag releases", fixPublisherIDforTagReleases),
// v157 -> v158
NewMigration("ensure repo topics are up-to-date", fixRepoTopics),
+ // v158 -> v159
+ NewMigration("code comment replies should have the commitID of the review they are replying to", updateCodeCommentReplies),
}
// GetCurrentDBVersion returns the current db version
diff --git a/models/migrations/v158.go b/models/migrations/v158.go
new file mode 100644
index 0000000000..a6f2178f87
--- /dev/null
+++ b/models/migrations/v158.go
@@ -0,0 +1,110 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import (
+ "fmt"
+ "strconv"
+
+ "code.gitea.io/gitea/modules/log"
+ "code.gitea.io/gitea/modules/setting"
+
+ "xorm.io/xorm"
+)
+
+func updateCodeCommentReplies(x *xorm.Engine) error {
+ type Comment struct {
+ ID int64 `xorm:"pk autoincr"`
+ CommitSHA string `xorm:"VARCHAR(40)"`
+ Patch string `xorm:"TEXT patch"`
+ Invalidated bool
+
+ // Not extracted but used in the below query
+ Type int `xorm:"INDEX"`
+ Line int64 // - previous line / + proposed line
+ TreePath string
+ ReviewID int64 `xorm:"index"`
+ }
+
+ if err := x.Sync2(new(Comment)); err != nil {
+ return err
+ }
+
+ sqlSelect := `SELECT comment.id as id, first.commit_sha as commit_sha, first.patch as patch, first.invalidated as invalidated`
+ sqlTail := ` FROM comment INNER JOIN (
+ SELECT C.id, C.review_id, C.line, C.tree_path, C.patch, C.commit_sha, C.invalidated
+ FROM comment AS C
+ WHERE C.type = 21
+ AND C.created_unix =
+ (SELECT MIN(comment.created_unix)
+ FROM comment
+ WHERE comment.review_id = C.review_id
+ AND comment.type = 21
+ AND comment.line = C.line
+ AND comment.tree_path = C.tree_path)
+ ) AS first
+ ON comment.review_id = first.review_id
+ AND comment.tree_path = first.tree_path AND comment.line = first.line
+ WHERE comment.type = 21
+ AND comment.id != first.id
+ AND comment.commit_sha != first.commit_sha`
+
+ var sqlCmd string
+ var start = 0
+ var batchSize = 100
+ sess := x.NewSession()
+ defer sess.Close()
+ for {
+ if err := sess.Begin(); err != nil {
+ return err
+ }
+
+ if setting.Database.UseMSSQL {
+ if _, err := sess.Exec(sqlSelect + " INTO #temp_comments" + sqlTail); err != nil {
+ log.Error("unable to create temporary table")
+ return err
+ }
+ }
+
+ var comments = make([]*Comment, 0, batchSize)
+
+ switch {
+ case setting.Database.UseMySQL:
+ sqlCmd = sqlSelect + sqlTail + " LIMIT " + strconv.Itoa(batchSize) + ", " + strconv.Itoa(start)
+ case setting.Database.UsePostgreSQL:
+ fallthrough
+ case setting.Database.UseSQLite3:
+ sqlCmd = sqlSelect + sqlTail + " LIMIT " + strconv.Itoa(batchSize) + " OFFSET " + strconv.Itoa(start)
+ case setting.Database.UseMSSQL:
+ sqlCmd = "SELECT TOP " + strconv.Itoa(batchSize) + " * FROM #temp_comments WHERE " +
+ "(id NOT IN ( SELECT TOP " + strconv.Itoa(start) + " id FROM #temp_comments ORDER BY id )) ORDER BY id"
+ default:
+ return fmt.Errorf("Unsupported database type")
+ }
+
+ if err := sess.SQL(sqlCmd).Find(&comments); err != nil {
+ log.Error("failed to select: %v", err)
+ return err
+ }
+
+ for _, comment := range comments {
+ if _, err := sess.Table("comment").ID(comment.ID).Cols("commit_sha", "patch", "invalidated").Update(comment); err != nil {
+ log.Error("failed to update comment[%d]: %v %v", comment.ID, comment, err)
+ return err
+ }
+ }
+
+ start += len(comments)
+
+ if err := sess.Commit(); err != nil {
+ return err
+ }
+ if len(comments) < batchSize {
+ break
+ }
+ }
+
+ return nil
+}
diff --git a/services/pull/review.go b/services/pull/review.go
index 99afdd73c2..f0ee234a42 100644
--- a/services/pull/review.go
+++ b/services/pull/review.go
@@ -122,41 +122,76 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models
}
defer gitRepo.Close()
- // FIXME validate treePath
- // Get latest commit referencing the commented line
- // No need for get commit for base branch changes
+ invalidated := false
+ head := pr.GetGitRefName()
if line > 0 {
- commit, err := gitRepo.LineBlame(pr.GetGitRefName(), gitRepo.Path, treePath, uint(line))
- if err == nil {
- commitID = commit.ID.String()
- } else if !(strings.Contains(err.Error(), "exit status 128 - fatal: no such path") || notEnoughLines.MatchString(err.Error())) {
- return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
+ if reviewID != 0 {
+ first, err := models.FindComments(models.FindCommentsOptions{
+ ReviewID: reviewID,
+ Line: line,
+ TreePath: treePath,
+ Type: models.CommentTypeCode,
+ ListOptions: models.ListOptions{
+ PageSize: 1,
+ Page: 1,
+ },
+ })
+ if err == nil && len(first) > 0 {
+ commitID = first[0].CommitSHA
+ invalidated = first[0].Invalidated
+ patch = first[0].Patch
+ } else if err != nil && !models.IsErrCommentNotExist(err) {
+ return nil, fmt.Errorf("Find first comment for %d line %d path %s. Error: %v", reviewID, line, treePath, err)
+ } else {
+ review, err := models.GetReviewByID(reviewID)
+ if err == nil && len(review.CommitID) > 0 {
+ head = review.CommitID
+ } else if err != nil && !models.IsErrReviewNotExist(err) {
+ return nil, fmt.Errorf("GetReviewByID %d. Error: %v", reviewID, err)
+ }
+ }
+ }
+
+ if len(commitID) == 0 {
+ // FIXME validate treePath
+ // Get latest commit referencing the commented line
+ // No need for get commit for base branch changes
+ commit, err := gitRepo.LineBlame(head, gitRepo.Path, treePath, uint(line))
+ if err == nil {
+ commitID = commit.ID.String()
+ } else if !(strings.Contains(err.Error(), "exit status 128 - fatal: no such path") || notEnoughLines.MatchString(err.Error())) {
+ return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
+ }
}
}
// Only fetch diff if comment is review comment
- if reviewID != 0 {
- headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
- if err != nil {
- return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
+ if len(patch) == 0 && reviewID != 0 {
+ if len(commitID) == 0 {
+ commitID, err = gitRepo.GetRefCommitID(pr.GetGitRefName())
+ if err != nil {
+ return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
+ }
}
+
patchBuf := new(bytes.Buffer)
- if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, treePath, patchBuf); err != nil {
- return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath)
+ if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, commitID, git.RawDiffNormal, treePath, patchBuf); err != nil {
+ return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", gitRepo.Path, pr.MergeBase, commitID, treePath, err)
}
patch = git.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
}
return models.CreateComment(&models.CreateCommentOptions{
- Type: models.CommentTypeCode,
- Doer: doer,
- Repo: repo,
- Issue: issue,
- Content: content,
- LineNum: line,
- TreePath: treePath,
- CommitSHA: commitID,
- ReviewID: reviewID,
- Patch: patch,
+ Type: models.CommentTypeCode,
+ Doer: doer,
+ Repo: repo,
+ Issue: issue,
+ Content: content,
+ LineNum: line,
+ TreePath: treePath,
+ CommitSHA: commitID,
+ ReviewID: reviewID,
+ Patch: patch,
+ Invalidated: invalidated,
})
}