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

Merge ff7a6616fb into 82c9589faa

Browse Source
This commit is contained in:
Lunny Xiao 2025-07-26 12:06:31 -07:00 committed by GitHub
commit 94a8f0e48c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
50 changed files with 1383 additions and 691 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
models/issues/comment.go
View File

@ -293,7 +293,8 @@ type Comment struct {
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
// Reference issue in commit message
CommitSHA string `xorm:"VARCHAR(64)"`
BeforeCommitID string `xorm:"VARCHAR(64)"`
CommitSHA string `xorm:"VARCHAR(64)"`
Attachments []*repo_model.Attachment `xorm:"-"`
Reactions ReactionList `xorm:"-"`
@ -764,6 +765,10 @@ func (c *Comment) CodeCommentLink(ctx context.Context) string {
return fmt.Sprintf("%s/files#%s", c.Issue.Link(), c.HashTag())
}
func GetCodeCommentRefName(prIndex, commentID int64, suffix string) string {
return fmt.Sprintf("refs/pull/%d/code-comment-%d", prIndex, commentID)
}
// CreateComment creates comment with context
func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment, err error) {
return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) {
@ -796,6 +801,7 @@ func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment,
AssigneeID: opts.AssigneeID,
AssigneeTeamID: opts.AssigneeTeamID,
CommitID: opts.CommitID,
BeforeCommitID: opts.BeforeCommitID,
CommitSHA: opts.CommitSHA,
Line: opts.LineNum,
Content: opts.Content,
@ -965,7 +971,8 @@ type CreateCommentOptions struct {
OldRef string
NewRef string
CommitID int64
CommitSHA string
BeforeCommitID string // before commit id when creating this code comment
CommitSHA string // after commit id when creating this code comment, ref commit id for other comment
Patch string
LineNum int64
TreePath string

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)
}

15
models/issues/comment_test.go
View File

@ -68,15 +68,20 @@ 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)
fourthLineComments := []*issues_model.Comment{}
for _, comment := range res["README.md"] {
if comment.Line == 4 {
fourthLineComments = append(fourthLineComments, comment)
}
}
assert.Len(t, fourthLineComments, 1)
assert.Equal(t, int64(4), fourthLineComments[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)
}

4
models/issues/issue.go
View File

@ -223,7 +223,7 @@ func (issue *Issue) LoadPullRequest(ctx context.Context) (err error) {
return nil
}
func (issue *Issue) loadComments(ctx context.Context) (err error) {
func (issue *Issue) LoadComments(ctx context.Context) (err error) {
return issue.loadCommentsByType(ctx, CommentTypeUndefined)
}
@ -344,7 +344,7 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
return err
}
if err = issue.loadComments(ctx); err != nil {
if err = issue.LoadComments(ctx); err != nil {
return err
}

28
models/issues/pull.go
View File

@ -15,7 +15,6 @@ import (
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
org_model "code.gitea.io/gitea/models/organization"
pull_model "code.gitea.io/gitea/models/pull"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
@ -156,26 +155,6 @@ func init() {
db.RegisterModel(new(PullRequest))
}
// DeletePullsByBaseRepoID deletes all pull requests by the base repository ID
func DeletePullsByBaseRepoID(ctx context.Context, repoID int64) error {
deleteCond := builder.Select("id").From("pull_request").Where(builder.Eq{"pull_request.base_repo_id": repoID})
// Delete scheduled auto merges
if _, err := db.GetEngine(ctx).In("pull_id", deleteCond).
Delete(&pull_model.AutoMerge{}); err != nil {
return err
}
// Delete review states
if _, err := db.GetEngine(ctx).In("pull_id", deleteCond).
Delete(&pull_model.ReviewState{}); err != nil {
return err
}
_, err := db.DeleteByBean(ctx, &PullRequest{BaseRepoID: repoID})
return err
}
func (pr *PullRequest) String() string {
if pr == nil {
return "<PullRequest nil>"
@ -406,11 +385,16 @@ func (pr *PullRequest) getReviewedByLines(ctx context.Context, writer io.Writer)
return nil
}
// GetGitHeadRefName returns git ref for hidden pull request branch
// GetGitHeadRefName returns git head commit id ref for the pull request's branch
func (pr *PullRequest) GetGitHeadRefName() string {
return fmt.Sprintf("%s%d/head", git.PullPrefix, pr.Index)
}
// GetGitMergeRefName returns git merged commit id ref for the pull request
func (pr *PullRequest) GetGitMergeRefName() string {
return fmt.Sprintf("%s%d/merge", git.PullPrefix, pr.Index)
}
func (pr *PullRequest) GetGitHeadBranchRefName() string {
return fmt.Sprintf("%s%s", git.BranchPrefix, pr.HeadBranch)
}

6
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
}
@ -429,6 +429,10 @@ func SubmitReview(ctx context.Context, doer *user_model.User, issue *Issue, revi
defer committer.Close()
sess := db.GetEngine(ctx)
if err := issue.LoadRepo(ctx); err != nil {
return nil, nil, fmt.Errorf("LoadRepo: %w", err)
}
official := false
review, err := GetCurrentReview(ctx, doer, issue)

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) {

1
models/migrations/migrations.go
View File

@ -386,6 +386,7 @@ func prepareMigrationTasks() []*migration {
// Gitea 1.24.0 ends at database version 321
newMigration(321, "Use LONGTEXT for some columns and fix review_state.updated_files column", v1_25.UseLongTextInSomeColumnsAndFixBugs),
newMigration(322, "Add BeforeCommitID to Comment table", v1_25.AddBeforeCommitIDForComment),
}
return preparedMigrations
}

28
models/migrations/v1_25/v322.go Normal file
View File

@ -0,0 +1,28 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package v1_25
import (
"xorm.io/xorm"
)
type comment struct {
BeforeCommitID string `xorm:"VARCHAR(64)"`
}
// TableName return database table name for xorm
func (comment) TableName() string {
return "comment"
}
func AddBeforeCommitIDForComment(x *xorm.Engine) error {
if _, err := x.SyncWithOptions(xorm.SyncOptions{
IgnoreConstrains: true,
IgnoreIndices: true,
}, new(comment)); err != nil {
return err
}
_, err := x.Exec("UPDATE comment SET before_commit_id = (SELECT merge_base FROM pull_request WHERE pull_request.issue_id = comment.issue_id) WHERE `type`=21 AND before_commit_id IS NULL")
return err
}

103
models/migrations/v1_25/v322_test.go Normal file
View File

@ -0,0 +1,103 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package v1_25
import (
"testing"
"code.gitea.io/gitea/models/migrations/base"
"code.gitea.io/gitea/modules/references"
"code.gitea.io/gitea/modules/timeutil"
"github.com/stretchr/testify/assert"
)
func Test_AddBeforeCommitIDForComment(t *testing.T) {
type Comment struct { // old struct
ID int64 `xorm:"pk autoincr"`
Type int `xorm:"INDEX"`
PosterID int64 `xorm:"INDEX"`
OriginalAuthor string
OriginalAuthorID int64
IssueID int64 `xorm:"INDEX"`
LabelID int64
OldProjectID int64
ProjectID int64
OldMilestoneID int64
MilestoneID int64
TimeID int64
AssigneeID int64
RemovedAssignee bool
AssigneeTeamID int64 `xorm:"NOT NULL DEFAULT 0"`
ResolveDoerID int64
OldTitle string
NewTitle string
OldRef string
NewRef string
DependentIssueID int64 `xorm:"index"` // This is used by issue_service.deleteIssue
CommitID int64
Line int64 // - previous line / + proposed line
TreePath string
Content string `xorm:"LONGTEXT"`
ContentVersion int `xorm:"NOT NULL DEFAULT 0"`
// Path represents the 4 lines of code cemented by this comment
Patch string `xorm:"-"`
PatchQuoted string `xorm:"LONGTEXT patch"`
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
// Reference issue in commit message
CommitSHA string `xorm:"VARCHAR(64)"`
ReviewID int64 `xorm:"index"`
Invalidated bool
// Reference an issue or pull from another comment, issue or PR
// All information is about the origin of the reference
RefRepoID int64 `xorm:"index"` // Repo where the referencing
RefIssueID int64 `xorm:"index"`
RefCommentID int64 `xorm:"index"` // 0 if origin is Issue title or content (or PR's)
RefAction references.XRefAction `xorm:"SMALLINT"` // What happens if RefIssueID resolves
RefIsPull bool
CommentMetaData string `xorm:"JSON TEXT"` // put all non-index metadata in a single field
}
type PullRequest struct {
ID int64 `xorm:"pk autoincr"`
Type int
Status int
ConflictedFiles []string `xorm:"TEXT JSON"`
CommitsAhead int
CommitsBehind int
ChangedProtectedFiles []string `xorm:"TEXT JSON"`
IssueID int64 `xorm:"INDEX"`
Index int64
HeadRepoID int64 `xorm:"INDEX"`
BaseRepoID int64 `xorm:"INDEX"`
HeadBranch string
BaseBranch string
MergeBase string `xorm:"VARCHAR(64)"`
AllowMaintainerEdit bool `xorm:"NOT NULL DEFAULT false"`
HasMerged bool `xorm:"INDEX"`
MergedCommitID string `xorm:"VARCHAR(64)"`
MergerID int64 `xorm:"INDEX"`
MergedUnix timeutil.TimeStamp `xorm:"updated INDEX"`
Flow int `xorm:"NOT NULL DEFAULT 0"`
}
// Prepare and load the testing database
x, deferable := base.PrepareTestEnv(t, 0, new(Comment), new(PullRequest))
defer deferable()
assert.NoError(t, AddBeforeCommitIDForComment(x))
}

6
modules/git/commit.go
View File

@ -258,11 +258,7 @@ func (c *Commit) CommitsBeforeLimit(num int) ([]*Commit, error) {
// CommitsBeforeUntil returns the commits between commitID to current revision
func (c *Commit) CommitsBeforeUntil(commitID string) ([]*Commit, error) {
endCommit, err := c.repo.GetCommit(commitID)
if err != nil {
return nil, err
}
return c.repo.CommitsBetween(c, endCommit)
return c.repo.CommitsBetween(c.ID.String(), commitID)
}
// SearchCommitsOptions specify the parameters for SearchCommits

56
modules/git/diff.go
View File

@ -107,12 +107,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 = 1
}
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 = 1
}
} else {
log.Debug("Parse line number failed: %v", diffHunk)
@ -342,3 +346,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(hunks, &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
}

25
modules/git/diff_test.go
View File

@ -4,6 +4,7 @@
package git
import (
"path/filepath"
"strings"
"testing"
@ -181,4 +182,28 @@ 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, 1, rightLine)
assert.Equal(t, 0, rightHunk)
leftLine, leftHunk, rightLine, rightHunk = ParseDiffHunkString("@@ -2 +2 @@")
assert.Equal(t, 2, leftLine)
assert.Equal(t, 1, leftHunk)
assert.Equal(t, 2, rightLine)
assert.Equal(t, 1, rightHunk)
}
func Test_GetAffectedHunksForTwoCommitsSpecialFile(t *testing.T) {
repoPath := filepath.Join(testReposDir, "repo4_commitsbetween")
hunks, err := GetAffectedHunksForTwoCommitsSpecialFile(t.Context(), repoPath, "fdc1b615bdcff0f0658b216df0c9209e5ecb7c78", "a78e5638b66ccfe7e1b4689d3d5684e42c97d7ca", "test.txt")
assert.NoError(t, err)
assert.Len(t, hunks, 1)
// @@ -1 +1 @@
assert.Equal(t, int64(1), hunks[0].LeftLine)
assert.Equal(t, int64(1), hunks[0].LeftHunk)
assert.Equal(t, int64(1), hunks[0].RightLine)
assert.Equal(t, int64(1), hunks[0].RightHunk)
}

