Skip to content

Commit 0f5de85

Browse files
committed
refactor submit review
1 parent 16a4315 commit 0f5de85

File tree

3 files changed

+195
-181
lines changed

3 files changed

+195
-181
lines changed

routers/repo/pull_review.go

Lines changed: 17 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ import (
1111
"code.gitea.io/gitea/modules/auth"
1212
"code.gitea.io/gitea/modules/context"
1313
"code.gitea.io/gitea/modules/log"
14-
"code.gitea.io/gitea/modules/notification"
15-
comment_service "code.gitea.io/gitea/services/comments"
1614
pull_service "code.gitea.io/gitea/services/pull"
1715
)
1816

@@ -31,64 +29,33 @@ func CreateCodeComment(ctx *context.Context, form auth.CodeCommentForm) {
3129
ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
3230
return
3331
}
34-
var comment *models.Comment
35-
defer func() {
36-
if comment != nil {
37-
ctx.Redirect(comment.HTMLURL())
38-
} else {
39-
ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
40-
}
41-
}()
32+
4233
signedLine := form.Line
4334
if form.Side == "previous" {
4435
signedLine *= -1
4536
}
4637

47-
review := new(models.Review)
48-
if form.IsReview {
49-
var err error
50-
// Check if the user has already a pending review for this issue
51-
if review, err = models.GetCurrentReview(ctx.User, issue); err != nil {
52-
if !models.IsErrReviewNotExist(err) {
53-
ctx.ServerError("CreateCodeComment", err)
54-
return
55-
}
56-
// No pending review exists
57-
// Create a new pending review for this issue & user
58-
if review, err = pull_service.CreateReview(models.CreateReviewOptions{
59-
Type: models.ReviewTypePending,
60-
Reviewer: ctx.User,
61-
Issue: issue,
62-
}); err != nil {
63-
ctx.ServerError("CreateCodeComment", err)
64-
return
65-
}
66-
67-
}
68-
}
69-
if review.ID == 0 {
70-
review.ID = form.Reply
71-
}
72-
//FIXME check if line, commit and treepath exist
73-
comment, err := comment_service.CreateCodeComment(
38+
comment, err := pull_service.CreateCodeComment(
7439
ctx.User,
75-
issue.Repo,
7640
issue,
41+
signedLine,
7742
form.Content,
7843
form.TreePath,
79-
signedLine,
80-
review.ID,
44+
form.IsReview,
45+
form.Reply,
8146
)
8247
if err != nil {
8348
ctx.ServerError("CreateCodeComment", err)
8449
return
8550
}
86-
// Send no notification if comment is pending
87-
if !form.IsReview || form.Reply != 0 {
88-
notification.NotifyCreateIssueComment(ctx.User, issue.Repo, issue, comment)
89-
}
9051

9152
log.Trace("Comment created: %d/%d/%d", ctx.Repo.Repository.ID, issue.ID, comment.ID)
53+
54+
if comment != nil {
55+
ctx.Redirect(comment.HTMLURL())
56+
} else {
57+
ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
58+
}
9259
}
9360

9461
// SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist
@@ -105,23 +72,17 @@ func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) {
10572
ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
10673
return
10774
}
108-
var review *models.Review
109-
var err error
11075

