mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-12 22:10:25 +00:00
While developing tests for #12092, I came across a case where making a comment on a single-commit doesn't include the correct diff for the comment. This is because code comment placement occurs between the PR's base and the commit being viewed, but, that diff could be different from the commit's parent to the commit, which is what is being viewed on a single-commit diff. Similar to #12055, this PR changes code comments to be more precise in their diff generation by providing the backend with both the base commit (`before_commit_id`) and head commit (`after_commit_id`) currently being viewed. As a result, the diffs attached to comments should exactly match the diffs being viewed by the user when the comment was placed. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests for Go changes - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I ran... - [x] `make pr-go` before pushing ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12107 Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org> Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
503 lines
16 KiB
Go
503 lines
16 KiB
Go
// Copyright 2019 The Gitea Authors.
|
|
// All rights reserved.
|
|
// SPDX-License-Identifier: MIT
|
|
|
|
package pull
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"fmt"
|
|
"io"
|
|
|
|
"forgejo.org/models/db"
|
|
issues_model "forgejo.org/models/issues"
|
|
repo_model "forgejo.org/models/repo"
|
|
user_model "forgejo.org/models/user"
|
|
"forgejo.org/modules/git"
|
|
"forgejo.org/modules/gitrepo"
|
|
"forgejo.org/modules/log"
|
|
"forgejo.org/modules/optional"
|
|
"forgejo.org/modules/setting"
|
|
"forgejo.org/modules/util"
|
|
notify_service "forgejo.org/services/notify"
|
|
)
|
|
|
|
// ErrDismissRequestOnClosedPR represents an error when an user tries to dismiss a review associated to a closed or merged PR.
|
|
type ErrDismissRequestOnClosedPR struct{}
|
|
|
|
// IsErrDismissRequestOnClosedPR checks if an error is an ErrDismissRequestOnClosedPR.
|
|
func IsErrDismissRequestOnClosedPR(err error) bool {
|
|
_, ok := err.(ErrDismissRequestOnClosedPR)
|
|
return ok
|
|
}
|
|
|
|
func (err ErrDismissRequestOnClosedPR) Error() string {
|
|
return "can't dismiss a review associated to a closed or merged PR"
|
|
}
|
|
|
|
func (err ErrDismissRequestOnClosedPR) Unwrap() error {
|
|
return util.ErrPermissionDenied
|
|
}
|
|
|
|
// checkInvalidation checks if the line of code comment got changed by another commit.
|
|
// If the line got changed the comment is going to be invalidated.
|
|
func checkInvalidation(ctx context.Context, c *issues_model.Comment, repo *repo_model.Repository, newCommitID string) error {
|
|
reverseBlame, err := c.ResolveCurrentLine(ctx, repo, newCommitID)
|
|
if err != nil {
|
|
log.Warn("ResolveCurrentLine failed: %s", err.Error())
|
|
} else if reverseBlame.CommitID != newCommitID {
|
|
c.Invalidated = true
|
|
return issues_model.UpdateCommentInvalidate(ctx, c)
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// InvalidateCodeComments will lookup the prs for code comments which got invalidated by change
|
|
func InvalidateCodeComments(ctx context.Context, prs issues_model.PullRequestList, doer *user_model.User, repo *repo_model.Repository, newCommitID string) error {
|
|
if len(prs) == 0 {
|
|
return nil
|
|
}
|
|
issueIDs := prs.GetIssueIDs()
|
|
|
|
codeComments, err := db.Find[issues_model.Comment](ctx, issues_model.FindCommentsOptions{
|
|
ListOptions: db.ListOptionsAll,
|
|
Type: issues_model.CommentTypeCode,
|
|
Invalidated: optional.Some(false),
|
|
IssueIDs: issueIDs,
|
|
})
|
|
if err != nil {
|
|
return fmt.Errorf("find code comments: %v", err)
|
|
}
|
|
for _, comment := range codeComments {
|
|
if err := checkInvalidation(ctx, comment, repo, newCommitID); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// 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, beforeCommitID, latestCommitID string, attachments []string,
|
|
) (*issues_model.Comment, error) {
|
|
var (
|
|
existsReview bool
|
|
err error
|
|
)
|
|
|
|
// CreateCodeComment() is used for:
|
|
// - Single comments
|
|
// - Comments that are part of a review
|
|
// - Comments that reply to an existing review
|
|
|
|
if !pendingReview && replyReviewID != 0 {
|
|
// It's not part of a review; maybe a reply to a review comment or a single comment.
|
|
// Check if there are reviews for that line already; if there are, this is a reply
|
|
if existsReview, err = issues_model.ReviewExists(ctx, issue, treePath, line); err != nil {
|
|
return nil, err
|
|
}
|
|
}
|
|
|
|
// Comments that are replies don't require a review header to show up in the issue view
|
|
if !pendingReview && existsReview {
|
|
if err = issue.LoadRepo(ctx); err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
comment, err := CreateCodeCommentKnownReviewID(ctx,
|
|
doer,
|
|
issue.Repo,
|
|
issue,
|
|
content,
|
|
treePath,
|
|
beforeCommitID,
|
|
latestCommitID,
|
|
line,
|
|
replyReviewID,
|
|
attachments,
|
|
)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, doer, comment.Content)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
notify_service.CreateIssueComment(ctx, doer, issue.Repo, issue, comment, mentions)
|
|
|
|
return comment, nil
|
|
}
|
|
|
|
review, err := issues_model.GetCurrentReview(ctx, doer, issue)
|
|
if err != nil {
|
|
if !issues_model.IsErrReviewNotExist(err) {
|
|
return nil, err
|
|
}
|
|
|
|
if review, err = issues_model.CreateReview(ctx, issues_model.CreateReviewOptions{
|
|
Type: issues_model.ReviewTypePending,
|
|
Reviewer: doer,
|
|
Issue: issue,
|
|
Official: false,
|
|
CommitID: latestCommitID,
|
|
}); err != nil {
|
|
return nil, err
|
|
}
|
|
}
|
|
|
|
comment, err := CreateCodeCommentKnownReviewID(ctx,
|
|
doer,
|
|
issue.Repo,
|
|
issue,
|
|
content,
|
|
treePath,
|
|
beforeCommitID,
|
|
latestCommitID,
|
|
line,
|
|
review.ID,
|
|
attachments,
|
|
)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
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 {
|
|
return nil, err
|
|
}
|
|
}
|
|
|
|
// NOTICE: if it's a pending review the notifications will not be fired until user submit review.
|
|
|
|
return comment, nil
|
|
}
|
|
|
|
// CreateCodeCommentKnownReviewID creates a plain code comment at the specified line / path
|
|
func CreateCodeCommentKnownReviewID(ctx context.Context, doer *user_model.User, repo *repo_model.Repository,
|
|
issue *issues_model.Issue, content, treePath, beforeCommitID, afterCommitID string,
|
|
line, reviewID int64, attachments []string,
|
|
) (*issues_model.Comment, error) {
|
|
var commitID, blamedCommitID, patch string
|
|
blamedLine := line
|
|
if err := issue.LoadPullRequest(ctx); err != nil {
|
|
return nil, fmt.Errorf("LoadPullRequest: %w", err)
|
|
}
|
|
pr := issue.PullRequest
|
|
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.GetGitRefName()
|
|
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, lineres, err := gitRepo.LineBlame(afterCommitID, treePath, uint64(line))
|
|
if err == nil {
|
|
blamedCommitID = commit.ID.String()
|
|
blamedLine = int64(lineres)
|
|
} else if !errors.Is(err, git.ErrBlameFileDoesNotExist) && !errors.Is(err, git.ErrBlameFileNotEnoughLines) {
|
|
return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %w", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
|
|
}
|
|
} else {
|
|
blamedCommitID = commitID
|
|
}
|
|
} else {
|
|
// Commenting on a line that was removed. In this case, what we want to track in the comment is which line of
|
|
// code was this, in the last commit that the line of code actually existed in. We'll use a reverse git blame to
|
|
// identify this, from the PR base -> commit being viewed.
|
|
blame, err := gitRepo.ReverseLineBlame(beforeCommitID, treePath, uint64(-1*line), afterCommitID)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("ReverseLineBlame[%s, %s, %d, %s]: %w", beforeCommitID, treePath, -1*line, afterCommitID, err)
|
|
} else if blame.CommitID == afterCommitID {
|
|
// Although this is a comment on the "previous" side of the diff, the reverse blame indicates that the line
|
|
// of code still exists in the commit being viewed (eg. it was a comment on a white line in the left-side of
|
|
// the diff, not a red removed line). In order to record the right information for where to place this
|
|
// commit, we'll convert this into a right-hand comment -- using the present line number that the reverse
|
|
// blame gave us:
|
|
commit, lineres, err := gitRepo.LineBlame(afterCommitID, treePath, blame.LineNumber)
|
|
if err == nil {
|
|
blamedCommitID = commit.ID.String()
|
|
blamedLine = int64(lineres)
|
|
} else if !errors.Is(err, git.ErrBlameFileDoesNotExist) && !errors.Is(err, git.ErrBlameFileNotEnoughLines) {
|
|
return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %w", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
|
|
}
|
|
} else {
|
|
blamedCommitID = blame.CommitID
|
|
// retain negative line numbering to identify we're commenting on the "previous" side of the diff
|
|
blamedLine = -1 * int64(blame.LineNumber)
|
|
}
|
|
}
|
|
|
|
// Only fetch diff if comment is review comment
|
|
if len(patch) == 0 && reviewID != 0 {
|
|
if len(commitID) == 0 {
|
|
commitID, err = gitRepo.GetRefCommitID(head)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("GetRefCommitID[%s]: %w", head, err)
|
|
}
|
|
}
|
|
if len(blamedCommitID) == 0 {
|
|
blamedCommitID = commitID
|
|
}
|
|
reader, writer := io.Pipe()
|
|
defer func() {
|
|
_ = reader.Close()
|
|
_ = writer.Close()
|
|
}()
|
|
go func() {
|
|
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, pr.MergeBase, 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 issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{
|
|
Type: issues_model.CommentTypeCode,
|
|
Doer: doer,
|
|
Repo: repo,
|
|
Issue: issue,
|
|
Content: content,
|
|
LineNum: blamedLine,
|
|
TreePath: treePath,
|
|
CommitSHA: blamedCommitID,
|
|
ReviewID: reviewID,
|
|
Patch: patch,
|
|
Invalidated: invalidated,
|
|
Attachments: attachments,
|
|
})
|
|
}
|
|
|
|
// SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist
|
|
func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, issue *issues_model.Issue, reviewType issues_model.ReviewType, content, commitID string, attachmentUUIDs []string) (*issues_model.Review, *issues_model.Comment, error) {
|
|
if err := issue.LoadPullRequest(ctx); err != nil {
|
|
return nil, nil, err
|
|
}
|
|
|
|
pr := issue.PullRequest
|
|
var stale bool
|
|
if reviewType != issues_model.ReviewTypeApprove && reviewType != issues_model.ReviewTypeReject {
|
|
stale = false
|
|
} else {
|
|
headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
|
|
if err != nil {
|
|
return nil, nil, err
|
|
}
|
|
|
|
if headCommitID == commitID {
|
|
stale = false
|
|
} else {
|
|
testPatchCtx, err := getTestPatchCtx(ctx, pr, true)
|
|
defer testPatchCtx.close()
|
|
if err != nil {
|
|
return nil, nil, err
|
|
}
|
|
|
|
stale, err = testPatchCtx.gitRepo.CheckIfDiffDiffers(testPatchCtx.baseRev, commitID, headCommitID, testPatchCtx.env)
|
|
if err != nil {
|
|
return nil, nil, fmt.Errorf("CheckIfDiffDiffers: %w", err)
|
|
}
|
|
}
|
|
}
|
|
|
|
review, comm, err := issues_model.SubmitReview(ctx, doer, issue, reviewType, content, commitID, stale, attachmentUUIDs)
|
|
if err != nil {
|
|
return nil, nil, err
|
|
}
|
|
|
|
mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, doer, comm.Content)
|
|
if err != nil {
|
|
return nil, nil, err
|
|
}
|
|
|
|
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)
|
|
}
|
|
}
|
|
}
|
|
|
|
return review, comm, nil
|
|
}
|
|
|
|
// DismissApprovalReviews dismiss all approval reviews because of new commits
|
|
func DismissApprovalReviews(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest) error {
|
|
reviews, err := issues_model.FindReviews(ctx, issues_model.FindReviewOptions{
|
|
ListOptions: db.ListOptionsAll,
|
|
IssueID: pull.IssueID,
|
|
Types: []issues_model.ReviewType{issues_model.ReviewTypeApprove},
|
|
Dismissed: optional.Some(false),
|
|
})
|
|
if err != nil {
|
|
return err
|
|
}
|
|
|
|
if err := reviews.LoadIssues(ctx); err != nil {
|
|
return err
|
|
}
|
|
|
|
return db.WithTx(ctx, func(ctx context.Context) error {
|
|
for _, review := range reviews {
|
|
if err := issues_model.DismissReview(ctx, review, true); err != nil {
|
|
return err
|
|
}
|
|
|
|
comment, err := issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{
|
|
Doer: doer,
|
|
Content: "New commits pushed, approval review dismissed automatically according to repository settings",
|
|
Type: issues_model.CommentTypeDismissReview,
|
|
ReviewID: review.ID,
|
|
Issue: review.Issue,
|
|
Repo: review.Issue.Repo,
|
|
})
|
|
if err != nil {
|
|
return err
|
|
}
|
|
|
|
comment.Review = review
|
|
comment.Poster = doer
|
|
comment.Issue = review.Issue
|
|
|
|
notify_service.PullReviewDismiss(ctx, doer, review, comment)
|
|
}
|
|
return nil
|
|
})
|
|
}
|
|
|
|
// DismissReview dismissing stale review by repo admin
|
|
func DismissReview(ctx context.Context, reviewID, repoID int64, message string, doer *user_model.User, isDismiss, dismissPriors bool) (comment *issues_model.Comment, err error) {
|
|
review, err := issues_model.GetReviewByID(ctx, reviewID)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
if review.Type != issues_model.ReviewTypeApprove && review.Type != issues_model.ReviewTypeReject {
|
|
return nil, errors.New("not need to dismiss this review because it's type is not Approve or change request")
|
|
}
|
|
|
|
// load data for notify
|
|
if err := review.LoadAttributes(ctx); err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
// Check if the review's repoID is the one we're currently expecting.
|
|
if review.Issue.RepoID != repoID {
|
|
return nil, errors.New("reviews's repository is not the same as the one we expect")
|
|
}
|
|
|
|
issue := review.Issue
|
|
|
|
if issue.IsClosed {
|
|
return nil, ErrDismissRequestOnClosedPR{}
|
|
}
|
|
|
|
if issue.IsPull {
|
|
if err := issue.LoadPullRequest(ctx); err != nil {
|
|
return nil, err
|
|
}
|
|
if issue.PullRequest.HasMerged {
|
|
return nil, ErrDismissRequestOnClosedPR{}
|
|
}
|
|
}
|
|
|
|
if err := issues_model.DismissReview(ctx, review, isDismiss); err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
if dismissPriors {
|
|
reviews, err := issues_model.FindReviews(ctx, issues_model.FindReviewOptions{
|
|
IssueID: review.IssueID,
|
|
ReviewerID: review.ReviewerID,
|
|
Dismissed: optional.Some(false),
|
|
})
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
for _, oldReview := range reviews {
|
|
if err = issues_model.DismissReview(ctx, oldReview, true); err != nil {
|
|
return nil, err
|
|
}
|
|
}
|
|
}
|
|
|
|
if !isDismiss {
|
|
return nil, nil
|
|
}
|
|
|
|
if err := review.Issue.LoadAttributes(ctx); err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
comment, err = issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{
|
|
Doer: doer,
|
|
Content: message,
|
|
Type: issues_model.CommentTypeDismissReview,
|
|
ReviewID: review.ID,
|
|
Issue: review.Issue,
|
|
Repo: review.Issue.Repo,
|
|
})
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
comment.Review = review
|
|
comment.Poster = doer
|
|
comment.Issue = review.Issue
|
|
|
|
notify_service.PullReviewDismiss(ctx, doer, review, comment)
|
|
|
|
return comment, nil
|
|
}
|