12
modules/git/ref.go
View File

@ -4,6 +4,7 @@
package git
import (
"context"
"regexp"
"strings"
@ -220,3 +221,14 @@ func (ref RefName) RefWebLinkPath() string {
}
return string(refType) + "/" + util.PathEscapeSegments(ref.ShortName())
}
func UpdateRef(ctx context.Context, repoPath, refName, newCommitID string) error {
_, _, err := NewCommand("update-ref").AddDynamicArguments(refName, newCommitID).RunStdString(ctx, &RunOpts{Dir: repoPath})
return err
}
func RemoveRef(ctx context.Context, repoPath, refName string) error {
_, _, err := NewCommand("update-ref", "--no-deref", "-d").
AddDynamicArguments(refName).RunStdString(ctx, &RunOpts{Dir: repoPath})
return err
}

55
modules/git/repo_commit.go
View File

@ -6,6 +6,7 @@ package git
import (
"bytes"
"context"
"io"
"os"
"strconv"
@ -304,23 +305,50 @@ func (repo *Repository) FilesCountBetween(startCommitID, endCommitID string) (in
// CommitsBetween returns a list that contains commits between [before, last).
// If before is detached (removed by reset + push) it is not included.
func (repo *Repository) CommitsBetween(last, before *Commit) ([]*Commit, error) {
func (repo *Repository) CommitsBetween(lastCommitID, beforeCommitID string) ([]*Commit, error) {
commitIDs, err := CommitIDsBetween(repo.Ctx, repo.Path, beforeCommitID, lastCommitID)
if err != nil {
return nil, err
}
commits := make([]*Commit, 0, len(commitIDs))
for _, commitID := range commitIDs {
commit, err := repo.GetCommit(commitID)
if err != nil {
return nil, err
}
commits = append(commits, commit)
}
return commits, nil
}
// CommitIDsBetween returns a list that contains commit IDs between (beforeCommitID, afterCommitID].
// If beforeCommitID is empty, it will return all commits before afterCommitID.
// If beforeCommitID is given, it will not be included in the result.
func CommitIDsBetween(ctx context.Context, repoPath, beforeCommitID, afterCommitID string) ([]string, error) {
var stdout []byte
var err error
if before == nil {
stdout, _, err = NewCommand("rev-list").AddDynamicArguments(last.ID.String()).RunStdBytes(repo.Ctx, &RunOpts{Dir: repo.Path})
if beforeCommitID == "" {
stdout, _, err = NewCommand("rev-list").AddDynamicArguments(afterCommitID).RunStdBytes(ctx, &RunOpts{Dir: repoPath})
} else {
stdout, _, err = NewCommand("rev-list").AddDynamicArguments(before.ID.String()+".."+last.ID.String()).RunStdBytes(repo.Ctx, &RunOpts{Dir: repo.Path})
stdout, _, err = NewCommand("rev-list").AddDynamicArguments(beforeCommitID+".."+afterCommitID).RunStdBytes(ctx, &RunOpts{Dir: repoPath})
if err != nil && strings.Contains(err.Error(), "no merge base") {
// future versions of git >= 2.28 are likely to return an error if before and last have become unrelated.
// previously it would return the results of git rev-list before last so let's try that...
stdout, _, err = NewCommand("rev-list").AddDynamicArguments(before.ID.String(), last.ID.String()).RunStdBytes(repo.Ctx, &RunOpts{Dir: repo.Path})
stdout, _, err = NewCommand("rev-list").AddDynamicArguments(beforeCommitID, afterCommitID).RunStdBytes(ctx, &RunOpts{Dir: repoPath})
}
}
if err != nil {
return nil, err
}
return repo.parsePrettyFormatLogToList(bytes.TrimSpace(stdout))
commitIDs := make([]string, 0, 10)
for commitID := range bytes.SplitSeq(stdout, []byte{'\n'}) {
if len(commitID) > 0 {
commitIDs = append(commitIDs, string(commitID))
}
}
return commitIDs, nil
}
// CommitsBetweenLimit returns a list that contains at most limit commits skipping the first skip commits between [before, last)
@ -375,18 +403,17 @@ func (repo *Repository) CommitsBetweenNotBase(last, before *Commit, baseBranch s
// CommitsBetweenIDs return commits between twoe commits
func (repo *Repository) CommitsBetweenIDs(last, before string) ([]*Commit, error) {
lastCommit, err := repo.GetCommit(last)
_, err := repo.GetCommit(last)
if err != nil {
return nil, err
}
if before == "" {
return repo.CommitsBetween(lastCommit, nil)
if before != "" {
_, err := repo.GetCommit(before)
if err != nil {
return nil, err
}
}
beforeCommit, err := repo.GetCommit(before)
if err != nil {
return nil, err
}
return repo.CommitsBetween(lastCommit, beforeCommit)
return repo.CommitsBetween(last, before)
}
// CommitsCountBetween return numbers of commits between two commits

12
modules/git/repo_commit_nogogit.go
View File

@ -50,18 +50,6 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) {
return string(shaBs), nil
}
// SetReference sets the commit ID string of given reference (e.g. branch or tag).
func (repo *Repository) SetReference(name, commitID string) error {
_, _, err := NewCommand("update-ref").AddDynamicArguments(name, commitID).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path})
return err
}
// RemoveReference removes the given reference (e.g. branch or tag).
func (repo *Repository) RemoveReference(name string) error {
_, _, err := NewCommand("update-ref", "--no-deref", "-d").AddDynamicArguments(name).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path})
return err
}
// IsCommitExist returns true if given commit exists in current repository.
func (repo *Repository) IsCommitExist(name string) bool {
if err := ensureValidGitRepository(repo.Ctx, repo.Path); err != nil {

23
modules/git/repo_commit_test.go
View File

@ -148,3 +148,26 @@ func TestCommitsByFileAndRange(t *testing.T) {
require.NoError(t, err)
assert.Len(t, commits, 1)
}
func Test_CommitIDsBetween(t *testing.T) {
defer test.MockVariableValue(&setting.Git.CommitsRangeSize, 2)()
bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
bareRepo1, err := openRepositoryWithDefaultContext(bareRepo1Path)
require.NoError(t, err)
defer bareRepo1.Close()
// Test with empty beforeCommitID
commitIDs, err := CommitIDsBetween(bareRepo1.Ctx, bareRepo1.Path, "", "master")
require.NoError(t, err)
assert.Len(t, commitIDs, 7)
assert.Equal(t, "ce064814f4a0d337b333e646ece456cd39fab612", commitIDs[0])
assert.Equal(t, "95bb4d39648ee7e325106df01a621c530863a653", commitIDs[6])
// Test with a specific beforeCommitID
commitIDs, err = CommitIDsBetween(bareRepo1.Ctx, bareRepo1.Path, "37991dec2c8e592043f47155ce4808d4580f9123", "master")
require.NoError(t, err)
assert.Len(t, commitIDs, 2)
assert.Equal(t, "ce064814f4a0d337b333e646ece456cd39fab612", commitIDs[0])
assert.Equal(t, "feaf4ba6bc635fec442f46ddd4512416ec43c2c2", commitIDs[1])
}

28
modules/git/repo_compare.go
View File

@ -5,14 +5,10 @@
package git
import (
"bufio"
"bytes"
"context"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"regexp"
"strconv"
"strings"
@ -193,8 +189,6 @@ func GetDiffShortStatByCmdArgs(ctx context.Context, repoPath string, trustedArgs
var shortStatFormat = regexp.MustCompile(
`\s*(\d+) files? changed(?:, (\d+) insertions?\(\+\))?(?:, (\d+) deletions?\(-\))?`)
var patchCommits = regexp.MustCompile(`^From\s(\w+)\s`)
func parseDiffStat(stdout string) (numFiles, totalAdditions, totalDeletions int, err error) {
if len(stdout) == 0 || stdout == "\n" {
return 0, 0, 0, nil
@ -282,25 +276,3 @@ func (repo *Repository) GetFilesChangedBetween(base, head string) ([]string, err
return split, err
}
// ReadPatchCommit will check if a diff patch exists and return stats
func (repo *Repository) ReadPatchCommit(prID int64) (commitSHA string, err error) {
// Migrated repositories download patches to "pulls" location
patchFile := fmt.Sprintf("pulls/%d.patch", prID)
loadPatch, err := os.Open(filepath.Join(repo.Path, patchFile))
if err != nil {
return "", err
}
defer loadPatch.Close()
// Read only the first line of the patch - usually it contains the first commit made in patch
scanner := bufio.NewScanner(loadPatch)
scanner.Scan()
// Parse the Patch stats, sometimes Migration returns a 404 for the patch file
commitSHAGroups := patchCommits.FindStringSubmatch(scanner.Text())
if len(commitSHAGroups) != 0 {
commitSHA = commitSHAGroups[1]
} else {
return "", errors.New("patch file doesn't contain valid commit ID")
}
return commitSHA, nil
}

34
modules/git/repo_compare_test.go
View File

@ -45,36 +45,6 @@ func TestGetFormatPatch(t *testing.T) {
assert.Contains(t, patch, "Subject: [PATCH] Add file2.txt")
}
func TestReadPatch(t *testing.T) {
// Ensure we can read the patch files
bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
repo, err := openRepositoryWithDefaultContext(bareRepo1Path)
if err != nil {
assert.NoError(t, err)
return
}
defer repo.Close()
// This patch doesn't exist
noFile, err := repo.ReadPatchCommit(0)
assert.Error(t, err)
// This patch is an empty one (sometimes it's a 404)
noCommit, err := repo.ReadPatchCommit(1)
assert.Error(t, err)
// This patch is legit and should return a commit
oldCommit, err := repo.ReadPatchCommit(2)
if err != nil {
assert.NoError(t, err)
return
}
assert.Empty(t, noFile)
assert.Empty(t, noCommit)
assert.Len(t, oldCommit, 40)
assert.Equal(t, "6e8e2a6f9efd71dbe6917816343ed8415ad696c3", oldCommit)
}
func TestReadWritePullHead(t *testing.T) {
// Ensure we can write SHA1 head corresponding to PR and open them
bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
@ -99,7 +69,7 @@ func TestReadWritePullHead(t *testing.T) {
// Write a fake sha1 with only 40 zeros
newCommit := "feaf4ba6bc635fec442f46ddd4512416ec43c2c2"
err = repo.SetReference(PullPrefix+"1/head", newCommit)
err = UpdateRef(t.Context(), repo.Path, PullPrefix+"1/head", newCommit)
if err != nil {
assert.NoError(t, err)
return
@ -116,7 +86,7 @@ func TestReadWritePullHead(t *testing.T) {
assert.Equal(t, headContents, newCommit)
// Remove file after the test
err = repo.RemoveReference(PullPrefix + "1/head")
err = RemoveRef(t.Context(), repo.Path, PullPrefix+"1/head")
assert.NoError(t, err)
}

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)

5
routers/private/hook_post_receive_test.go
View File

@ -37,13 +37,14 @@ func TestHandlePullRequestMerging(t *testing.T) {
PullRequestID: pr.ID,
UserID: 2,
}, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, []*repo_module.PushUpdateOptions{
{NewCommitID: "01234567"},
// assume the first commit is merged from this pull request but it's not a real world scenario
{NewCommitID: "65f1bf27bc3bf70f64657658635e66094edbcb4d"},
})
assert.Empty(t, resp.Body.String())
pr, err = issues_model.GetPullRequestByID(db.DefaultContext, pr.ID)
assert.NoError(t, err)
assert.True(t, pr.HasMerged)
assert.Equal(t, "01234567", pr.MergedCommitID)
assert.Equal(t, "65f1bf27bc3bf70f64657658635e66094edbcb4d", pr.MergedCommitID)
unittest.AssertNotExistsBean(t, &pull_model.AutoMerge{ID: autoMerge.ID})
}

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 {

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

@ -190,7 +190,7 @@ func GetPullDiffStats(ctx *context.Context) {
}
pull := issue.PullRequest
mergeBaseCommitID := GetMergedBaseCommitID(ctx, issue)
mergeBaseCommitID := GetMergedBaseCommitID(ctx, pull)
if mergeBaseCommitID == "" {
return // no merge base, do nothing, do not stop the route handler, see below
}
@ -210,48 +210,17 @@ func GetPullDiffStats(ctx *context.Context) {
ctx.Data["DiffShortStat"] = diffShortStat
}
func GetMergedBaseCommitID(ctx *context.Context, issue *issues_model.Issue) string {
pull := issue.PullRequest
var baseCommit string
// Some migrated PR won't have any Base SHA and lose history, try to get one
if pull.MergeBase == "" {
var commitSHA, parentCommit string
// If there is a head or a patch file, and it is readable, grab info
commitSHA, err := ctx.Repo.GitRepo.GetRefCommitID(pull.GetGitHeadRefName())
if err != nil {
// Head File does not exist, try the patch
commitSHA, err = ctx.Repo.GitRepo.ReadPatchCommit(pull.Index)
if err == nil {
// Recreate pull head in files for next time
if err := ctx.Repo.GitRepo.SetReference(pull.GetGitHeadRefName(), commitSHA); err != nil {
log.Error("Could not write head file", err)
}
} else {
// There is no history available
log.Trace("No history file available for PR %d", pull.Index)
}
}
if commitSHA != "" {
// Get immediate parent of the first commit in the patch, grab history back
parentCommit, _, err = git.NewCommand("rev-list", "-1", "--skip=1").AddDynamicArguments(commitSHA).RunStdString(ctx, &git.RunOpts{Dir: ctx.Repo.GitRepo.Path})
if err == nil {
parentCommit = strings.TrimSpace(parentCommit)
}
// Special case on Git < 2.25 that doesn't fail on immediate empty history
if err != nil || parentCommit == "" {
log.Info("No known parent commit for PR %d, error: %v", pull.Index, err)
// bring at least partial history if it can work
parentCommit = commitSHA
}
}
baseCommit = parentCommit
} else {
// Keep an empty history or original commit
baseCommit = pull.MergeBase
func GetMergedBaseCommitID(ctx *context.Context, pull *issues_model.PullRequest) string {
if pull.MergeBase != "" {
return pull.MergeBase
}
return baseCommit
var err error
pull.MergeBase, err = pull_service.CalcMergeBase(ctx, pull)
if err != nil {
log.Error("CalcMergeBase: %v", err)
}
return pull.MergeBase
}
func preparePullViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo {
@ -271,7 +240,13 @@ func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue)
setMergeTarget(ctx, pull)
ctx.Data["HasMerged"] = true
baseCommit := GetMergedBaseCommitID(ctx, issue)
baseCommit := GetMergedBaseCommitID(ctx, pull)
if baseCommit == "" {
ctx.Data["BaseTarget"] = pull.BaseBranch
ctx.Data["NumCommits"] = 0
ctx.Data["NumFiles"] = 0
return nil // no merge base, do nothing
}
compareInfo, err := ctx.Repo.GitRepo.GetCompareInfo(ctx.Repo.Repository.RepoPath(),
baseCommit, pull.GetGitHeadRefName(), false, false)
@ -643,8 +618,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 +637,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 +647,70 @@ 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
isSingleCommit := beforeCommitID == "" && afterCommitID != ""
ctx.Data["IsShowingOnlySingleCommit"] = isSingleCommit
isShowAllCommits := (beforeCommitID == "" || beforeCommitID == prInfo.MergeBase) && (afterCommitID == "" || afterCommitID == headCommitID)
ctx.Data["IsShowingAllCommits"] = isShowAllCommits
if willShowSpecifiedCommit || willShowSpecifiedCommitRange {
if len(specifiedEndCommit) > 0 {
endCommitID = specifiedEndCommit
var beforeCommit, afterCommit *git.Commit
if !isSingleCommit {
if beforeCommitID == "" || beforeCommitID == prInfo.MergeBase {
beforeCommitID = prInfo.MergeBase
// 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
}
} else {
endCommitID = headCommitID
beforeCommit = indexCommit(prInfo.Commits, beforeCommitID)
if beforeCommit == nil {
ctx.HTTPError(http.StatusBadRequest, "before commit not found in PR commits")
return
}
}
if len(specifiedStartCommit) > 0 {
startCommitID = specifiedStartCommit
} else {
startCommitID = prInfo.MergeBase
}
if afterCommitID == "" || afterCommitID == headCommitID {
afterCommitID = headCommitID
}
afterCommit = indexCommit(prInfo.Commits, afterCommitID)
if afterCommit == nil {
ctx.HTTPError(http.StatusBadRequest, "after commit not found in PR commits")
return
}
if isSingleCommit {
beforeCommit, err = afterCommit.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()
}
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["MergeBase"] = prInfo.MergeBase
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 +718,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 +729,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 +737,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 +749,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 +785,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 +805,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 +812,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 +859,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 +888,17 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
}
func ViewPullFilesForSingleCommit(ctx *context.Context) {
viewPullFiles(ctx, "", ctx.PathParam("sha"), true, true)
// it doesn't support showing files from mergebase to the special commit
// otherwise it will be ambiguous
viewPullFiles(ctx, "", ctx.PathParam("sha"))
}
func ViewPullFilesForRange(ctx *context.Context) {
viewPullFiles(ctx, ctx.PathParam("shaFrom"), ctx.PathParam("shaTo"), true, false)
}
func ViewPullFilesStartingFromCommit(ctx *context.Context) {
viewPullFiles(ctx, "", ctx.PathParam("sha"), true, false)
viewPullFiles(ctx, ctx.PathParam("shaFrom"), ctx.PathParam("shaTo"))
}
func ViewPullFilesForAllCommitsOfPr(ctx *context.Context) {
viewPullFiles(ctx, "", "", false, false)
viewPullFiles(ctx, "", "")
}
// UpdatePullRequest merge PR's baseBranch into headBranch

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

@ -7,15 +7,18 @@ import (
"errors"
"fmt"
"net/http"
"slices"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/organization"
pull_model "code.gitea.io/gitea/models/pull"
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/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 +52,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 +76,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 +88,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 +109,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 +160,47 @@ 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
}
headCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(comment.Issue.PullRequest.GetGitHeadRefName())
if err != nil {
ctx.ServerError("GetRefCommitID", err)
return
}
prCommitIDs, err := git.CommitIDsBetween(ctx, ctx.Repo.GitRepo.Path, comment.Issue.PullRequest.MergeBase, headCommitID)
if err != nil {
ctx.ServerError("CommitIDsBetween", err)
return
}
if beforeCommitID == "" || beforeCommitID == comment.Issue.PullRequest.MergeBase {
beforeCommitID = comment.Issue.PullRequest.MergeBase
} else if !slices.Contains(prCommitIDs, beforeCommitID) { // beforeCommitID must be one of the pull request commits
ctx.HTTPError(http.StatusBadRequest, fmt.Sprintf("beforeCommitID[%s] is not a valid pull request commit", beforeCommitID))
return
}
if afterCommitID == "" || afterCommitID == headCommitID {
afterCommitID = headCommitID
} else if !slices.Contains(prCommitIDs, afterCommitID) { // afterCommitID must be one of the pull request commits
ctx.HTTPError(http.StatusBadRequest, fmt.Sprintf("afterCommitID[%s] is not a valid pull request commit", afterCommitID))
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.GitRepo, ctx.Repo.Repository, comment.IssueID,
ctx.Doer, beforeCommitID, afterCommitID, comment.TreePath, comment.Line, showOutdatedComments)
if err != nil {
ctx.ServerError("FetchCodeCommentsByLine", err)
return
@ -195,16 +225,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`)
})

1
routers/web/web.go
View File

@ -1540,7 +1540,6 @@ func registerWebRoutes(m *web.Router) {
m.Post("/cleanup", context.RepoMustNotBeArchived(), repo.CleanUpPullRequest)
m.Group("/files", func() {
m.Get("", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForAllCommitsOfPr)
m.Get("/{sha:[a-f0-9]{7,40}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesStartingFromCommit)
m.Get("/{shaFrom:[a-f0-9]{7,40}}..{shaTo:[a-f0-9]{7,40}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForRange)
m.Group("/reviews", func() {
m.Get("/new_comment", repo.RenderNewCodeCommentForm)

56
services/convert/pull_review.go
View File

@ -21,6 +21,10 @@ func ToPullReview(ctx context.Context, r *issues_model.Review, doer *user_model.
r.Reviewer = user_model.NewGhostUser()
}
if err := r.Issue.LoadRepo(ctx); err != nil {
return nil, err
}
result := &api.PullReview{
ID: r.ID,
Reviewer: ToUser(ctx, r.Reviewer, doer),
@ -87,34 +91,36 @@ func ToPullReviewCommentList(ctx context.Context, review *issues_model.Review, d
review.Reviewer = user_model.NewGhostUser()
}
if err := review.Issue.LoadRepo(ctx); err != nil {
return nil, err
}
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())

31
services/issue/comments.go
View File

@ -13,8 +13,10 @@ import (
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/json"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/timeutil"
git_service "code.gitea.io/gitea/services/git"
notify_service "code.gitea.io/gitea/services/notify"
@ -132,13 +134,36 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, contentVersion
// DeleteComment deletes the comment
func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) error {
err := db.WithTx(ctx, func(ctx context.Context) error {
if comment.Type == issues_model.CommentTypeCode {
if err := comment.LoadIssue(ctx); err != nil {
return err
}
if err := comment.Issue.LoadRepo(ctx); err != nil {
return err
}
if err := comment.Issue.LoadPullRequest(ctx); err != nil {
return err
}
}
if err := db.WithTx(ctx, func(ctx context.Context) error {
return issues_model.DeleteComment(ctx, comment)
})
if err != nil {
}); err != nil {
return err
}
if comment.Type == issues_model.CommentTypeCode {
// We should not return error here, because the comment has been removed from database.
// users have to delete this ref manually or we should have a synchronize between
// database comment table and git refs.
if err := git.RemoveRef(ctx, comment.Issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(comment.Issue.PullRequest.Index, comment.ID, "before")); err != nil {
log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", comment.Issue.PullRequest.ID, err)
}
if err := git.RemoveRef(ctx, comment.Issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(comment.Issue.PullRequest.Index, comment.ID, "after")); err != nil {
log.Error("Unable to remove ref in base repository for PR[%d] Error: %v", comment.Issue.PullRequest.ID, err)
}
}
notify_service.DeleteComment(ctx, doer, comment)
return nil

180
services/issue/issue.go
View File

@ -12,6 +12,7 @@ import (
issues_model "code.gitea.io/gitea/models/issues"
access_model "code.gitea.io/gitea/models/perm/access"
project_model "code.gitea.io/gitea/models/project"
pull_model "code.gitea.io/gitea/models/pull"
repo_model "code.gitea.io/gitea/models/repo"
system_model "code.gitea.io/gitea/models/system"
user_model "code.gitea.io/gitea/models/user"
@ -200,8 +201,24 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi
// delete pull request related git data
if issue.IsPull && gitRepo != nil {
if err := gitRepo.RemoveReference(issue.PullRequest.GetGitHeadRefName()); err != nil {
return err
if err := git.RemoveRef(ctx, gitRepo.Path, issue.PullRequest.GetGitHeadRefName()); err != nil {
log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issue.PullRequest.GetGitHeadRefName(), issue.PullRequest.ID, err)
}
if issue.PullRequest.HasMerged {
if err := git.RemoveRef(ctx, gitRepo.Path, issue.PullRequest.GetGitMergeRefName()); err != nil {
log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issue.PullRequest.GetGitMergeRefName(), issue.PullRequest.ID, err)
}
}
}
for _, comment := range issue.Comments {
if comment.Type == issues_model.CommentTypeCode {
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "before")); err != nil {
log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "before"), issue.PullRequest.ID, err)
}
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "after")); err != nil {
log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "after"), issue.PullRequest.ID, err)
}
}
}
@ -261,74 +278,102 @@ func GetRefEndNamesAndURLs(issues []*issues_model.Issue, repoLink string) (map[i
// deleteIssue deletes the issue
func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, error) {
ctx, committer, err := db.TxContext(ctx)
if err != nil {
return nil, err
}
defer committer.Close()
if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil {
return nil, err
}
// update the total issue numbers
if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil {
return nil, err
}
// if the issue is closed, update the closed issue numbers
if issue.IsClosed {
if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil {
return db.WithTx2(ctx, func(ctx context.Context) ([]string, error) {
// update the total issue numbers
if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil {
return nil, err
}
}
// if the issue is closed, update the closed issue numbers
if issue.IsClosed {
if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil {
return nil, err
}
}
if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil {
return nil, fmt.Errorf("error updating counters for milestone id %d: %w",
issue.MilestoneID, err)
}
if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil {
return nil, fmt.Errorf("error updating counters for milestone id %d: %w",
issue.MilestoneID, err)
}
if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil {
return nil, err
}
if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil {
return nil, err
}
// find attachments related to this issue and remove them
if err := issue.LoadAttachments(ctx); err != nil {
return nil, err
}
// find attachments related to this issue and remove them
if err := issue.LoadAttachments(ctx); err != nil {
return nil, err
}
var attachmentPaths []string
for i := range issue.Attachments {
attachmentPaths = append(attachmentPaths, issue.Attachments[i].RelativePath())
}
var attachmentPaths []string
for i := range issue.Attachments {
attachmentPaths = append(attachmentPaths, issue.Attachments[i].RelativePath())
}
// delete all database data still assigned to this issue
if err := db.DeleteBeans(ctx,
&issues_model.ContentHistory{IssueID: issue.ID},
&issues_model.Comment{IssueID: issue.ID},
&issues_model.IssueLabel{IssueID: issue.ID},
&issues_model.IssueDependency{IssueID: issue.ID},
&issues_model.IssueAssignees{IssueID: issue.ID},
&issues_model.IssueUser{IssueID: issue.ID},
&activities_model.Notification{IssueID: issue.ID},
&issues_model.Reaction{IssueID: issue.ID},
&issues_model.IssueWatch{IssueID: issue.ID},
&issues_model.Stopwatch{IssueID: issue.ID},
&issues_model.TrackedTime{IssueID: issue.ID},
&project_model.ProjectIssue{IssueID: issue.ID},
&repo_model.Attachment{IssueID: issue.ID},
&issues_model.PullRequest{IssueID: issue.ID},
&issues_model.Comment{RefIssueID: issue.ID},
&issues_model.IssueDependency{DependencyID: issue.ID},
&issues_model.Comment{DependentIssueID: issue.ID},
&issues_model.IssuePin{IssueID: issue.ID},
); err != nil {
return nil, err
}
// deference all review comments
if err := issue.LoadRepo(ctx); err != nil {
return nil, err
}
if err := issue.LoadPullRequest(ctx); err != nil {
return nil, err
}
if err := committer.Commit(); err != nil {
return nil, err
}
return attachmentPaths, nil
if err := issue.LoadComments(ctx); err != nil {
return nil, err
}
// delete all database data still assigned to this issue
if err := db.DeleteBeans(ctx,
&issues_model.ContentHistory{IssueID: issue.ID},
&issues_model.IssueLabel{IssueID: issue.ID},
&issues_model.IssueDependency{IssueID: issue.ID},
&issues_model.IssueAssignees{IssueID: issue.ID},
&issues_model.IssueUser{IssueID: issue.ID},
&activities_model.Notification{IssueID: issue.ID},
&issues_model.Reaction{IssueID: issue.ID},
&issues_model.IssueWatch{IssueID: issue.ID},
&issues_model.Stopwatch{IssueID: issue.ID},
&issues_model.TrackedTime{IssueID: issue.ID},
&project_model.ProjectIssue{IssueID: issue.ID},
&repo_model.Attachment{IssueID: issue.ID},
&issues_model.PullRequest{IssueID: issue.ID},
&issues_model.Comment{RefIssueID: issue.ID},
&issues_model.IssueDependency{DependencyID: issue.ID},
&issues_model.Comment{DependentIssueID: issue.ID},
&issues_model.IssuePin{IssueID: issue.ID},
); err != nil {
return nil, err
}
for _, comment := range issue.Comments {
if err := issues_model.DeleteComment(ctx, comment); err != nil {
return nil, err
}
}
// delete all pull request records
if issue.IsPull {
// Delete scheduled auto merges
if _, err := db.GetEngine(ctx).Where("pull_id=?", issue.PullRequest.ID).
Delete(&pull_model.AutoMerge{}); err != nil {
return nil, err
}
// Delete review states
if _, err := db.GetEngine(ctx).Where("pull_id=?", issue.PullRequest.ID).
Delete(&pull_model.ReviewState{}); err != nil {
return nil, err
}
if _, err := db.GetEngine(ctx).ID(issue.PullRequest.ID).Delete(&issues_model.PullRequest{}); err != nil {
return nil, err
}
}
if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil {
return nil, err
}
return attachmentPaths, nil
})
}
// DeleteOrphanedIssues delete issues without a repo
@ -381,6 +426,17 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []
return nil, fmt.Errorf("deleteIssue [issue_id: %d]: %w", issue.ID, err)
}
for _, comment := range issue.Comments {
if comment.Type == issues_model.CommentTypeCode {
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "before")); err != nil {
log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "before"), issue.PullRequest.ID, err)
}
if err := git.RemoveRef(ctx, issue.Repo.RepoPath(), issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "after")); err != nil {
log.Error("Unable to remove ref %s in base repository for PR[%d] Error: %v", issues_model.GetCodeCommentRefName(issue.PullRequest.Index, comment.ID, "after"), issue.PullRequest.ID, err)
}
}
}
attachmentPaths = append(attachmentPaths, issueAttachPaths...)
}
}

5
services/issue/issue_test.go
View File

@ -39,10 +39,7 @@ func TestIssue_DeleteIssue(t *testing.T) {
assert.NoError(t, err)
assert.Len(t, issueIDs, 5)
issue := &issues_model.Issue{
RepoID: 1,
ID: issueIDs[2],
}
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: issueIDs[2]})
_, err = deleteIssue(db.DefaultContext, issue)
assert.NoError(t, err)

26
services/issue/pull.go
View File

@ -7,33 +7,15 @@ import (
"context"
"fmt"
"slices"
"time"
issues_model "code.gitea.io/gitea/models/issues"
org_model "code.gitea.io/gitea/models/organization"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
)
func getMergeBase(repo *git.Repository, pr *issues_model.PullRequest, baseBranch, headBranch string) (string, error) {
// Add a temporary remote
tmpRemote := fmt.Sprintf("mergebase-%d-%d", pr.ID, time.Now().UnixNano())
if err := repo.AddRemote(tmpRemote, repo.Path, false); err != nil {
return "", fmt.Errorf("AddRemote: %w", err)
}
defer func() {
if err := repo.RemoveRemote(tmpRemote); err != nil {
log.Error("getMergeBase: RemoveRemote: %v", err)
}
}()
mergeBase, _, err := repo.GetMergeBase(tmpRemote, baseBranch, headBranch)
return mergeBase, err
}
type ReviewRequestNotifier struct {
Comment *issues_model.Comment
IsAdd bool
@ -96,15 +78,9 @@ func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullReque
return nil, nil
}
// get the mergebase
mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName())
if err != nil {
return nil, err
}
// https://github.com/go-gitea/gitea/issues/29763, we need to get the files changed
// between the merge base and the head commit but not the base branch and the head commit
changedFiles, err := repo.GetFilesChangedBetween(mergeBase, pr.GetGitHeadRefName())
changedFiles, err := repo.GetFilesChangedBetween(pr.MergeBase, pr.GetGitHeadRefName())
if err != nil {
return nil, err
}

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)

19
services/migrations/gitea_uploader.go
View File

@ -927,15 +927,16 @@ func (g *GiteaLocalUploader) CreateReviews(ctx context.Context, reviews ...*base
}
c := issues_model.Comment{
Type: issues_model.CommentTypeCode,
IssueID: issue.ID,
Content: comment.Content,
Line: int64(line + comment.Position - 1),
TreePath: comment.TreePath,
CommitSHA: comment.CommitID,
Patch: patch,
CreatedUnix: timeutil.TimeStamp(comment.CreatedAt.Unix()),
UpdatedUnix: timeutil.TimeStamp(comment.UpdatedAt.Unix()),
Type: issues_model.CommentTypeCode,
IssueID: issue.ID,
Content: comment.Content,
Line: int64(line + comment.Position - 1),
TreePath: comment.TreePath,
BeforeCommitID: pr.MergeBase,
CommitSHA: comment.CommitID,
Patch: patch,
CreatedUnix: timeutil.TimeStamp(comment.CreatedAt.Unix()),
UpdatedUnix: timeutil.TimeStamp(comment.UpdatedAt.Unix()),
}
if err := g.remapUser(ctx, review, &c); err != nil {

6
services/pull/merge.go
View File

@ -721,6 +721,12 @@ func SetMerged(ctx context.Context, pr *issues_model.PullRequest, mergedCommitID
return false, issues_model.ErrIssueAlreadyChanged
}
// update merge ref, this is necessary to ensure pr.MergedCommitID can be used to do diff operations even
// if the repository rebased/force-pushed and the pull request's merge commit is no longer in the history
if err := git.UpdateRef(ctx, pr.Issue.Repo.RepoPath(), pr.GetGitMergeRefName(), pr.MergedCommitID); err != nil {
return false, fmt.Errorf("UpdateRef: %w", err)
}
return true, nil
})
}

27
services/pull/merge_merge.go
View File

@ -4,6 +4,10 @@
package pull
import (
"context"
"strings"
issues_model "code.gitea.io/gitea/models/issues"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
@ -23,3 +27,26 @@ func doMergeStyleMerge(ctx *mergeContext, message string) error {
}
return nil
}
// CalcMergeBase calculates the merge base for a pull request.
func CalcMergeBase(ctx context.Context, pr *issues_model.PullRequest) (string, error) {
repoPath := pr.BaseRepo.RepoPath()
if pr.HasMerged {
mergeBase, _, err := git.NewCommand("merge-base").AddDashesAndList(pr.MergedCommitID+"^", pr.GetGitHeadRefName()).
RunStdString(ctx, &git.RunOpts{Dir: repoPath})
return strings.TrimSpace(mergeBase), err
}
mergeBase, _, err := git.NewCommand("merge-base").AddDashesAndList(pr.BaseBranch, pr.GetGitHeadRefName()).
RunStdString(ctx, &git.RunOpts{Dir: repoPath})
if err != nil {
var err2 error
mergeBase, _, err2 = git.NewCommand("rev-parse").AddDynamicArguments(git.BranchPrefix+pr.BaseBranch).
RunStdString(ctx, &git.RunOpts{Dir: repoPath})
if err2 != nil {
log.Error("Unable to get merge base for PR ID %d, Index %d in %s/%s. Error: %v & %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err, err2)
return "", err2
}
}
return strings.TrimSpace(mergeBase), nil
}

4
services/pull/pull.go
View File

@ -184,7 +184,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
return nil
}); err != nil {
// cleanup: this will only remove the reference, the real commit will be clean up when next GC
if err1 := baseGitRepo.RemoveReference(pr.GetGitHeadRefName()); err1 != nil {
if err1 := git.RemoveRef(ctx, baseGitRepo.Path, pr.GetGitHeadRefName()); err1 != nil {
log.Error("RemoveReference: %v", err1)
}
return err
@ -648,7 +648,7 @@ func UpdateRef(ctx context.Context, pr *issues_model.PullRequest) (err error) {
return err
}
_, _, err = git.NewCommand("update-ref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.HeadCommitID).RunStdString(ctx, &git.RunOpts{Dir: pr.BaseRepo.RepoPath()})
err = git.UpdateRef(ctx, pr.BaseRepo.RepoPath(), pr.GetGitHeadRefName(), pr.HeadCommitID)
if err != nil {
log.Error("Unable to update ref in base repository for PR[%d] Error: %v", pr.ID, err)
}

368
services/pull/review.go
View File

@ -10,18 +10,22 @@ import (
"fmt"
"io"
"regexp"
"slices"
"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 +95,49 @@ 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 err := issue.LoadPullRequest(ctx); err != nil {
return nil, fmt.Errorf("LoadPullRequest: %w", err)
}
headCommitID, err := gitRepo.GetRefCommitID(issue.PullRequest.GetGitHeadRefName())
if err != nil {
return nil, fmt.Errorf("GetRefCommitID[%s]: %w", issue.PullRequest.GetGitHeadRefName(), err)
}
prCommitIDs, err := git.CommitIDsBetween(ctx, gitRepo.Path, issue.PullRequest.MergeBase, headCommitID)
if err != nil {
return nil, fmt.Errorf("CommitIDsBetween[%s, %s]: %w", beforeCommitID, afterCommitID, err)
}
if beforeCommitID == "" || beforeCommitID == issue.PullRequest.MergeBase {
beforeCommitID = issue.PullRequest.MergeBase
} else if !slices.Contains(prCommitIDs, beforeCommitID) { // beforeCommitID must be one of the pull request commits
return nil, fmt.Errorf("beforeCommitID[%s] is not a valid pull request commit", beforeCommitID)
}
if afterCommitID == "" || afterCommitID == headCommitID {
afterCommitID = headCommitID
} else if !slices.Contains(prCommitIDs, afterCommitID) { // afterCommitID must be one of the pull request commits
return nil, fmt.Errorf("afterCommitID[%s] is not a valid pull request commit", afterCommitID)
}
// CreateCodeComment() is used for:
// - Single comments
// - Comments that are part of a review
@ -119,7 +160,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 +195,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 +204,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 +220,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 +230,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,96 +247,59 @@ 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)
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)
}
return db.WithTx2(ctx, func(ctx context.Context) (*issues_model.Comment, error) {
comment, err := issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{
Type: issues_model.CommentTypeCode,
Doer: doer,
Repo: repo,
Issue: issue,
Content: content,
LineNum: line,
TreePath: treePath,
BeforeCommitID: beforeCommitID,
CommitSHA: afterCommitID,
ReviewID: reviewID,
Patch: patch,
Invalidated: false,
Attachments: attachments,
})
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: line,
TreePath: treePath,
CommitSHA: commitID,
ReviewID: reviewID,
Patch: patch,
Invalidated: invalidated,
Attachments: attachments,
// The line commit ID Must be referenced in the git repository, because the branch maybe rebased or force-pushed.
// If the review commit is GC, the position can not be calculated dynamically.
if err := git.UpdateRef(ctx, pr.BaseRepo.RepoPath(), issues_model.GetCodeCommentRefName(pr.Index, comment.ID, "before"), beforeCommitID); err != nil {
log.Error("Unable to update ref before_commitid in base repository for PR[%d] Error: %v", pr.ID, err)
return nil, err
}
if err := git.UpdateRef(ctx, pr.BaseRepo.RepoPath(), issues_model.GetCodeCommentRefName(pr.Index, comment.ID, "after"), afterCommitID); err != nil {
log.Error("Unable to update ref after_commitid in base repository for PR[%d] Error: %v", pr.ID, err)
return nil, err
}
return comment, nil
})
}
@ -328,15 +345,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 +486,180 @@ 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.
// left side is the commit the comment was created on, right side is the commit the comment is displayed on.
// 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 absLine < hunk.LeftLine {
// The line is before the hunk, so we can ignore it
continue
} else if hunk.LeftLine <= absLine && absLine < hunk.LeftLine+hunk.LeftHunk {
// The line is within the hunk, that means the line is deleted from the current commit
// So that we don't need to display this line
return 0
}
// The line is after the hunk, so we can add the right hunk size
newLine += hunk.RightHunk - hunk.LeftHunk
}
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, gitRepo *git.Repository, repo *repo_model.Repository, issueID int64, currentUser *user_model.User, beforeCommitID, afterCommitID, 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
}
afterCommit, err := gitRepo.GetCommit(afterCommitID)
if err != nil {
return nil, fmt.Errorf("GetCommit[%s]: %w", afterCommitID, err)
}
n := 0
hunksCache := make(map[string][]*git.HunkInfo)
for _, comment := range comments {
// Code comment should always have a commit SHA, if not, we need to set it based on the line number
if comment.BeforeCommitID == "" {
comment.BeforeCommitID = beforeCommitID
}
if comment.CommitSHA == "" {
comment.CommitSHA = afterCommitID
}
dstCommitID := beforeCommitID
commentCommitID := comment.BeforeCommitID
if comment.Line > 0 {
dstCommitID = afterCommitID
commentCommitID = comment.CommitSHA
}
if commentCommitID != dstCommitID {
// If the comment is not for the current commit, we need to recalculate the line number
hunks, ok := hunksCache[commentCommitID+".."+dstCommitID]
if !ok {
hunks, err = git.GetAffectedHunksForTwoCommitsSpecialFile(ctx, repo.RepoPath(), commentCommitID, dstCommitID, treePath)
if err != nil {
return nil, fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s]: %w", repo.FullName(), commentCommitID, dstCommitID, err)
}
hunksCache[commentCommitID+".."+dstCommitID] = hunks
}
comment.Line = ReCalculateLineNumber(hunks, comment.Line)
}
if comment.Line == line {
commentAfterCommit, err := gitRepo.GetCommit(comment.CommitSHA)
if err != nil {
return nil, fmt.Errorf("GetCommit[%s]: %w", comment.CommitSHA, err)
}
// If the comment is not the first one or the comment created before the current commit
if n > 0 || comment.CommitSHA == afterCommitID ||
commentAfterCommit.Committer.When.Before(afterCommit.Committer.When) {
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,
beforeCommit, afterCommit *git.Commit, showOutdatedComments bool,
) error {
if beforeCommit == nil || afterCommit == 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 {
if comment.BeforeCommitID == "" {
comment.BeforeCommitID = beforeCommit.ID.String()
}
if comment.CommitSHA == "" {
comment.CommitSHA = afterCommit.ID.String()
}
dstCommitID := beforeCommit.ID.String()
commentCommitID := comment.BeforeCommitID
if comment.Line > 0 {
dstCommitID = afterCommit.ID.String()
commentCommitID = comment.CommitSHA
}
if commentCommitID != dstCommitID {
// If the comment is not for the current commit, we need to recalculate the line number
hunks, ok := hunksCache[commentCommitID+".."+dstCommitID]
if !ok {
hunks, err = git.GetAffectedHunksForTwoCommitsSpecialFile(ctx, repo.RepoPath(), commentCommitID, dstCommitID, file.Name)
if err != nil {
return fmt.Errorf("GetAffectedHunksForTwoCommitsSpecialFile[%s, %s, %s]: %w", repo.FullName(), commentCommitID, dstCommitID, err)
}
hunksCache[commentCommitID+".."+dstCommitID] = hunks
}
comment.Line = ReCalculateLineNumber(hunks, comment.Line)
}
if comment.Line != 0 {
commentAfterCommit, err := gitRepo.GetCommit(comment.CommitSHA)
if err != nil {
return fmt.Errorf("GetCommit[%s]: %w", comment.CommitSHA, err)
}
// If the comment is not the first one or the comment created before the current commit
if lineComments[comment.Line] != nil || comment.CommitSHA == afterCommit.ID.String() ||
commentAfterCommit.Committer.When.Before(afterCommit.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()
beforeCommit, err := gitRepo.GetCommit(issue.PullRequest.MergeBase)
assert.NoError(t, err)
afterCommit, 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, beforeCommit, afterCommit, 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))
}

14
services/repository/delete.go
View File

@ -97,10 +97,6 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams
}
needRewriteKeysFile := deleted > 0
if err := deleteDBRepository(ctx, repoID); err != nil {
return err
}
if org != nil && org.IsOrganization() {
teams, err := organization.FindOrgTeams(ctx, org.ID)
if err != nil {
@ -187,11 +183,6 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams
return err
}
// Delete Pulls and related objects
if err := issues_model.DeletePullsByBaseRepoID(ctx, repoID); err != nil {
return err
}
// Delete Issues and related objects
var attachmentPaths []string
if attachmentPaths, err = issue_service.DeleteIssuesByRepoID(ctx, repoID); err != nil {
@ -291,6 +282,11 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams
return err
}
// delete all related database records first before deleting the repository record
if err := deleteDBRepository(ctx, repoID); err != nil {
return err
}
if err = committer.Commit(); err != nil {
return err
}

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

@ -35,7 +35,7 @@
{{template "repo/diff/whitespace_dropdown" .}}
{{template "repo/diff/options_dropdown" .}}
{{if .PageIsPullFiles}}
<div id="diff-commit-select" data-issuelink="{{$.Issue.Link}}" data-queryparams="?style={{if $.IsSplitStyle}}split{{else}}unified{{end}}&whitespace={{$.WhitespaceBehavior}}&show-outdated={{$.ShowOutdatedComments}}" data-filter_changes_by_commit="{{ctx.Locale.Tr "repo.pulls.filter_changes_by_commit"}}">
<div id="diff-commit-select" data-merge-base="{{.MergeBase}}" data-issuelink="{{$.Issue.Link}}" data-queryparams="?style={{if $.IsSplitStyle}}split{{else}}unified{{end}}&whitespace={{$.WhitespaceBehavior}}&show-outdated={{$.ShowOutdatedComments}}" data-filter_changes_by_commit="{{ctx.Locale.Tr "repo.pulls.filter_changes_by_commit"}}">
{{/* the following will be replaced by vue component, but this avoids any loading artifacts till the vue component is initialized */}}
<div class="ui jump dropdown tiny basic button custom">
{{svg "octicon-git-commit"}}
@ -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}}

4
tests/integration/pull_diff_test.go
View File

@ -25,10 +25,6 @@ func TestPullDiff_CommitRangePRDiff(t *testing.T) {
doTestPRDiff(t, "/user2/commitsonpr/pulls/1/files/4ca8bcaf27e28504df7bf996819665986b01c847..23576dd018294e476c06e569b6b0f170d0558705", true, []string{"test2.txt", "test3.txt", "test4.txt"})
}
func TestPullDiff_StartingFromBaseToCommitPRDiff(t *testing.T) {
doTestPRDiff(t, "/user2/commitsonpr/pulls/1/files/c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2", true, []string{"test1.txt", "test2.txt", "test3.txt"})
}
func doTestPRDiff(t *testing.T, prDiffURL string, reviewBtnDisabled bool, expectedFilenames []string) {
defer tests.PrepareTestEnv(t)()

242
tests/integration/pull_review_test.go
View File

@ -4,6 +4,7 @@
package integration
import (
"fmt"
"net/http"
"net/http/httptest"
"net/url"
@ -17,6 +18,7 @@ import (
"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/modules/test"
issue_service "code.gitea.io/gitea/services/issue"
repo_service "code.gitea.io/gitea/services/repository"
@ -276,3 +278,243 @@ func testIssueClose(t *testing.T, session *TestSession, owner, repo, issueNumber
req = NewRequestWithValues(t, "POST", closeURL, options)
return session.MakeRequest(t, req, http.StatusOK)
}
func Test_ReviewCodeComment(t *testing.T) {
onGiteaRun(t, func(t *testing.T, u *url.URL) {
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
// Create the repo.
repo, err := repo_service.CreateRepositoryDirectly(db.DefaultContext, user2, user2, repo_service.CreateRepoOptions{
Name: "test_codecomment",
Readme: "Default",
AutoInit: true,
ObjectFormatName: git.Sha1ObjectFormat.Name(),
DefaultBranch: "master",
}, true)
assert.NoError(t, err)
// add README.md to default branch
_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
OldBranch: repo.DefaultBranch,
Files: []*files_service.ChangeRepoFile{
{
Operation: "update",
TreePath: "README.md",
ContentReader: strings.NewReader("# 111\n# 222\n"),
},
},
})
assert.NoError(t, err)
var pr *issues_model.PullRequest
t.Run("Create Pull Request", func(t *testing.T) {
// create a new branch to prepare for pull request
_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
NewBranch: "branch_codecomment1",
Files: []*files_service.ChangeRepoFile{
{
Operation: "update",
TreePath: "README.md",
// add 5 lines to the file
ContentReader: strings.NewReader("# 111\n# 222\n# 333\n# 444\n# 555\n# 666\n# 777\n"),
},
},
})
assert.NoError(t, err)
// Create a pull request.
session := loginUser(t, "user2")
testPullCreate(t, session, "user2", "test_codecomment", false, repo.DefaultBranch, "branch_codecomment1", "Test Pull Request1")
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "branch_codecomment1"})
})
t.Run("Create Code Comment", func(t *testing.T) {
session := loginUser(t, "user2")
// Grab the CSRF token.
req := NewRequest(t, "GET", path.Join("user2", "test_codecomment", "pulls", "1"))
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
// Create a code comment on the pull request.
commentURL := fmt.Sprintf("/user2/test_codecomment/pulls/%d/files/reviews/comments", pr.Index)
options := map[string]string{
"_csrf": htmlDoc.GetCSRF(),
"origin": "diff",
"content": "code comment on right line 4",
"side": "proposed",
"line": "4",
"path": "README.md",
"single_review": "true",
"reply": "0",
"before_commit_id": "",
"after_commit_id": "",
}
req = NewRequestWithValues(t, "POST", commentURL, options)
session.MakeRequest(t, req, http.StatusOK)
// Check if the comment was created.
comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{
Type: issues_model.CommentTypeCode,
IssueID: pr.IssueID,
})
assert.Equal(t, "code comment on right line 4", comment.Content)
assert.Equal(t, "README.md", comment.TreePath)
assert.Equal(t, int64(4), comment.Line)
gitRepo, err := gitrepo.OpenRepository(t.Context(), repo)
assert.NoError(t, err)
defer gitRepo.Close()
commitID, err := gitRepo.GetRefCommitID(pr.GetGitHeadRefName())
assert.NoError(t, err)
assert.Equal(t, commitID, comment.CommitSHA)
// load the files page and confirm the comment is there
filesPageURL := fmt.Sprintf("/user2/test_codecomment/pulls/%d/files", pr.Index)
req = NewRequest(t, "GET", filesPageURL)
resp = session.MakeRequest(t, req, http.StatusOK)
htmlDoc = NewHTMLParser(t, resp.Body)
commentHTML := htmlDoc.Find(fmt.Sprintf("#issuecomment-%d", comment.ID))
assert.NotNil(t, commentHTML)
assert.Equal(t, "code comment on right line 4", strings.TrimSpace(commentHTML.Find(".comment-body .render-content").Text()))
// the last line of this comment line number is 4
parentTr := commentHTML.ParentsFiltered("tr").First()
assert.NotNil(t, parentTr)
previousTr := parentTr.PrevAllFiltered("tr").First()
val, _ := previousTr.Attr("data-line-type")
assert.Equal(t, "add", val)
td := previousTr.Find("td.lines-num-new")
val, _ = td.Attr("data-line-num")
assert.Equal(t, "4", val)
})
t.Run("Pushing new commit to the pull request to add lines", func(t *testing.T) {
// create a new branch to prepare for pull request
_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
OldBranch: "branch_codecomment1",
Files: []*files_service.ChangeRepoFile{
{
Operation: "update",
TreePath: "README.md",
// add 1 line before the code comment line 4
ContentReader: strings.NewReader("# 111\n# 222\n# 333\n# 334\n# 444\n# 555\n# 666\n# 777\n"),
},
},
})
assert.NoError(t, err)
session := loginUser(t, "user2")
comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{
Type: issues_model.CommentTypeCode,
IssueID: pr.IssueID,
})
// load the files page and confirm the comment's line number is dynamically adjusted
filesPageURL := fmt.Sprintf("/user2/test_codecomment/pulls/%d/files", pr.Index)
req := NewRequest(t, "GET", filesPageURL)
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
commentHTML := htmlDoc.Find(fmt.Sprintf("#issuecomment-%d", comment.ID))
assert.NotNil(t, commentHTML)
assert.Equal(t, "code comment on right line 4", strings.TrimSpace(commentHTML.Find(".comment-body .render-content").Text()))
// the last line of this comment line number is 4
parentTr := commentHTML.ParentsFiltered("tr").First()
assert.NotNil(t, parentTr)
previousTr := parentTr.PrevAllFiltered("tr").First()
val, _ := previousTr.Attr("data-line-type")
assert.Equal(t, "add", val)
td := previousTr.Find("td.lines-num-new")
val, _ = td.Attr("data-line-num")
assert.Equal(t, "5", val) // one line have inserted in this commit, so the line number should be 5 now
})
t.Run("Pushing new commit to the pull request to delete lines", func(t *testing.T) {
// create a new branch to prepare for pull request
_, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
OldBranch: "branch_codecomment1",
Files: []*files_service.ChangeRepoFile{
{
Operation: "update",
TreePath: "README.md",
// delete the second line before the code comment line 4
ContentReader: strings.NewReader("# 111\n# 333\n# 334\n# 444\n# 555\n# 666\n# 777\n"),
},
},
})
assert.NoError(t, err)
session := loginUser(t, "user2")
comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{
Type: issues_model.CommentTypeCode,
IssueID: pr.IssueID,
})
// load the files page and confirm the comment's line number is dynamically adjusted
filesPageURL := fmt.Sprintf("/user2/test_codecomment/pulls/%d/files", pr.Index)
req := NewRequest(t, "GET", filesPageURL)
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
commentHTML := htmlDoc.Find(fmt.Sprintf("#issuecomment-%d", comment.ID))
assert.NotNil(t, commentHTML)
assert.Equal(t, "code comment on right line 4", strings.TrimSpace(commentHTML.Find(".comment-body .render-content").Text()))
// the last line of this comment line number is 4
parentTr := commentHTML.ParentsFiltered("tr").First()
assert.NotNil(t, parentTr)
previousTr := parentTr.PrevAllFiltered("tr").First()
val, _ := previousTr.Attr("data-line-type")
assert.Equal(t, "add", val)
td := previousTr.Find("td.lines-num-new")
val, _ = td.Attr("data-line-num")
assert.Equal(t, "4", val) // one line have inserted and one line deleted before this line in this commit, so the line number should be 4 now
// add a new comment on the deleted line
commentURL := fmt.Sprintf("/user2/test_codecomment/pulls/%d/files/reviews/comments", pr.Index)
options := map[string]string{
"_csrf": htmlDoc.GetCSRF(),
"origin": "diff",
"content": "code comment on left line 2",
"side": "previous",
"line": "2",
"path": "README.md",
"single_review": "true",
"reply": "0",
"before_commit_id": "",
"after_commit_id": "",
}
req = NewRequestWithValues(t, "POST", commentURL, options)
session.MakeRequest(t, req, http.StatusOK)
// Check if the comment was created.
commentLast := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{
Type: issues_model.CommentTypeCode,
IssueID: pr.IssueID,
Content: "code comment on left line 2",
})
assert.Equal(t, "code comment on left line 2", commentLast.Content)
assert.Equal(t, "README.md", commentLast.TreePath)
assert.Equal(t, int64(-2), commentLast.Line)
assert.Equal(t, pr.MergeBase, commentLast.BeforeCommitID)
// load the files page and confirm the comment's line number is dynamically adjusted
filesPageURL = fmt.Sprintf("/user2/test_codecomment/pulls/%d/files", pr.Index)
req = NewRequest(t, "GET", filesPageURL)
resp = session.MakeRequest(t, req, http.StatusOK)
htmlDoc = NewHTMLParser(t, resp.Body)
commentHTML = htmlDoc.Find(fmt.Sprintf("#issuecomment-%d", commentLast.ID))
assert.NotNil(t, commentHTML)
assert.Equal(t, "code comment on left line 2", strings.TrimSpace(commentHTML.Find(".comment-body .render-content").Text()))
// the last line of this comment line number is 4
parentTr = commentHTML.ParentsFiltered("tr").First()
assert.NotNil(t, parentTr)
previousTr = parentTr.PrevAllFiltered("tr").First()
val, _ = previousTr.Attr("data-line-type")
assert.Equal(t, "del", val)
td = previousTr.Find("td.lines-num-old")
val, _ = td.Attr("data-line-num")
assert.Equal(t, "2", val)
})
})
}

29
web_src/js/components/DiffCommitSelector.vue
View File

@ -32,6 +32,7 @@ export default defineComponent({
locale: {
filter_changes_by_commit: el.getAttribute('data-filter_changes_by_commit'),
} as Record<string, string>,
merge_base: el.getAttribute('data-merge-base'),
commits: [] as Array<Commit>,
hoverActivated: false,
lastReviewCommitSha: '',
@ -179,9 +180,6 @@ export default defineComponent({
* When a commit is clicked with shift this enables the range
* selection. Second click (with shift) defines the end of the
* range. This opens the diff of this range
* Exception: first commit is the first commit of this PR. Then
* the diff from beginning of PR up to the second clicked commit is
* opened
*/
commitClickedShift(commit: Commit) {
this.hoverActivated = !this.hoverActivated;
@ -189,18 +187,21 @@ export default defineComponent({
// Second click -> determine our range and open links accordingly
if (!this.hoverActivated) {
// find all selected commits and generate a link
if (this.commits[0].selected) {
// first commit is selected - generate a short url with only target sha
const lastCommitIdx = this.commits.findLastIndex((x) => x.selected);
if (lastCommitIdx === this.commits.length - 1) {
// user selected all commits - just show the normal diff page
window.location.assign(`${this.issueLink}/files${this.queryParams}`);
} else {
window.location.assign(`${this.issueLink}/files/${this.commits[lastCommitIdx].id}${this.queryParams}`);
}
const firstSelected = this.commits.findIndex((x) => x.selected);
let start: string;
if (firstSelected === 0) {
start = this.merge_base;
} else {
start = this.commits[firstSelected - 1].id;
}
const end = this.commits.findLast((x) => x.selected).id;
if (start === end) {
// if the start and end are the same, we show this single commit
window.location.assign(`${this.issueLink}/commits/${start}${this.queryParams}`);
} else if (start === this.merge_base && end === this.commits.at(-1).id) {
// if the first commit is selected and the last commit is selected, we show all commits
window.location.assign(`${this.issueLink}/files${this.queryParams}`);
} else {
const start = this.commits[this.commits.findIndex((x) => x.selected) - 1].id;
const end = this.commits.findLast((x) => x.selected).id;
window.location.assign(`${this.issueLink}/files/${start}..${end}${this.queryParams}`);
}
}