11176
reviewType := form.ReviewType()
112-
11377
switch reviewType {
11478
case models.ReviewTypeUnknown:
11579
ctx.ServerError("GetCurrentReview", fmt.Errorf("unknown ReviewType: %s", form.Type))
11680
return
11781

11882
// can not approve/reject your own PR
11983
case models.ReviewTypeApprove, models.ReviewTypeReject:
120-
12184
if issue.Poster.ID == ctx.User.ID {
122-
12385
var translated string
124-
12586
if reviewType == models.ReviewTypeApprove {
12687
translated = ctx.Tr("repo.issues.review.self.approval")
12788
} else {
@@ -134,69 +95,17 @@ func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) {
13495
}
13596
}
13697

137-
review, err = models.GetCurrentReview(ctx.User, issue)
138-
if err == nil {
139-
review.Issue = issue
140-
if errl := review.LoadCodeComments(); errl != nil {
141-
ctx.ServerError("LoadCodeComments", err)
142-
return
143-
}
144-
}
145-
146-
if ((err == nil && len(review.CodeComments) == 0) ||
147-
(err != nil && models.IsErrReviewNotExist(err))) &&
148-
form.HasEmptyContent() {
149-
ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty"))
150-
ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
151-
return
152-
}
153-
98+
_, comm, err := pull_service.SubmitReview(ctx.User, issue, reviewType, form.Content)
15499
if err != nil {
155-
if !models.IsErrReviewNotExist(err) {
156-
ctx.ServerError("GetCurrentReview", err)
157-
return
158-
}
159-
// No current review. Create a new one!
160-
if review, err = pull_service.CreateReview(models.CreateReviewOptions{
161-
Type: reviewType,
162-
Issue: issue,
163-
Reviewer: ctx.User,
164-
Content: form.Content,
165-
}); err != nil {
166-
ctx.ServerError("CreateReview", err)
167-
return
168-
}
169-
} else {
170-
review.Content = form.Content
171-
review.Type = reviewType
172-
if err = pull_service.UpdateReview(review); err != nil {
173-
ctx.ServerError("UpdateReview", err)
174-
return
100+
if pull_service.IsContentEmptyErr(err) {
101+
ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty"))
102+
ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
103+
} else {
104+
ctx.ServerError("SubmitReview", err)
175105
}
176-
}
177-
comm, err := models.CreateComment(&models.CreateCommentOptions{
178-
Type: models.CommentTypeReview,
179-
Doer: ctx.User,
180-
Content: review.Content,
181-
Issue: issue,
182-
Repo: issue.Repo,
183-
ReviewID: review.ID,
184-
})
185-
if err != nil || comm == nil {
186-
ctx.ServerError("CreateComment", err)
187-
return
188-
}
189-
if err = review.Publish(); err != nil {
190-
ctx.ServerError("Publish", err)
191-
return
192-
}
193106

194-
pr, err := issue.GetPullRequest()
195-
if err != nil {
196-
ctx.ServerError("GetPullRequest", err)
197107
return
198108
}
199-
notification.NotifyPullRequestReview(pr, review, comm)
200109

201110
ctx.Redirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag()))
202111
}

services/comments/comments.go

Lines changed: 0 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,8 @@
55
package comments
66

77
import (
8-
"bytes"
9-
"fmt"
10-
"strings"
11-
128
"code.gitea.io/gitea/models"
13-
"code.gitea.io/gitea/modules/git"
149
"code.gitea.io/gitea/modules/notification"
15-
"code.gitea.io/gitea/modules/setting"
16-
"code.gitea.io/gitea/services/gitdiff"
1710
)
1811

1912
// CreateIssueComment creates a plain issue comment.
@@ -35,60 +28,6 @@ func CreateIssueComment(doer *models.User, repo *models.Repository, issue *model
3528
return comment, nil
3629
}
3730

38-
// CreateCodeComment creates a plain code comment at the specified line / path
39-
func CreateCodeComment(doer *models.User, repo *models.Repository, issue *models.Issue, content, treePath string, line, reviewID int64) (*models.Comment, error) {
40-
var commitID, patch string
41-
pr, err := models.GetPullRequestByIssueID(issue.ID)
42-
if err != nil {
43-
return nil, fmt.Errorf("GetPullRequestByIssueID: %v", err)
44-
}
45-
if err := pr.GetBaseRepo(); err != nil {
46-
return nil, fmt.Errorf("GetHeadRepo: %v", err)
47-
}
48-
gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
49-
if err != nil {
50-
return nil, fmt.Errorf("OpenRepository: %v", err)
51-
}
52-
defer gitRepo.Close()
53-
54-
// FIXME validate treePath
55-
// Get latest commit referencing the commented line
56-
// No need for get commit for base branch changes
57-
if line > 0 {
58-
commit, err := gitRepo.LineBlame(pr.GetGitRefName(), gitRepo.Path, treePath, uint(line))
59-
if err == nil {
60-
commitID = commit.ID.String()
61-
} else if !strings.Contains(err.Error(), "exit status 128 - fatal: no such path") {
62-
return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
63-
}
64-
}
65-
66-
// Only fetch diff if comment is review comment
67-
if reviewID != 0 {
68-
headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
69-
if err != nil {
70-
return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
71-
}
72-
patchBuf := new(bytes.Buffer)
73-
if err := gitdiff.GetRawDiffForFile(gitRepo.Path, pr.MergeBase, headCommitID, gitdiff.RawDiffNormal, treePath, patchBuf); err != nil {
74-
return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath)
75-
}
76-
patch = gitdiff.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
77-
}
78-
return models.CreateComment(&models.CreateCommentOptions{
79-
Type: models.CommentTypeCode,
80-
Doer: doer,
81-
Repo: repo,
82-
Issue: issue,
83-
Content: content,
84-
LineNum: line,
85-
TreePath: treePath,
86-
CommitSHA: commitID,
87-
ReviewID: reviewID,
88-
Patch: patch,
89-
})
90-
}
91-
9231
// UpdateComment updates information of comment.
9332
func UpdateComment(c *models.Comment, doer *models.User, oldContent string) error {
9433
if err := models.UpdateComment(c, doer); err != nil {

0 commit comments

Comments
 (0)