Refactor pull request review (#8954)

* refactor submit review

* remove unnecessary code

* remove unused comment

* fix lint

* remove duplicated actions

* remove duplicated actions

* fix typo

* fix comment content

models/issue_comment.go

@@ -539,8 +539,10 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
 		return nil, err
 	}
 
-	if err = sendCreateCommentAction(e, opts, comment); err != nil {
+	if !opts.NoAction {
-		return nil, err
+		if err = sendCreateCommentAction(e, opts, comment); err != nil {
+			return nil, err
+		}
 	}
 
 	if err = comment.addCrossReferences(e, opts.Doer); err != nil {
@@ -816,6 +818,7 @@ type CreateCommentOptions struct {
 	RefCommentID     int64
 	RefAction        references.XRefAction
 	RefIsPull        bool
+	NoAction         bool
 }
 
 // CreateComment creates comment of issue or commit.

models/repo_watch.go

@@ -216,6 +216,21 @@ func NotifyWatchers(act *Action) error {
 	return notifyWatchers(x, act)
 }
 
+// NotifyWatchersActions creates batch of actions for every watcher.
+func NotifyWatchersActions(acts []*Action) error {
+	sess := x.NewSession()
+	defer sess.Close()
+	if err := sess.Begin(); err != nil {
+		return err
+	}
+	for _, act := range acts {
+		if err := notifyWatchers(sess, act); err != nil {
+			return err
+		}
+	}
+	return sess.Commit()
+}
+
 func watchIfAuto(e Engine, userID, repoID int64, isWrite bool) error {
 	if !isWrite || !setting.Service.AutoWatchOnChanges {
 		return nil

models/review.go

@@ -5,14 +5,12 @@
 package models
 
 import (
-	"fmt"
+	"strings"
 
-	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/timeutil"
 
 	"xorm.io/builder"
 	"xorm.io/core"
-	"xorm.io/xorm"
 )
 
 // ReviewType defines the sort of feedback a review gives
@@ -86,6 +84,11 @@ func (r *Review) loadReviewer(e Engine) (err error) {
 	return
 }
 
+// LoadReviewer loads reviewer
+func (r *Review) LoadReviewer() error {
+	return r.loadReviewer(x)
+}
+
 func (r *Review) loadAttributes(e Engine) (err error) {
 	if err = r.loadReviewer(e); err != nil {
 		return
@@ -101,54 +104,6 @@ func (r *Review) LoadAttributes() error {
 	return r.loadAttributes(x)
 }
 
-// Publish will send notifications / actions to participants for all code comments; parts are concurrent
-func (r *Review) Publish() error {
-	return r.publish(x)
-}
-
-func (r *Review) publish(e *xorm.Engine) error {
-	if r.Type == ReviewTypePending || r.Type == ReviewTypeUnknown {
-		return fmt.Errorf("review cannot be published if type is pending or unknown")
-	}
-	if r.Issue == nil {
-		if err := r.loadIssue(e); err != nil {
-			return err
-		}
-	}
-	if err := r.Issue.loadRepo(e); err != nil {
-		return err
-	}
-	if len(r.CodeComments) == 0 {
-		if err := r.loadCodeComments(e); err != nil {
-			return err
-		}
-	}
-	for _, lines := range r.CodeComments {
-		for _, comments := range lines {
-			for _, comment := range comments {
-				go func(en *xorm.Engine, review *Review, comm *Comment) {
-					sess := en.NewSession()
-					defer sess.Close()
-					opts := &CreateCommentOptions{
-						Doer:    comm.Poster,
-						Issue:   review.Issue,
-						Repo:    review.Issue.Repo,
-						Type:    comm.Type,
-						Content: comm.Content,
-					}
-					if err := updateCommentInfos(sess, opts, comm); err != nil {
-						log.Warn("updateCommentInfos: %v", err)
-					}
-					if err := sendCreateCommentAction(sess, opts, comm); err != nil {
-						log.Warn("sendCreateCommentAction: %v", err)
-					}
-				}(e, r, comment)
-			}
-		}
-	}
-	return nil
-}
-
 func getReviewByID(e Engine, id int64) (*Review, error) {
 	review := new(Review)
 	if has, err := e.ID(id).Get(review); err != nil {
@@ -271,12 +226,79 @@ func GetCurrentReview(reviewer *User, issue *Issue) (*Review, error) {
 	return getCurrentReview(x, reviewer, issue)
 }
 
-// UpdateReview will update all cols of the given review in db
+// ContentEmptyErr represents an content empty error
-func UpdateReview(r *Review) error {
+type ContentEmptyErr struct {
-	if _, err := x.ID(r.ID).AllCols().Update(r); err != nil {
+}
-		return err
+
+func (ContentEmptyErr) Error() string {
+	return "Review content is empty"
+}
+
+// IsContentEmptyErr returns true if err is a ContentEmptyErr
+func IsContentEmptyErr(err error) bool {
+	_, ok := err.(ContentEmptyErr)
+	return ok
+}
+
+// SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist
+func SubmitReview(doer *User, issue *Issue, reviewType ReviewType, content string) (*Review, *Comment, error) {
+	sess := x.NewSession()
+	defer sess.Close()
+	if err := sess.Begin(); err != nil {
+		return nil, nil, err
+	}
+
+	review, err := getCurrentReview(sess, doer, issue)
+	if err != nil {
+		if !IsErrReviewNotExist(err) {
+			return nil, nil, err
+		}
+
+		if len(strings.TrimSpace(content)) == 0 {
+			return nil, nil, ContentEmptyErr{}
+		}
+
+		// No current review. Create a new one!
+		review, err = createReview(sess, CreateReviewOptions{
+			Type:     reviewType,
+			Issue:    issue,
+			Reviewer: doer,
+			Content:  content,
+		})
+		if err != nil {
+			return nil, nil, err
+		}
+	} else {
+		if err := review.loadCodeComments(sess); err != nil {
+			return nil, nil, err
+		}
+		if len(review.CodeComments) == 0 && len(strings.TrimSpace(content)) == 0 {
+			return nil, nil, ContentEmptyErr{}
+		}
+
+		review.Issue = issue
+		review.Content = content
+		review.Type = reviewType
+		if _, err := sess.ID(review.ID).Cols("content, type").Update(review); err != nil {
+			return nil, nil, err
+		}
 	}
-	return nil
+
+	comm, err := createComment(sess, &CreateCommentOptions{
+		Type:     CommentTypeReview,
+		Doer:     doer,
+		Content:  review.Content,
+		Issue:    issue,
+		Repo:     issue.Repo,
+		ReviewID: review.ID,
+		NoAction: true,
+	})
+	if err != nil || comm == nil {
+		return nil, nil, err
+	}
+
+	comm.Review = review
+	return review, comm, sess.Commit()
 }
 
 // PullReviewersWithType represents the type used to display a review overview

models/review_test.go

@@ -98,14 +98,6 @@ func TestCreateReview(t *testing.T) {
 	AssertExistsAndLoadBean(t, &Review{Content: "New Review"})
 }
 
-func TestUpdateReview(t *testing.T) {
-	assert.NoError(t, PrepareTestDatabase())
-	review := AssertExistsAndLoadBean(t, &Review{ID: 1}).(*Review)
-	review.Content = "Updated Review"
-	assert.NoError(t, UpdateReview(review))
-	AssertExistsAndLoadBean(t, &Review{ID: 1, Content: "Updated Review"})
-}
-
 func TestGetReviewersByPullID(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 

modules/notification/action/action.go

@@ -6,6 +6,7 @@ package action
 
 import (
 	"fmt"
+	"strings"
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/log"
@@ -117,3 +118,51 @@ func (a *actionNotifier) NotifyForkRepository(doer *models.User, oldRepo, repo *
 		log.Error("notify watchers '%d/%d': %v", doer.ID, repo.ID, err)
 	}
 }
+
+func (a *actionNotifier) NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment) {
+	if err := review.LoadReviewer(); err != nil {
+		log.Error("LoadReviewer '%d/%d': %v", review.ID, review.ReviewerID, err)
+		return
+	}
+	if err := review.LoadCodeComments(); err != nil {
+		log.Error("LoadCodeComments '%d/%d': %v", review.Reviewer.ID, review.ID, err)
+		return
+	}
+
+	var actions = make([]*models.Action, 0, 10)
+	for _, lines := range review.CodeComments {
+		for _, comments := range lines {
+			for _, comm := range comments {
+				actions = append(actions, &models.Action{
+					ActUserID: review.Reviewer.ID,
+					ActUser:   review.Reviewer,
+					Content:   fmt.Sprintf("%d|%s", review.Issue.Index, strings.Split(comm.Content, "\n")[0]),
+					OpType:    models.ActionCommentIssue,
+					RepoID:    review.Issue.RepoID,
+					Repo:      review.Issue.Repo,
+					IsPrivate: review.Issue.Repo.IsPrivate,
+					Comment:   comm,
+					CommentID: comm.ID,
+				})
+			}
+		}
+	}
+
+	if strings.TrimSpace(comment.Content) != "" {
+		actions = append(actions, &models.Action{
+			ActUserID: review.Reviewer.ID,
+			ActUser:   review.Reviewer,
+			Content:   fmt.Sprintf("%d|%s", review.Issue.Index, strings.Split(comment.Content, "\n")[0]),
+			OpType:    models.ActionCommentIssue,
+			RepoID:    review.Issue.RepoID,
+			Repo:      review.Issue.Repo,
+			IsPrivate: review.Issue.Repo.IsPrivate,
+			Comment:   comment,
+			CommentID: comment.ID,
+		})
+	}
+
+	if err := models.NotifyWatchersActions(actions); err != nil {
+		log.Error("notify watchers '%d/%d': %v", review.Reviewer.ID, review.Issue.RepoID, err)
+	}
+}

routers/repo/pull_review.go

@@ -11,8 +11,6 @@ import (
 	"code.gitea.io/gitea/modules/auth"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/log"
-	"code.gitea.io/gitea/modules/notification"
-	comment_service "code.gitea.io/gitea/services/comments"
 	pull_service "code.gitea.io/gitea/services/pull"
 )
 
@@ -31,64 +29,33 @@ func CreateCodeComment(ctx *context.Context, form auth.CodeCommentForm) {
 		ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
 		return
 	}
-	var comment *models.Comment
+
-	defer func() {
-		if comment != nil {
-			ctx.Redirect(comment.HTMLURL())
-		} else {
-			ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
-		}
-	}()
 	signedLine := form.Line
 	if form.Side == "previous" {
 		signedLine *= -1
 	}
 
-	review := new(models.Review)
+	comment, err := pull_service.CreateCodeComment(
-	if form.IsReview {
-		var err error
-		// Check if the user has already a pending review for this issue
-		if review, err = models.GetCurrentReview(ctx.User, issue); err != nil {
-			if !models.IsErrReviewNotExist(err) {
-				ctx.ServerError("CreateCodeComment", err)
-				return
-			}
-			// No pending review exists
-			// Create a new pending review for this issue & user
-			if review, err = pull_service.CreateReview(models.CreateReviewOptions{
-				Type:     models.ReviewTypePending,
-				Reviewer: ctx.User,
-				Issue:    issue,
-			}); err != nil {
-				ctx.ServerError("CreateCodeComment", err)
-				return
-			}
-
-		}
-	}
-	if review.ID == 0 {
-		review.ID = form.Reply
-	}
-	//FIXME check if line, commit and treepath exist
-	comment, err := comment_service.CreateCodeComment(
 		ctx.User,
-		issue.Repo,
 		issue,
+		signedLine,
 		form.Content,
 		form.TreePath,
-		signedLine,
+		form.IsReview,
-		review.ID,
+		form.Reply,
 	)
 	if err != nil {
 		ctx.ServerError("CreateCodeComment", err)
 		return
 	}
-	// Send no notification if comment is pending
-	if !form.IsReview || form.Reply != 0 {
-		notification.NotifyCreateIssueComment(ctx.User, issue.Repo, issue, comment)
-	}
 
 	log.Trace("Comment created: %d/%d/%d", ctx.Repo.Repository.ID, issue.ID, comment.ID)
+
+	if comment != nil {
+		ctx.Redirect(comment.HTMLURL())
+	} else {
+		ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
+	}
 }
 
 // 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) {
 		ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
 		return
 	}
-	var review *models.Review
-	var err error
 
 	reviewType := form.ReviewType()
-
 	switch reviewType {
 	case models.ReviewTypeUnknown:
-		ctx.ServerError("GetCurrentReview", fmt.Errorf("unknown ReviewType: %s", form.Type))
+		ctx.ServerError("ReviewType", fmt.Errorf("unknown ReviewType: %s", form.Type))
 		return
 
 	// can not approve/reject your own PR
 	case models.ReviewTypeApprove, models.ReviewTypeReject:
-
 		if issue.Poster.ID == ctx.User.ID {
-
 			var translated string
-
 			if reviewType == models.ReviewTypeApprove {
 				translated = ctx.Tr("repo.issues.review.self.approval")
 			} else {
@@ -134,69 +95,16 @@ func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) {
 		}
 	}
 
-	review, err = models.GetCurrentReview(ctx.User, issue)
+	_, comm, err := pull_service.SubmitReview(ctx.User, issue, reviewType, form.Content)
-	if err == nil {
-		review.Issue = issue
-		if errl := review.LoadCodeComments(); errl != nil {
-			ctx.ServerError("LoadCodeComments", err)
-			return
-		}
-	}
-
-	if ((err == nil && len(review.CodeComments) == 0) ||
-		(err != nil && models.IsErrReviewNotExist(err))) &&
-		form.HasEmptyContent() {
-		ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty"))
-		ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
-		return
-	}
-
 	if err != nil {
-		if !models.IsErrReviewNotExist(err) {
+		if models.IsContentEmptyErr(err) {
-			ctx.ServerError("GetCurrentReview", err)
+			ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty"))
-			return
+			ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index))
-		}
+		} else {
-		// No current review. Create a new one!
+			ctx.ServerError("SubmitReview", err)
-		if review, err = pull_service.CreateReview(models.CreateReviewOptions{
-			Type:     reviewType,
-			Issue:    issue,
-			Reviewer: ctx.User,
-			Content:  form.Content,
-		}); err != nil {
-			ctx.ServerError("CreateReview", err)
-			return
-		}
-	} else {
-		review.Content = form.Content
-		review.Type = reviewType
-		if err = pull_service.UpdateReview(review); err != nil {
-			ctx.ServerError("UpdateReview", err)
-			return
 		}
-	}
-	comm, err := models.CreateComment(&models.CreateCommentOptions{
-		Type:     models.CommentTypeReview,
-		Doer:     ctx.User,
-		Content:  review.Content,
-		Issue:    issue,
-		Repo:     issue.Repo,
-		ReviewID: review.ID,
-	})
-	if err != nil || comm == nil {
-		ctx.ServerError("CreateComment", err)
-		return
-	}
-	if err = review.Publish(); err != nil {
-		ctx.ServerError("Publish", err)
-		return
-	}
-
-	pr, err := issue.GetPullRequest()
-	if err != nil {
-		ctx.ServerError("GetPullRequest", err)
 		return
 	}
-	notification.NotifyPullRequestReview(pr, review, comm)
 
 	ctx.Redirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag()))
 }

services/comments/comments.go

@@ -5,15 +5,8 @@
 package comments
 
 import (
-	"bytes"
-	"fmt"
-	"strings"
-
 	"code.gitea.io/gitea/models"
-	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/notification"
-	"code.gitea.io/gitea/modules/setting"
-	"code.gitea.io/gitea/services/gitdiff"
 )
 
 // CreateIssueComment creates a plain issue comment.
@@ -35,60 +28,6 @@ func CreateIssueComment(doer *models.User, repo *models.Repository, issue *model
 	return comment, nil
 }
 
-// CreateCodeComment creates a plain code comment at the specified line / path
-func CreateCodeComment(doer *models.User, repo *models.Repository, issue *models.Issue, content, treePath string, line, reviewID int64) (*models.Comment, error) {
-	var commitID, patch string
-	pr, err := models.GetPullRequestByIssueID(issue.ID)
-	if err != nil {
-		return nil, fmt.Errorf("GetPullRequestByIssueID: %v", err)
-	}
-	if err := pr.GetBaseRepo(); err != nil {
-		return nil, fmt.Errorf("GetHeadRepo: %v", err)
-	}
-	gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
-	if err != nil {
-		return nil, fmt.Errorf("OpenRepository: %v", err)
-	}
-	defer gitRepo.Close()
-
-	// FIXME validate treePath
-	// Get latest commit referencing the commented line
-	// No need for get commit for base branch changes
-	if line > 0 {
-		commit, err := gitRepo.LineBlame(pr.GetGitRefName(), gitRepo.Path, treePath, uint(line))
-		if err == nil {
-			commitID = commit.ID.String()
-		} else if !strings.Contains(err.Error(), "exit status 128 - fatal: no such path") {
-			return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
-		}
-	}
-
-	// Only fetch diff if comment is review comment
-	if reviewID != 0 {
-		headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
-		if err != nil {
-			return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
-		}
-		patchBuf := new(bytes.Buffer)
-		if err := gitdiff.GetRawDiffForFile(gitRepo.Path, pr.MergeBase, headCommitID, gitdiff.RawDiffNormal, treePath, patchBuf); err != nil {
-			return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath)
-		}
-		patch = gitdiff.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
-	}
-	return models.CreateComment(&models.CreateCommentOptions{
-		Type:      models.CommentTypeCode,
-		Doer:      doer,
-		Repo:      repo,
-		Issue:     issue,
-		Content:   content,
-		LineNum:   line,
-		TreePath:  treePath,
-		CommitSHA: commitID,
-		ReviewID:  reviewID,
-		Patch:     patch,
-	})
-}
-
 // UpdateComment updates information of comment.
 func UpdateComment(c *models.Comment, doer *models.User, oldContent string) error {
 	if err := models.UpdateComment(c, doer); err != nil {

services/pull/review.go

@@ -6,32 +6,144 @@
 package pull
 
 import (
+	"bytes"
+	"fmt"
+	"strings"
+
 	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/notification"
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/services/gitdiff"
 )
 
-// CreateReview creates a new review based on opts
+// CreateCodeComment creates a comment on the code line
-func CreateReview(opts models.CreateReviewOptions) (*models.Review, error) {
+func CreateCodeComment(doer *models.User, issue *models.Issue, line int64, content string, treePath string, isReview bool, replyReviewID int64) (*models.Comment, error) {
-	review, err := models.CreateReview(opts)
+	// It's not a review, maybe a reply to a review comment or a single comment.
+	if !isReview {
+		if err := issue.LoadRepo(); err != nil {
+			return nil, err
+		}
+
+		comment, err := createCodeComment(
+			doer,
+			issue.Repo,
+			issue,
+			content,
+			treePath,
+			line,
+			replyReviewID,
+		)
+		if err != nil {
+			return nil, err
+		}
+
+		notification.NotifyCreateIssueComment(doer, issue.Repo, issue, comment)
+
+		return comment, nil
+	}
+
+	review, err := models.GetCurrentReview(doer, issue)
+	if err != nil {
+		if !models.IsErrReviewNotExist(err) {
+			return nil, err
+		}
+
+		review, err = models.CreateReview(models.CreateReviewOptions{
+			Type:     models.ReviewTypePending,
+			Reviewer: doer,
+			Issue:    issue,
+		})
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	comment, err := createCodeComment(
+		doer,
+		issue.Repo,
+		issue,
+		content,
+		treePath,
+		line,
+		review.ID,
+	)
 	if err != nil {
 		return nil, err
 	}
 
-	if opts.Type != models.ReviewTypePending {
+	// NOTICE: it's a pending review, so the notifications will not be fired until user submit review.
-		notification.NotifyPullRequestReview(review.Issue.PullRequest, review, nil)
+
+	return comment, nil
+}
+
+// createCodeComment creates a plain code comment at the specified line / path
+func createCodeComment(doer *models.User, repo *models.Repository, issue *models.Issue, content, treePath string, line, reviewID int64) (*models.Comment, error) {
+	var commitID, patch string
+	if err := issue.LoadPullRequest(); err != nil {
+		return nil, fmt.Errorf("GetPullRequestByIssueID: %v", err)
+	}
+	pr := issue.PullRequest
+	if err := pr.GetBaseRepo(); err != nil {
+		return nil, fmt.Errorf("GetHeadRepo: %v", err)
+	}
+	gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
+	if err != nil {
+		return nil, fmt.Errorf("OpenRepository: %v", err)
 	}
+	defer gitRepo.Close()
 
-	return review, nil
+	// FIXME validate treePath
+	// Get latest commit referencing the commented line
+	// No need for get commit for base branch changes
+	if line > 0 {
+		commit, err := gitRepo.LineBlame(pr.GetGitRefName(), gitRepo.Path, treePath, uint(line))
+		if err == nil {
+			commitID = commit.ID.String()
+		} else if !strings.Contains(err.Error(), "exit status 128 - fatal: no such path") {
+			return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err)
+		}
+	}
+
+	// Only fetch diff if comment is review comment
+	if reviewID != 0 {
+		headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
+		if err != nil {
+			return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
+		}
+		patchBuf := new(bytes.Buffer)
+		if err := gitdiff.GetRawDiffForFile(gitRepo.Path, pr.MergeBase, headCommitID, gitdiff.RawDiffNormal, treePath, patchBuf); err != nil {
+			return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath)
+		}
+		patch = gitdiff.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
+	}
+	return models.CreateComment(&models.CreateCommentOptions{
+		Type:      models.CommentTypeCode,
+		Doer:      doer,
+		Repo:      repo,
+		Issue:     issue,
+		Content:   content,
+		LineNum:   line,
+		TreePath:  treePath,
+		CommitSHA: commitID,
+		ReviewID:  reviewID,
+		Patch:     patch,
+		NoAction:  true,
+	})
 }
 
-// UpdateReview updates a review
+// SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist
-func UpdateReview(review *models.Review) error {
+func SubmitReview(doer *models.User, issue *models.Issue, reviewType models.ReviewType, content string) (*models.Review, *models.Comment, error) {
-	err := models.UpdateReview(review)
+	review, comm, err := models.SubmitReview(doer, issue, reviewType, content)
 	if err != nil {
-		return err
+		return nil, nil, err
 	}
 
-	notification.NotifyPullRequestReview(review.Issue.PullRequest, review, nil)
+	pr, err := issue.GetPullRequest()
+	if err != nil {
+		return nil, nil, err
+	}
+	notification.NotifyPullRequestReview(pr, review, comm)
 
-	return nil
+	return review, comm, nil
 }