github/gitea
1
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2025-08-05 18:19:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

Allow code review comments display cross commits even if the head branch has a force-push

Browse Source
This commit is contained in:
Lunny Xiao 2025-07-13 16:07:46 -07:00
parent 3531e9dbfd
commit 345ae04837
24 changed files with 562 additions and 384 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
models/issues/comment.go
View File

@ -1007,6 +1007,7 @@ type FindCommentsOptions struct {
RepoID int64
IssueID int64
ReviewID int64
CommitSHA string
Since int64
Before int64
Line int64
@ -1052,6 +1053,9 @@ func (opts FindCommentsOptions) ToConds() builder.Cond {
if opts.IsPull.Has() {
cond = cond.And(builder.Eq{"issue.is_pull": opts.IsPull.Value()})
}
if opts.CommitSHA != "" {
cond = cond.And(builder.Eq{"comment.commit_sha": opts.CommitSHA})
}
return cond
}

53
models/issues/comment_code.go
View File

@ -9,6 +9,7 @@ import (
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/renderhelper"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/markup/markdown"
@ -16,39 +17,44 @@ import (
)
// CodeComments represents comments on code by using this structure: FILENAME -> LINE (+ == proposed; - == previous) -> COMMENTS
type CodeComments map[string]map[int64][]*Comment
type CodeComments map[string][]*Comment
// FetchCodeComments will return a 2d-map: ["Path"]["Line"] = Comments at line
func FetchCodeComments(ctx context.Context, issue *Issue, currentUser *user_model.User, showOutdatedComments bool) (CodeComments, error) {
return fetchCodeCommentsByReview(ctx, issue, currentUser, nil, showOutdatedComments)
func (cc CodeComments) AllComments() []*Comment {
var allComments []*Comment
for _, comments := range cc {
allComments = append(allComments, comments...)
}
return allComments
}
func fetchCodeCommentsByReview(ctx context.Context, issue *Issue, currentUser *user_model.User, review *Review, showOutdatedComments bool) (CodeComments, error) {
pathToLineToComment := make(CodeComments)
// FetchCodeComments will return a 2d-map: ["Path"]["Line"] = Comments at line
func FetchCodeComments(ctx context.Context, repo *repo_model.Repository, issueID int64, currentUser *user_model.User, showOutdatedComments bool) (CodeComments, error) {
return fetchCodeCommentsByReview(ctx, repo, issueID, currentUser, nil, showOutdatedComments)
}
func fetchCodeCommentsByReview(ctx context.Context, repo *repo_model.Repository, issueID int64, currentUser *user_model.User, review *Review, showOutdatedComments bool) (CodeComments, error) {
codeCommentsPathMap := make(CodeComments)
if review == nil {
review = &Review{ID: 0}
}
opts := FindCommentsOptions{
Type: CommentTypeCode,
IssueID: issue.ID,
IssueID: issueID,
ReviewID: review.ID,
}
comments, err := findCodeComments(ctx, opts, issue, currentUser, review, showOutdatedComments)
comments, err := FindCodeComments(ctx, opts, repo, currentUser, review, showOutdatedComments)
if err != nil {
return nil, err
}
for _, comment := range comments {
if pathToLineToComment[comment.TreePath] == nil {
pathToLineToComment[comment.TreePath] = make(map[int64][]*Comment)
}
pathToLineToComment[comment.TreePath][comment.Line] = append(pathToLineToComment[comment.TreePath][comment.Line], comment)
codeCommentsPathMap[comment.TreePath] = append(codeCommentsPathMap[comment.TreePath], comment)
}
return pathToLineToComment, nil
return codeCommentsPathMap, nil
}
func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issue, currentUser *user_model.User, review *Review, showOutdatedComments bool) ([]*Comment, error) {
func FindCodeComments(ctx context.Context, opts FindCommentsOptions, repo *repo_model.Repository, currentUser *user_model.User, review *Review, showOutdatedComments bool) ([]*Comment, error) {
var comments CommentList
if review == nil {
review = &Review{ID: 0}
@ -67,10 +73,6 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu
return nil, err
}
if err := issue.LoadRepo(ctx); err != nil {
return nil, err
}
if err := comments.LoadPosters(ctx); err != nil {
return nil, err
}
@ -110,12 +112,12 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu
return nil, err
}
if err := comment.LoadReactions(ctx, issue.Repo); err != nil {
if err := comment.LoadReactions(ctx, repo); err != nil {
return nil, err
}
var err error
rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo, renderhelper.RepoCommentOptions{
rctx := renderhelper.NewRenderContextRepoComment(ctx, repo, renderhelper.RepoCommentOptions{
FootnoteContextID: strconv.FormatInt(comment.ID, 10),
})
if comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content); err != nil {
@ -124,14 +126,3 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu
}
return comments[:n], nil
}
// FetchCodeCommentsByLine fetches the code comments for a given treePath and line number
func FetchCodeCommentsByLine(ctx context.Context, issue *Issue, currentUser *user_model.User, treePath string, line int64, showOutdatedComments bool) (CommentList, error) {
opts := FindCommentsOptions{
Type: CommentTypeCode,
IssueID: issue.ID,
TreePath: treePath,
Line: line,
}
return findCodeComments(ctx, opts, issue, currentUser, nil, showOutdatedComments)
}

6
models/issues/comment_test.go
View File

@ -68,15 +68,15 @@ func TestFetchCodeComments(t *testing.T) {
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
res, err := issues_model.FetchCodeComments(db.DefaultContext, issue, user, false)
res, err := issues_model.FetchCodeComments(db.DefaultContext, issue.Repo, issue.ID, user, false)
assert.NoError(t, err)
assert.Contains(t, res, "README.md")
assert.Contains(t, res["README.md"], int64(4))
assert.Len(t, res["README.md"][4], 1)
assert.Equal(t, int64(4), res["README.md"][4][0].ID)
assert.Equal(t, int64(4), res["README.md"][0].ID)
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
res, err = issues_model.FetchCodeComments(db.DefaultContext, issue, user2, false)
res, err = issues_model.FetchCodeComments(db.DefaultContext, issue.Repo, issue.ID, user2, false)
assert.NoError(t, err)
assert.Len(t, res, 1)
}

2
models/issues/review.go
View File

@ -159,7 +159,7 @@ func (r *Review) LoadCodeComments(ctx context.Context) (err error) {
if err = r.LoadIssue(ctx); err != nil {
return err
}
r.CodeComments, err = fetchCodeCommentsByReview(ctx, r.Issue, nil, r, false)
r.CodeComments, err = fetchCodeCommentsByReview(ctx, r.Issue.Repo, r.Issue.ID, nil, r, false)
return err
}

2
models/issues/review_test.go
View File

@ -48,7 +48,7 @@ func TestReview_LoadCodeComments(t *testing.T) {
assert.NoError(t, review.LoadAttributes(db.DefaultContext))
assert.NoError(t, review.LoadCodeComments(db.DefaultContext))
assert.Len(t, review.CodeComments, 1)
assert.Equal(t, int64(4), review.CodeComments["README.md"][int64(4)][0].Line)
assert.Equal(t, int64(4), review.CodeComments["README.md"][0].Line)
}
func TestReviewType_Icon(t *testing.T) {

57
modules/git/diff.go
View File

@ -15,6 +15,7 @@ import (
"strings"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/util"
)
// RawDiffType type of a raw diff.
@ -107,12 +108,16 @@ func ParseDiffHunkString(diffHunk string) (leftLine, leftHunk, rightLine, rightH
leftLine, _ = strconv.Atoi(leftRange[0][1:])
if len(leftRange) > 1 {
leftHunk, _ = strconv.Atoi(leftRange[1])
} else {
leftHunk = util.Iif(leftLine > 0, leftLine, -leftLine)
}
if len(ranges) > 1 {
rightRange := strings.Split(ranges[1], ",")
rightLine, _ = strconv.Atoi(rightRange[0])
if len(rightRange) > 1 {
rightHunk, _ = strconv.Atoi(rightRange[1])
} else {
rightHunk = rightLine
}
} else {
log.Debug("Parse line number failed: %v", diffHunk)
@ -342,3 +347,55 @@ func GetAffectedFiles(repo *Repository, branchName, oldCommitID, newCommitID str
return affectedFiles, err
}
type HunkInfo struct {
LeftLine int64 // Line number in the old file
LeftHunk int64 // Number of lines in the old file
RightLine int64 // Line number in the new file
RightHunk int64 // Number of lines in the new file
}
// GetAffectedHunksForTwoCommitsSpecialFile returns the affected hunks between two commits for a special file
// git diff --unified=0 abc123 def456 -- src/main.go
func GetAffectedHunksForTwoCommitsSpecialFile(ctx context.Context, repoPath, oldCommitID, newCommitID, filePath string) ([]*HunkInfo, error) {
reader, writer := io.Pipe()
defer func() {
_ = reader.Close()
_ = writer.Close()
}()
go func() {
if err := NewCommand("diff", "--unified=0", "--no-color").
AddDynamicArguments(oldCommitID, newCommitID).
AddDashesAndList(filePath).
Run(ctx, &RunOpts{
Dir: repoPath,
Stdout: writer,
}); err != nil {
_ = writer.CloseWithError(fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s, %s]: %w", repoPath, oldCommitID, newCommitID, filePath, err))
return
}
_ = writer.Close()
}()
scanner := bufio.NewScanner(reader)
hunks := make([]*HunkInfo, 0, 32)
for scanner.Scan() {
lof := scanner.Text()
if !strings.HasPrefix(lof, "@@") {
continue
}
// Parse the hunk header
leftLine, leftHunk, rightLine, rightHunk := ParseDiffHunkString(lof)
hunks = append([]*HunkInfo{}, &HunkInfo{
LeftLine: int64(leftLine),
LeftHunk: int64(leftHunk),
RightLine: int64(rightLine),
RightHunk: int64(rightHunk),
})
}
if scanner.Err() != nil {
return nil, fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s, %s]: %w", repoPath, oldCommitID, newCommitID, filePath, scanner.Err())
}
return hunks, nil
}

6
modules/git/diff_test.go
View File

@ -181,4 +181,10 @@ func TestParseDiffHunkString(t *testing.T) {
assert.Equal(t, 3, leftHunk)
assert.Equal(t, 19, rightLine)
assert.Equal(t, 5, rightHunk)
leftLine, leftHunk, rightLine, rightHunk = ParseDiffHunkString("@@ -1 +0,0 @@")
assert.Equal(t, 1, leftLine)
assert.Equal(t, 1, leftHunk)
assert.Equal(t, 0, rightLine)
assert.Equal(t, 0, rightHunk)
}

13
routers/api/v1/repo/pull_review.go
View File

@ -13,7 +13,6 @@ import (
"code.gitea.io/gitea/models/organization"
access_model "code.gitea.io/gitea/models/perm/access"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/gitrepo"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/routers/api/v1/utils"
@ -329,14 +328,7 @@ func CreatePullReview(ctx *context.APIContext) {
// if CommitID is empty, set it as lastCommitID
if opts.CommitID == "" {
gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.Issue.Repo)
if err != nil {
ctx.APIErrorInternal(err)
return
}
defer closer.Close()
headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitHeadRefName())
headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(pr.GetGitHeadRefName())
if err != nil {
ctx.APIErrorInternal(err)
return
@ -357,11 +349,12 @@ func CreatePullReview(ctx *context.APIContext) {
ctx.Repo.GitRepo,
pr.Issue,
line,
pr.MergeBase,
opts.CommitID,
c.Body,
c.Path,
true, // pending review
0, // no reply
opts.CommitID,
nil,
); err != nil {
ctx.APIErrorInternal(err)

35
routers/web/repo/issue_view.go
View File

@ -730,28 +730,23 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue
}
comment.Review.Reviewer = user_model.NewGhostUser()
}
if err = comment.Review.LoadCodeComments(ctx); err != nil {
ctx.ServerError("Review.LoadCodeComments", err)
return
}
for _, codeComments := range comment.Review.CodeComments {
for _, lineComments := range codeComments {
for _, c := range lineComments {
// Check tag.
role, ok = marked[c.PosterID]
if ok {
c.ShowRole = role
continue
}
c.ShowRole, err = roleDescriptor(ctx, issue.Repo, c.Poster, permCache, issue, c.HasOriginalAuthor())
if err != nil {
ctx.ServerError("roleDescriptor", err)
return
}
marked[c.PosterID] = c.ShowRole
participants = addParticipant(c.Poster, participants)
for _, codeComments := range comment.Review.CodeComments {
for _, c := range codeComments {
// Check tag.
role, ok = marked[c.PosterID]
if ok {
c.ShowRole = role
continue
}
c.ShowRole, err = roleDescriptor(ctx, issue.Repo, c.Poster, permCache, issue, c.HasOriginalAuthor())
if err != nil {
ctx.ServerError("roleDescriptor", err)
return
}
marked[c.PosterID] = c.ShowRole
participants = addParticipant(c.Poster, participants)
}
}
if err = comment.LoadResolveDoer(ctx); err != nil {

142
routers/web/repo/pull.go
View File

@ -643,8 +643,17 @@ func ViewPullCommits(ctx *context.Context) {
ctx.HTML(http.StatusOK, tplPullCommits)
}
func indexCommit(commits []*git.Commit, commitID string) *git.Commit {
for i := range commits {
if commits[i].ID.String() == commitID {
return commits[i]
}
}
return nil
}
// ViewPullFiles render pull request changed files list page
func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommit string, willShowSpecifiedCommitRange, willShowSpecifiedCommit bool) {
func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
ctx.Data["PageIsPullList"] = true
ctx.Data["PageIsPullFiles"] = true
@ -653,12 +662,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
return
}
pull := issue.PullRequest
var (
startCommitID string
endCommitID string
gitRepo = ctx.Repo.GitRepo
)
gitRepo := ctx.Repo.GitRepo
prInfo := preparePullViewPullInfo(ctx, issue)
if ctx.Written() {
@ -668,77 +672,65 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
return
}
// Validate the given commit sha to show (if any passed)
if willShowSpecifiedCommit || willShowSpecifiedCommitRange {
foundStartCommit := len(specifiedStartCommit) == 0
foundEndCommit := len(specifiedEndCommit) == 0
if !(foundStartCommit && foundEndCommit) {
for _, commit := range prInfo.Commits {
if commit.ID.String() == specifiedStartCommit {
foundStartCommit = true
}
if commit.ID.String() == specifiedEndCommit {
foundEndCommit = true
}
if foundStartCommit && foundEndCommit {
break
}
}
}
if !(foundStartCommit && foundEndCommit) {
ctx.NotFound(nil)
return
}
}
if ctx.Written() {
return
}
headCommitID, err := gitRepo.GetRefCommitID(pull.GetGitHeadRefName())
if err != nil {
ctx.ServerError("GetRefCommitID", err)
return
}
ctx.Data["IsShowingOnlySingleCommit"] = willShowSpecifiedCommit
ctx.Data["IsShowingOnlySingleCommit"] = beforeCommitID != "" && beforeCommitID == afterCommitID
isShowAllCommits := (beforeCommitID == "" || beforeCommitID == prInfo.MergeBase) && (afterCommitID == "" || afterCommitID == headCommitID)
ctx.Data["IsShowingAllCommits"] = isShowAllCommits
if willShowSpecifiedCommit || willShowSpecifiedCommitRange {
if len(specifiedEndCommit) > 0 {
endCommitID = specifiedEndCommit
} else {
endCommitID = headCommitID
if beforeCommitID == "" {
beforeCommitID = prInfo.MergeBase
}
if afterCommitID == "" {
afterCommitID = headCommitID
}
var beforeCommit, afterCommit *git.Commit
if beforeCommitID != prInfo.MergeBase {
beforeCommit = indexCommit(prInfo.Commits, beforeCommitID)
if beforeCommit == nil {
ctx.NotFound(errors.New("before commit not found in PR commits"))
return
}
if len(specifiedStartCommit) > 0 {
startCommitID = specifiedStartCommit
} else {
startCommitID = prInfo.MergeBase
beforeCommit, err = beforeCommit.Parent(0)
if err != nil {
ctx.ServerError("GetParentCommit", err)
return
}
ctx.Data["IsShowingAllCommits"] = false
} else {
endCommitID = headCommitID
startCommitID = prInfo.MergeBase
ctx.Data["IsShowingAllCommits"] = true
beforeCommitID = beforeCommit.ID.String()
} else { // mergebase commit is not in the list of the pull request commits
beforeCommit, err = gitRepo.GetCommit(beforeCommitID)
if err != nil {
ctx.ServerError("GetCommit", err)
return
}
}
afterCommit = indexCommit(prInfo.Commits, afterCommitID)
if afterCommit == nil {
ctx.NotFound(errors.New("after commit not found in PR commits"))
return
}
ctx.Data["Username"] = ctx.Repo.Owner.Name
ctx.Data["Reponame"] = ctx.Repo.Repository.Name
ctx.Data["AfterCommitID"] = endCommitID
ctx.Data["BeforeCommitID"] = startCommitID
fileOnly := ctx.FormBool("file-only")
ctx.Data["AfterCommitID"] = afterCommitID
ctx.Data["BeforeCommitID"] = beforeCommitID
maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles
files := ctx.FormStrings("files")
fileOnly := ctx.FormBool("file-only")
if fileOnly && (len(files) == 2 || len(files) == 1) {
maxLines, maxFiles = -1, -1
}
diffOptions := &gitdiff.DiffOptions{
AfterCommitID: endCommitID,
BeforeCommitID: beforeCommitID,
AfterCommitID: afterCommitID,
SkipTo: ctx.FormString("skip-to"),
MaxLines: maxLines,
MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters,
@ -746,10 +738,6 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
}
if !willShowSpecifiedCommit {
diffOptions.BeforeCommitID = startCommitID
}
diff, err := gitdiff.GetDiffForRender(ctx, ctx.Repo.RepoLink, gitRepo, diffOptions, files...)
if err != nil {
ctx.ServerError("GetDiff", err)
@ -761,7 +749,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
// as the viewed information is designed to be loaded only on latest PR
// diff and if you're signed in.
var reviewState *pull_model.ReviewState
if ctx.IsSigned && !willShowSpecifiedCommit && !willShowSpecifiedCommitRange {
if ctx.IsSigned && isShowAllCommits {
reviewState, err = gitdiff.SyncUserSpecificDiff(ctx, ctx.Doer.ID, pull, gitRepo, diff, diffOptions)
if err != nil {
ctx.ServerError("SyncUserSpecificDiff", err)
@ -769,7 +757,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
}
}
diffShortStat, err := gitdiff.GetDiffShortStat(ctx.Repo.GitRepo, startCommitID, endCommitID)
diffShortStat, err := gitdiff.GetDiffShortStat(ctx.Repo.GitRepo, beforeCommitID, afterCommitID)
if err != nil {
ctx.ServerError("GetDiffShortStat", err)
return
@ -781,7 +769,8 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
"numberOfViewedFiles": diff.NumViewedFiles,
}
if err = diff.LoadComments(ctx, issue, ctx.Doer, ctx.Data["ShowOutdatedComments"].(bool)); err != nil {
if err = pull_service.LoadCodeComments(ctx, ctx.Repo.GitRepo, ctx.Repo.Repository,
diff, issue.ID, ctx.Doer, beforeCommit, afterCommit, ctx.Data["ShowOutdatedComments"].(bool)); err != nil {
ctx.ServerError("LoadComments", err)
return
}
@ -816,7 +805,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
if !fileOnly {
// note: use mergeBase is set to false because we already have the merge base from the pull request info
diffTree, err := gitdiff.GetDiffTree(ctx, gitRepo, false, startCommitID, endCommitID)
diffTree, err := gitdiff.GetDiffTree(ctx, gitRepo, false, beforeCommitID, afterCommitID)
if err != nil {
ctx.ServerError("GetDiffTree", err)
return
@ -836,17 +825,6 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
ctx.Data["Diff"] = diff
ctx.Data["DiffNotAvailable"] = diffShortStat.NumFiles == 0
baseCommit, err := ctx.Repo.GitRepo.GetCommit(startCommitID)
if err != nil {
ctx.ServerError("GetCommit", err)
return
}
commit, err := gitRepo.GetCommit(endCommitID)
if err != nil {
ctx.ServerError("GetCommit", err)
return
}
if ctx.IsSigned && ctx.Doer != nil {
if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil {
ctx.ServerError("CanMarkConversation", err)
@ -854,7 +832,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
}
}
setCompareContext(ctx, baseCommit, commit, ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
setCompareContext(ctx, beforeCommit, afterCommit, ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
assigneeUsers, err := repo_model.GetRepoAssignees(ctx, ctx.Repo.Repository)
if err != nil {
@ -901,7 +879,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool {
return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee)
}
if !willShowSpecifiedCommit && !willShowSpecifiedCommitRange && pull.Flow == issues_model.PullRequestFlowGithub {
if !isShowAllCommits && pull.Flow == issues_model.PullRequestFlowGithub {
if err := pull.LoadHeadRepo(ctx); err != nil {
ctx.ServerError("LoadHeadRepo", err)
return
@ -930,19 +908,19 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
}
func ViewPullFilesForSingleCommit(ctx *context.Context) {
viewPullFiles(ctx, "", ctx.PathParam("sha"), true, true)
viewPullFiles(ctx, ctx.PathParam("sha"), ctx.PathParam("sha"))
}
func ViewPullFilesForRange(ctx *context.Context) {
viewPullFiles(ctx, ctx.PathParam("shaFrom"), ctx.PathParam("shaTo"), true, false)
viewPullFiles(ctx, ctx.PathParam("shaFrom"), ctx.PathParam("shaTo"))
}
func ViewPullFilesStartingFromCommit(ctx *context.Context) {
viewPullFiles(ctx, "", ctx.PathParam("sha"), true, false)
viewPullFiles(ctx, ctx.PathParam("sha"), "")
}
func ViewPullFilesForAllCommitsOfPr(ctx *context.Context) {
viewPullFiles(ctx, "", "", false, false)
viewPullFiles(ctx, "", "")
}
// UpdatePullRequest merge PR's baseBranch into headBranch

56
routers/web/repo/pull_review.go
View File

@ -16,6 +16,7 @@ import (
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/context/upload"
@ -49,12 +50,8 @@ func RenderNewCodeCommentForm(ctx *context.Context) {
ctx.Data["PageIsPullFiles"] = true
ctx.Data["Issue"] = issue
ctx.Data["CurrentReview"] = currentReview
pullHeadCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(issue.PullRequest.GetGitHeadRefName())
if err != nil {
ctx.ServerError("GetRefCommitID", err)
return
}
ctx.Data["AfterCommitID"] = pullHeadCommitID
ctx.Data["BeforeCommitID"] = ctx.FormString("before_commit_id")
ctx.Data["AfterCommitID"] = ctx.FormString("after_commit_id")
ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled
upload.AddUploadContext(ctx, "comment")
ctx.HTML(http.StatusOK, tplNewComment)
@ -77,10 +74,7 @@ func CreateCodeComment(ctx *context.Context) {
return
}
signedLine := form.Line
if form.Side == "previous" {
signedLine *= -1
}
signedLine := util.Iif(form.Side == "previous", -form.Line, form.Line)
var attachments []string
if setting.Attachment.Enabled {
@ -92,11 +86,12 @@ func CreateCodeComment(ctx *context.Context) {
ctx.Repo.GitRepo,
issue,
signedLine,
form.BeforeCommitID,
form.AfterCommitID,
form.Content,
form.TreePath,
!form.SingleReview,
form.Reply,
form.LatestCommitID,
attachments,
)
if err != nil {
@ -112,7 +107,7 @@ func CreateCodeComment(ctx *context.Context) {
log.Trace("Comment created: %-v #%d[%d] Comment[%d]", ctx.Repo.Repository, issue.Index, issue.ID, comment.ID)
renderConversation(ctx, comment, form.Origin)
renderConversation(ctx, comment, form.Origin, form.BeforeCommitID, form.AfterCommitID)
}
// UpdateResolveConversation add or remove an Conversation resolved mark
@ -163,14 +158,33 @@ func UpdateResolveConversation(ctx *context.Context) {
return
}
renderConversation(ctx, comment, origin)
beforeCommitID, afterCommitID := ctx.FormString("before_commit_id"), ctx.FormString("after_commit_id")
renderConversation(ctx, comment, origin, beforeCommitID, afterCommitID)
}
func renderConversation(ctx *context.Context, comment *issues_model.Comment, origin string) {
func renderConversation(ctx *context.Context, comment *issues_model.Comment, origin, beforeCommitID, afterCommitID string) {
ctx.Data["PageIsPullFiles"] = origin == "diff"
if err := comment.Issue.LoadPullRequest(ctx); err != nil {
ctx.ServerError("comment.Issue.LoadPullRequest", err)
return
}
if beforeCommitID == "" {
beforeCommitID = comment.Issue.PullRequest.MergeBase
}
if afterCommitID == "" {
var err error
afterCommitID, err = ctx.Repo.GitRepo.GetRefCommitID(comment.Issue.PullRequest.GetGitHeadRefName())
if err != nil {
ctx.ServerError("GetRefCommitID", err)
return
}
}
showOutdatedComments := origin == "timeline" || ctx.Data["ShowOutdatedComments"].(bool)
comments, err := issues_model.FetchCodeCommentsByLine(ctx, comment.Issue, ctx.Doer, comment.TreePath, comment.Line, showOutdatedComments)
comments, err := pull_service.FetchCodeCommentsByLine(ctx, ctx.Repo.Repository, comment.IssueID,
ctx.Doer, beforeCommitID, afterCommitID, comment.TreePath, comment.Line, showOutdatedComments)
if err != nil {
ctx.ServerError("FetchCodeCommentsByLine", err)
return
@ -195,16 +209,8 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment, ori
return
}
ctx.Data["Issue"] = comment.Issue
if err = comment.Issue.LoadPullRequest(ctx); err != nil {
ctx.ServerError("comment.Issue.LoadPullRequest", err)
return
}
pullHeadCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(comment.Issue.PullRequest.GetGitHeadRefName())
if err != nil {
ctx.ServerError("GetRefCommitID", err)
return
}
ctx.Data["AfterCommitID"] = pullHeadCommitID
ctx.Data["BeforeCommitID"] = beforeCommitID
ctx.Data["AfterCommitID"] = afterCommitID
ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool {
return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee)
}

14
routers/web/repo/pull_review_test.go
View File

@ -41,7 +41,7 @@ func TestRenderConversation(t *testing.T) {
var preparedComment *issues_model.Comment
run("prepare", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
comment, err := pull.CreateCodeComment(ctx, pr.Issue.Poster, ctx.Repo.GitRepo, pr.Issue, 1, "content", "", false, 0, pr.HeadCommitID, nil)
comment, err := pull.CreateCodeComment(ctx, pr.Issue.Poster, ctx.Repo.GitRepo, pr.Issue, 1, "", "", "content", "", false, 0, nil)
require.NoError(t, err)
comment.Invalidated = true
@ -54,29 +54,29 @@ func TestRenderConversation(t *testing.T) {
run("diff with outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
ctx.Data["ShowOutdatedComments"] = true
renderConversation(ctx, preparedComment, "diff")
renderConversation(ctx, preparedComment, "diff", "", "")
assert.Contains(t, resp.Body.String(), `<div class="content comment-container"`)
})
run("diff without outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
ctx.Data["ShowOutdatedComments"] = false
renderConversation(ctx, preparedComment, "diff")
renderConversation(ctx, preparedComment, "diff", "", "")
assert.Contains(t, resp.Body.String(), `conversation-not-existing`)
})
run("timeline with outdated", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
ctx.Data["ShowOutdatedComments"] = true
renderConversation(ctx, preparedComment, "timeline")
renderConversation(ctx, preparedComment, "timeline", "", "")
assert.Contains(t, resp.Body.String(), `<div id="code-comments-`)
})
run("timeline is not affected by ShowOutdatedComments=false", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
ctx.Data["ShowOutdatedComments"] = false
renderConversation(ctx, preparedComment, "timeline")
renderConversation(ctx, preparedComment, "timeline", "", "")
assert.Contains(t, resp.Body.String(), `<div id="code-comments-`)
})
run("diff non-existing review", func(t *testing.T, ctx *context.Context, resp *httptest.ResponseRecorder) {
err := db.TruncateBeans(db.DefaultContext, &issues_model.Review{})
assert.NoError(t, err)
ctx.Data["ShowOutdatedComments"] = true
renderConversation(ctx, preparedComment, "diff")
renderConversation(ctx, preparedComment, "diff", "", "")
assert.Equal(t, http.StatusOK, resp.Code)
assert.NotContains(t, resp.Body.String(), `status-page-500`)
})
@ -84,7 +84,7 @@ func TestRenderConversation(t *testing.T) {
err := db.TruncateBeans(db.DefaultContext, &issues_model.Review{})
assert.NoError(t, err)
ctx.Data["ShowOutdatedComments"] = true
renderConversation(ctx, preparedComment, "timeline")
renderConversation(ctx, preparedComment, "timeline", "", "")
assert.Equal(t, http.StatusOK, resp.Code)
assert.NotContains(t, resp.Body.String(), `status-page-500`)
})

48
services/convert/pull_review.go
View File

@ -89,32 +89,30 @@ func ToPullReviewCommentList(ctx context.Context, review *issues_model.Review, d
apiComments := make([]*api.PullReviewComment, 0, len(review.CodeComments))
for _, lines := range review.CodeComments {
for _, comments := range lines {
for _, comment := range comments {
apiComment := &api.PullReviewComment{
ID: comment.ID,
Body: comment.Content,
Poster: ToUser(ctx, comment.Poster, doer),
Resolver: ToUser(ctx, comment.ResolveDoer, doer),
ReviewID: review.ID,
Created: comment.CreatedUnix.AsTime(),
Updated: comment.UpdatedUnix.AsTime(),
Path: comment.TreePath,
CommitID: comment.CommitSHA,
OrigCommitID: comment.OldRef,
DiffHunk: patch2diff(comment.Patch),
HTMLURL: comment.HTMLURL(ctx),
HTMLPullURL: review.Issue.HTMLURL(),
}
if comment.Line < 0 {
apiComment.OldLineNum = comment.UnsignedLine()
} else {
apiComment.LineNum = comment.UnsignedLine()
}
apiComments = append(apiComments, apiComment)
for _, comments := range review.CodeComments {
for _, comment := range comments {
apiComment := &api.PullReviewComment{
ID: comment.ID,
Body: comment.Content,
Poster: ToUser(ctx, comment.Poster, doer),
Resolver: ToUser(ctx, comment.ResolveDoer, doer),
ReviewID: review.ID,
Created: comment.CreatedUnix.AsTime(),
Updated: comment.UpdatedUnix.AsTime(),
Path: comment.TreePath,
CommitID: comment.CommitSHA,
OrigCommitID: comment.OldRef,
DiffHunk: patch2diff(comment.Patch),
HTMLURL: comment.HTMLURL(ctx),
HTMLPullURL: review.Issue.HTMLURL(),
}
if comment.Line < 0 {
apiComment.OldLineNum = comment.UnsignedLine()
} else {
apiComment.LineNum = comment.UnsignedLine()
}
apiComments = append(apiComments, apiComment)
}
}
return apiComments, nil

28
services/feed/notifier.go
View File

@ -223,21 +223,19 @@ func (a *actionNotifier) PullRequestReview(ctx context.Context, pr *issues_model
}
actions := make([]*activities_model.Action, 0, 10)
for _, lines := range review.CodeComments {
for _, comments := range lines {
for _, comm := range comments {
actions = append(actions, &activities_model.Action{
ActUserID: review.Reviewer.ID,
ActUser: review.Reviewer,
Content: fmt.Sprintf("%d|%s", review.Issue.Index, strings.Split(comm.Content, "\n")[0]),
OpType: activities_model.ActionCommentPull,
RepoID: review.Issue.RepoID,
Repo: review.Issue.Repo,
IsPrivate: review.Issue.Repo.IsPrivate,
Comment: comm,
CommentID: comm.ID,
})
}
for _, comments := range review.CodeComments {
for _, comm := range comments {
actions = append(actions, &activities_model.Action{
ActUserID: review.Reviewer.ID,
ActUser: review.Reviewer,
Content: fmt.Sprintf("%d|%s", review.Issue.Index, strings.Split(comm.Content, "\n")[0]),
OpType: activities_model.ActionCommentPull,
RepoID: review.Issue.RepoID,
Repo: review.Issue.Repo,
IsPrivate: review.Issue.Repo.IsPrivate,
Comment: comm,
CommentID: comm.ID,
})
}
}

3
services/forms/repo_form.go
View File

@ -558,7 +558,8 @@ type CodeCommentForm struct {
TreePath string `form:"path" binding:"Required"`
SingleReview bool `form:"single_review"`
Reply int64 `form:"reply"`
LatestCommitID string
BeforeCommitID string `form:"before_commit_id"`
AfterCommitID string `form:"after_commit_id"`
Files []string
}

28
services/gitdiff/gitdiff.go
View File

@ -13,7 +13,6 @@ import (
"html/template"
"io"
"net/url"
"sort"
"strings"
"time"
@ -21,7 +20,6 @@ import (
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
pull_model "code.gitea.io/gitea/models/pull"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/analyze"
"code.gitea.io/gitea/modules/charset"
"code.gitea.io/gitea/modules/git"
@ -454,32 +452,6 @@ type Diff struct {
NumViewedFiles int // user-specific
}
// LoadComments loads comments into each line
func (diff *Diff) LoadComments(ctx context.Context, issue *issues_model.Issue, currentUser *user_model.User, showOutdatedComments bool) error {
allComments, err := issues_model.FetchCodeComments(ctx, issue, currentUser, showOutdatedComments)
if err != nil {
return err
}
for _, file := range diff.Files {
if lineCommits, ok := allComments[file.Name]; ok {
for _, section := range file.Sections {
for _, line := range section.Lines {
if comments, ok := lineCommits[int64(line.LeftIdx*-1)]; ok {
line.Comments = append(line.Comments, comments...)
}
if comments, ok := lineCommits[int64(line.RightIdx)]; ok {
line.Comments = append(line.Comments, comments...)
}
sort.SliceStable(line.Comments, func(i, j int) bool {
return line.Comments[i].CreatedUnix < line.Comments[j].CreatedUnix
})
}
}
}
}
return nil
}
const cmdDiffHead = "diff --git "
// ParsePatch builds a Diff object from a io.Reader and some parameters.

42
services/gitdiff/gitdiff_test.go
View File

@ -11,8 +11,6 @@ import (
"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/json"
"code.gitea.io/gitea/modules/setting"
@ -545,46 +543,6 @@ index 0000000..6bb8f39
}
}
func setupDefaultDiff() *Diff {
return &Diff{
Files: []*DiffFile{
{
Name: "README.md",
Sections: []*DiffSection{
{
Lines: []*DiffLine{
{
LeftIdx: 4,
RightIdx: 4,
},
},
},
},
},
},
}
}
func TestDiff_LoadCommentsNoOutdated(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
diff := setupDefaultDiff()
assert.NoError(t, diff.LoadComments(db.DefaultContext, issue, user, false))
assert.Len(t, diff.Files[0].Sections[0].Lines[0].Comments, 2)
}
func TestDiff_LoadCommentsWithOutdated(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
diff := setupDefaultDiff()
assert.NoError(t, diff.LoadComments(db.DefaultContext, issue, user, true))
assert.Len(t, diff.Files[0].Sections[0].Lines[0].Comments, 3)
}
func TestDiffLine_CanComment(t *testing.T) {
assert.False(t, (&DiffLine{Type: DiffLineSection}).CanComment())
assert.False(t, (&DiffLine{Type: DiffLineAdd, Comments: []*issues_model.Comment{{Content: "bla"}}}).CanComment())

3
services/mailer/incoming/incoming_handler.go
View File

@ -122,11 +122,12 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u
nil,
issue,
comment.Line,
"", // no special commit ID, so we use the merge base of the pull request
"", // no special commit ID, so we use the current HEAD of the pull request
content.Content,
comment.TreePath,
false, // not pending review but a single review
comment.ReviewID,
"",
attachmentIDs,
)
if err != nil {

7
services/mailer/mail_issue_common.go
View File

@ -90,12 +90,7 @@ func composeIssueCommentMessages(ctx context.Context, comment *mailComment, lang
fallback = prefix + fallbackIssueMailSubject(comment.Issue)
if comment.Comment != nil && comment.Comment.Review != nil {
reviewComments = make([]*issues_model.Comment, 0, 10)
for _, lines := range comment.Comment.Review.CodeComments {
for _, comments := range lines {
reviewComments = append(reviewComments, comments...)
}
}
reviewComments = comment.Comment.Review.CodeComments.AllComments()
}
locale := translation.NewLocale(lang)

297
services/pull/review.go
View File

@ -10,18 +10,20 @@ import (
"fmt"
"io"
"regexp"
"sort"
"strings"
"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/cache"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/gitdiff"
notify_service "code.gitea.io/gitea/services/notify"
)
@ -91,12 +93,48 @@ func InvalidateCodeComments(ctx context.Context, prs issues_model.PullRequestLis
}
// CreateCodeComment creates a comment on the code line
func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, issue *issues_model.Issue, line int64, content, treePath string, pendingReview bool, replyReviewID int64, latestCommitID string, attachments []string) (*issues_model.Comment, error) {
func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.Repository,
issue *issues_model.Issue, line int64, beforeCommitID, afterCommitID, content, treePath string,
pendingReview bool, replyReviewID int64, attachments []string,
) (*issues_model.Comment, error) {
var (
existsReview bool
err error
)
if gitRepo == nil {
var closer io.Closer
gitRepo, closer, err = gitrepo.RepositoryFromContextOrOpen(ctx, issue.Repo)
if err != nil {
return nil, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)
}
defer closer.Close()
}
if beforeCommitID == "" {
beforeCommitID = issue.PullRequest.MergeBase
} else {
beforeCommit, err := gitRepo.GetCommit(beforeCommitID) // Ensure beforeCommitID is valid
if err != nil {
return nil, fmt.Errorf("GetCommit[%s]: %w", beforeCommitID, err)
}
// TODO: beforeCommitID must be one of the pull request commits
beforeCommit, err = beforeCommit.Parent(0)
if err != nil {
return nil, fmt.Errorf("GetParent[%s]: %w", beforeCommitID, err)
}
beforeCommitID = beforeCommit.ID.String()
}
if afterCommitID == "" {
afterCommitID, err = gitRepo.GetRefCommitID(issue.PullRequest.GetGitHeadRefName())
if err != nil {
return nil, fmt.Errorf("GetRefCommitID[%s]: %w", issue.PullRequest.GetGitHeadRefName(), err)
}
} else { //nolint
// TODO: afterCommitID must be one of the pull request commits
}
// CreateCodeComment() is used for:
// - Single comments
// - Comments that are part of a review
@ -119,7 +157,10 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
comment, err := createCodeComment(ctx,
doer,
issue.Repo,
gitRepo,
issue,
beforeCommitID,
afterCommitID,
content,
treePath,
line,
@ -151,7 +192,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
Reviewer: doer,
Issue: issue,
Official: false,
CommitID: latestCommitID,
CommitID: afterCommitID,
}); err != nil {
return nil, err
}
@ -160,7 +201,10 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
comment, err := createCodeComment(ctx,
doer,
issue.Repo,
gitRepo,
issue,
beforeCommitID,
afterCommitID,
content,
treePath,
line,
@ -173,7 +217,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
if !pendingReview && !existsReview {
// Submit the review we've just created so the comment shows up in the issue view
if _, _, err = SubmitReview(ctx, doer, gitRepo, issue, issues_model.ReviewTypeComment, "", latestCommitID, nil); err != nil {
if _, _, err = SubmitReview(ctx, doer, gitRepo, issue, issues_model.ReviewTypeComment, "", afterCommitID, nil); err != nil {
return nil, err
}
}
@ -183,9 +227,16 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
return comment, nil
}
func patchCacheKey(issueID int64, beforeCommitID, afterCommitID, treePath string, line int64) string {
// The key is used to cache the patch for a specific line in a review comment.
// It is composed of the issue ID, commit IDs, tree path and line number.
return fmt.Sprintf("review-line-patch-%d-%s-%s-%s-%d", issueID, beforeCommitID, afterCommitID, treePath, line)
}
// createCodeComment creates a plain code comment at the specified line / path
func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, content, treePath string, line, reviewID int64, attachments []string) (*issues_model.Comment, error) {
var commitID, patch string
func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository,
issue *issues_model.Issue, beforeCommitID, afterCommitID, content, treePath string, line, reviewID int64, attachments []string,
) (*issues_model.Comment, error) {
if err := issue.LoadPullRequest(ctx); err != nil {
return nil, fmt.Errorf("LoadPullRequest: %w", err)
}
@ -193,83 +244,31 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
if err := pr.LoadBaseRepo(ctx); err != nil {
return nil, fmt.Errorf("LoadBaseRepo: %w", err)
}
gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
if err != nil {
return nil, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)
}
defer closer.Close()
invalidated := false
head := pr.GetGitHeadRefName()
if line > 0 {
if reviewID != 0 {
first, err := issues_model.FindComments(ctx, &issues_model.FindCommentsOptions{
ReviewID: reviewID,
Line: line,
TreePath: treePath,
Type: issues_model.CommentTypeCode,
ListOptions: db.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 && !issues_model.IsErrCommentNotExist(err) {
return nil, fmt.Errorf("Find first comment for %d line %d path %s. Error: %w", reviewID, line, treePath, err)
} else {
review, err := issues_model.GetReviewByID(ctx, reviewID)
if err == nil && len(review.CommitID) > 0 {
head = review.CommitID
} else if err != nil && !issues_model.IsErrReviewNotExist(err) {
return nil, fmt.Errorf("GetReviewByID %d. Error: %w", 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]: %w", pr.GetGitHeadRefName(), gitRepo.Path, treePath, line, err)
}
}
}
// Only fetch diff if comment is review comment
if len(patch) == 0 && reviewID != 0 {
headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitHeadRefName())
if err != nil {
return nil, fmt.Errorf("GetRefCommitID[%s]: %w", pr.GetGitHeadRefName(), err)
}
if len(commitID) == 0 {
commitID = headCommitID
}
patch, err := cache.GetString(patchCacheKey(issue.ID, beforeCommitID, afterCommitID, treePath, line), func() (string, error) {
reader, writer := io.Pipe()
defer func() {
_ = reader.Close()
_ = writer.Close()
}()
go func() {
if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, treePath, writer); err != nil {
_ = writer.CloseWithError(fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %w", gitRepo.Path, pr.MergeBase, headCommitID, treePath, err))
if err := git.GetRepoRawDiffForFile(gitRepo, beforeCommitID, afterCommitID, git.RawDiffNormal, treePath, writer); err != nil {
_ = writer.CloseWithError(fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %w", gitRepo.Path, beforeCommitID, afterCommitID, treePath, err))
return
}
_ = writer.Close()
}()
patch, err = git.CutDiffAroundLine(reader, int64((&issues_model.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
if err != nil {
log.Error("Error whilst generating patch: %v", err)
return nil, err
}
return git.CutDiffAroundLine(reader, int64((&issues_model.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
})
if err != nil {
return nil, fmt.Errorf("GetPatch failed: %w", err)
}
lineCommitID := util.Iif(line < 0, beforeCommitID, afterCommitID)
// TODO: the commit ID Must be referenced in the git repository, because the branch maybe rebased or force-pushed.
// If the commit ID is not referenced, it cannot be calculated the position dynamically.
return issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{
Type: issues_model.CommentTypeCode,
Doer: doer,
@ -278,10 +277,10 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
Content: content,
LineNum: line,
TreePath: treePath,
CommitSHA: commitID,
CommitSHA: lineCommitID,
ReviewID: reviewID,
Patch: patch,
Invalidated: invalidated,
Invalidated: false,
Attachments: attachments,
})
}
@ -328,15 +327,13 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos
notify_service.PullRequestReview(ctx, pr, review, comm, mentions)
for _, lines := range review.CodeComments {
for _, comments := range lines {
for _, codeComment := range comments {
mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, doer, codeComment.Content)
if err != nil {
return nil, nil, err
}
notify_service.PullRequestCodeComment(ctx, pr, codeComment, mentions)
for _, fileComments := range review.CodeComments {
for _, codeComment := range fileComments {
mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, doer, codeComment.Content)
if err != nil {
return nil, nil, err
}
notify_service.PullRequestCodeComment(ctx, pr, codeComment, mentions)
}
}
@ -471,3 +468,143 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string,
return comment, nil
}
// ReCalculateLineNumber recalculates the line number based on the hunks of the diff.
// If the returned line number is zero, it should not be displayed.
func ReCalculateLineNumber(hunks []*git.HunkInfo, leftLine int64) int64 {
if len(hunks) == 0 || leftLine == 0 {
return leftLine
}
isLeft := leftLine < 0
absLine := leftLine
if isLeft {
absLine = -leftLine
}
newLine := absLine
for _, hunk := range hunks {
if hunk.LeftLine+hunk.LeftHunk <= absLine {
newLine += hunk.RightHunk - hunk.LeftHunk
} else if hunk.LeftLine <= absLine && absLine < hunk.LeftLine+hunk.LeftHunk {
// The line has been removed, so it should not be displayed
return 0
} else if absLine < hunk.LeftLine {
// The line is before the hunk, so we can ignore it
continue
}
}
return util.Iif(isLeft, -newLine, newLine)
}
// FetchCodeCommentsByLine fetches the code comments for a given commit, treePath and line number of a pull request.
func FetchCodeCommentsByLine(ctx context.Context, repo *repo_model.Repository, issueID int64, currentUser *user_model.User, startCommitID, endCommitID, treePath string, line int64, showOutdatedComments bool) (issues_model.CommentList, error) {
opts := issues_model.FindCommentsOptions{
Type: issues_model.CommentTypeCode,
IssueID: issueID,
TreePath: treePath,
}
// load all the comments on this file and then filter them by line number
// we cannot use the line number in the options because some comments's line number may have changed
comments, err := issues_model.FindCodeComments(ctx, opts, repo, currentUser, nil, showOutdatedComments)
if err != nil {
return nil, fmt.Errorf("FindCodeComments: %w", err)
}
if len(comments) == 0 {
return nil, nil
}
n := 0
hunksCache := make(map[string][]*git.HunkInfo)
for _, comment := range comments {
if comment.CommitSHA == endCommitID {
if comment.Line == line {
comments[n] = comment
n++
}
continue
}
// If the comment is not for the current commit, we need to recalculate the line number
hunks, ok := hunksCache[comment.CommitSHA]
if !ok {
hunks, err = git.GetAffectedHunksForTwoCommitsSpecialFile(ctx, repo.RepoPath(), comment.CommitSHA, endCommitID, treePath)
if err != nil {
return nil, fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s]: %w", repo.FullName(), comment.CommitSHA, endCommitID, err)
}
hunksCache[comment.CommitSHA] = hunks
}
comment.Line = ReCalculateLineNumber(hunks, comment.Line)
comments[n] = comment
n++
}
return comments[:n], nil
}
// LoadCodeComments loads comments into each line, so that the comments can be displayed in the diff view.
// the comments' line number is recalculated based on the hunks of the diff.
func LoadCodeComments(ctx context.Context, gitRepo *git.Repository, repo *repo_model.Repository, diff *gitdiff.Diff, issueID int64, currentUser *user_model.User, startCommit, endCommit *git.Commit, showOutdatedComments bool) error {
if startCommit == nil || endCommit == nil {
return errors.New("startCommit and endCommit cannot be nil")
}
allComments, err := issues_model.FetchCodeComments(ctx, repo, issueID, currentUser, showOutdatedComments)
if err != nil {
return err
}
for _, file := range diff.Files {
if fileComments, ok := allComments[file.Name]; ok {
lineComments := make(map[int64][]*issues_model.Comment)
hunksCache := make(map[string][]*git.HunkInfo)
// filecomments should be sorted by created time, so that the latest comments are at the end
for _, comment := range fileComments {
dstCommitID := startCommit.ID.String()
if comment.Line > 0 {
dstCommitID = endCommit.ID.String()
}
if comment.CommitSHA == dstCommitID {
lineComments[comment.Line] = append(lineComments[comment.Line], comment)
continue
}
// If the comment is not for the current commit, we need to recalculate the line number
hunks, ok := hunksCache[comment.CommitSHA+".."+dstCommitID]
if !ok {
hunks, err = git.GetAffectedHunksForTwoCommitsSpecialFile(ctx, repo.RepoPath(), comment.CommitSHA, dstCommitID, file.Name)
if err != nil {
return fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s]: %w", repo.FullName(), dstCommitID, comment.CommitSHA, err)
}
hunksCache[comment.CommitSHA+".."+dstCommitID] = hunks
}
comment.Line = ReCalculateLineNumber(hunks, comment.Line)
if comment.Line != 0 {
dstCommit, err := gitRepo.GetCommit(dstCommitID)
if err != nil {
return fmt.Errorf("GetCommit[%s]: %w", dstCommitID, err)
}
// If the comment is not the first one or the comment created before the current commit
if len(lineComments[comment.Line]) > 0 || comment.CreatedUnix.AsTime().Before(dstCommit.Committer.When) {
lineComments[comment.Line] = append(lineComments[comment.Line], comment)
}
}
}
for _, section := range file.Sections {
for _, line := range section.Lines {
if comments, ok := lineComments[int64(line.LeftIdx*-1)]; ok {
line.Comments = append(line.Comments, comments...)
}
if comments, ok := lineComments[int64(line.RightIdx)]; ok {
line.Comments = append(line.Comments, comments...)
}
sort.SliceStable(line.Comments, func(i, j int) bool {
return line.Comments[i].CreatedUnix < line.Comments[j].CreatedUnix
})
}
}
}
}
return nil
}

89
services/pull/review_test.go
View File

@ -10,6 +10,9 @@ import (
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/services/gitdiff"
pull_service "code.gitea.io/gitea/services/pull"
"github.com/stretchr/testify/assert"
@ -46,3 +49,89 @@ func TestDismissReview(t *testing.T) {
assert.Error(t, err)
assert.True(t, pull_service.IsErrDismissRequestOnClosedPR(err))
}
func setupDefaultDiff() *gitdiff.Diff {
return &gitdiff.Diff{
Files: []*gitdiff.DiffFile{
{
Name: "README.md",
Sections: []*gitdiff.DiffSection{
{
Lines: []*gitdiff.DiffLine{
{
LeftIdx: 4,
RightIdx: 4,
},
},
},
},
},
},
}
}
func TestDiff_LoadCommentsNoOutdated(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
diff := setupDefaultDiff()
assert.NoError(t, issue.LoadRepo(t.Context()))
assert.NoError(t, issue.LoadPullRequest(t.Context()))
gitRepo, err := gitrepo.OpenRepository(t.Context(), issue.Repo)
assert.NoError(t, err)
defer gitRepo.Close()
startCommit, err := gitRepo.GetCommit(issue.PullRequest.MergeBase)
assert.NoError(t, err)
endCommit, err := gitRepo.GetCommit(issue.PullRequest.GetGitHeadRefName())
assert.NoError(t, err)
assert.NoError(t, pull_service.LoadCodeComments(db.DefaultContext, gitRepo, issue.Repo, diff, issue.ID, user, startCommit, endCommit, false))
assert.Len(t, diff.Files[0].Sections[0].Lines[0].Comments, 2)
}
func TestDiff_LoadCommentsWithOutdated(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
assert.NoError(t, issue.LoadRepo(t.Context()))
assert.NoError(t, issue.LoadPullRequest(t.Context()))
diff := setupDefaultDiff()
gitRepo, err := gitrepo.OpenRepository(t.Context(), issue.Repo)
assert.NoError(t, err)
defer gitRepo.Close()
startCommit, err := gitRepo.GetCommit(issue.PullRequest.MergeBase)
assert.NoError(t, err)
endCommit, err := gitRepo.GetCommit(issue.PullRequest.GetGitHeadRefName())
assert.NoError(t, err)
assert.NoError(t, pull_service.LoadCodeComments(db.DefaultContext, gitRepo, issue.Repo, diff, issue.ID, user, startCommit, endCommit, true))
assert.Len(t, diff.Files[0].Sections[0].Lines[0].Comments, 3)
}
func Test_reCalculateLineNumber(t *testing.T) {
hunks := []*git.HunkInfo{
{
LeftLine: 0,
LeftHunk: 0,
RightLine: 1,
RightHunk: 3,
},
}
assert.EqualValues(t, 6, pull_service.ReCalculateLineNumber(hunks, 3))
hunks = []*git.HunkInfo{
{
LeftLine: 1,
LeftHunk: 4,
RightLine: 1,
RightHunk: 4,
},
}
assert.EqualValues(t, 0, pull_service.ReCalculateLineNumber(hunks, 4))
assert.EqualValues(t, 5, pull_service.ReCalculateLineNumber(hunks, 5))
assert.EqualValues(t, 0, pull_service.ReCalculateLineNumber(hunks, -1))
}

2
templates/repo/diff/box.tmpl
View File

@ -184,7 +184,7 @@
{{end}}
</div>
{{else}}
<table class="chroma" data-new-comment-url="{{$.Issue.Link}}/files/reviews/new_comment" data-path="{{$file.Name}}">
<table class="chroma" data-new-comment-url="{{$.Issue.Link}}/files/reviews/new_comment?before_commit_id={{$.BeforeCommitID}}&after_commit_id={{$.AfterCommitID}}" data-path="{{$file.Name}}">
{{if $.IsSplitStyle}}
{{template "repo/diff/section_split" dict "file" . "root" $}}
{{else}}

3
templates/repo/diff/comment_form.tmpl
View File

@ -2,7 +2,8 @@
<form class="ui form {{if $.hidden}}tw-hidden comment-form{{end}}" action="{{$.root.Issue.Link}}/files/reviews/comments" method="post">
{{$.root.CsrfTokenHtml}}
<input type="hidden" name="origin" value="{{if $.root.PageIsPullFiles}}diff{{else}}timeline{{end}}">
<input type="hidden" name="latest_commit_id" value="{{$.root.AfterCommitID}}">
<input type="hidden" name="before_commit_id" value="{{$.root.BeforeCommitID}}">
<input type="hidden" name="after_commit_id" value="{{$.root.AfterCommitID}}">
<input type="hidden" name="side" value="{{if $.Side}}{{$.Side}}{{end}}">
<input type="hidden" name="line" value="{{if $.Line}}{{$.Line}}{{end}}">
<input type="hidden" name="path" value="{{if $.File}}{{$.File}}{{end}}">

6
templates/repo/issue/view_content/comments.tmpl
View File

@ -448,10 +448,8 @@
{{if and .Review .Review.CodeComments}}
<div class="timeline-item event">
{{range $filename, $lines := .Review.CodeComments}}
{{range $line, $comms := $lines}}
{{template "repo/issue/view_content/conversation" dict "." $ "comments" $comms}}
{{end}}
{{range $filename, $comms := .Review.CodeComments}}
{{template "repo/issue/view_content/conversation" dict "." $ "comments" $comms}}
{{end}}
</div>
{{end}}