From bcb7f352215be12368de937fc0faa91cdb3d639a Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Fri, 8 Jan 2021 22:49:55 +0100 Subject: [PATCH] Do not reload page after adding comments in Pull Request reviews (#13877) Fixed #8861 * use ajax on PR review page * handle review comments * extract duplicate code FetchCodeCommentsByLine was initially more or less copied from fetchCodeCommentsByReview. Now they both use a common findCodeComments function instead * use the Engine that was passed into the method Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: Lunny Xiao Co-authored-by: Lauris BH --- models/issue_comment.go | 65 +++++++---- modules/auth/repo_form.go | 1 + routers/repo/pull_review.go | 62 +++++++++++ routers/routes/macaron.go | 1 + templates/repo/diff/box.tmpl | 37 +++---- templates/repo/diff/comment_form.tmpl | 3 +- templates/repo/diff/conversation.tmpl | 4 +- templates/repo/diff/new_comment.tmpl | 6 +- templates/repo/diff/section_split.tmpl | 8 +- templates/repo/diff/section_unified.tmpl | 6 +- .../repo/issue/view_content/comments.tmpl | 2 +- web_src/js/index.js | 102 +++++++++++------- 12 files changed, 205 insertions(+), 92 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index a5cb91ac75e52..dd979edcda526 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -979,7 +979,7 @@ func (opts *FindCommentsOptions) toConds() builder.Cond { if opts.Type != CommentTypeUnknown { cond = cond.And(builder.Eq{"comment.type": opts.Type}) } - if opts.Line > 0 { + if opts.Line != 0 { cond = cond.And(builder.Eq{"comment.line": opts.Line}) } if len(opts.TreePath) > 0 { @@ -1078,18 +1078,35 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review if review == nil { review = &Review{ID: 0} } - //Find comments opts := FindCommentsOptions{ Type: CommentTypeCode, IssueID: issue.ID, ReviewID: review.ID, } + + comments, err := findCodeComments(e, opts, issue, currentUser, review) + 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) + } + return pathToLineToComment, nil +} + +func findCodeComments(e Engine, opts FindCommentsOptions, issue *Issue, currentUser *User, review *Review) ([]*Comment, error) { + var comments []*Comment + if review == nil { + review = &Review{ID: 0} + } conds := opts.toConds() if review.ID == 0 { conds = conds.And(builder.Eq{"invalidated": false}) } - - var comments []*Comment if err := e.Where(conds). Asc("comment.created_unix"). Asc("comment.id"). @@ -1117,7 +1134,19 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review return nil, err } + n := 0 for _, comment := range comments { + if re, ok := reviews[comment.ReviewID]; ok && re != nil { + // If the review is pending only the author can see the comments (except if the review is set) + if review.ID == 0 && re.Type == ReviewTypePending && + (currentUser == nil || currentUser.ID != re.ReviewerID) { + continue + } + comment.Review = re + } + comments[n] = comment + n++ + if err := comment.LoadResolveDoer(); err != nil { return nil, err } @@ -1126,25 +1155,21 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review return nil, err } - if re, ok := reviews[comment.ReviewID]; ok && re != nil { - // If the review is pending only the author can see the comments (except the review is set) - if review.ID == 0 { - if re.Type == ReviewTypePending && - (currentUser == nil || currentUser.ID != re.ReviewerID) { - continue - } - } - comment.Review = re - } - comment.RenderedContent = string(markdown.Render([]byte(comment.Content), issue.Repo.Link(), issue.Repo.ComposeMetas())) - if pathToLineToComment[comment.TreePath] == nil { - pathToLineToComment[comment.TreePath] = make(map[int64][]*Comment) - } - pathToLineToComment[comment.TreePath][comment.Line] = append(pathToLineToComment[comment.TreePath][comment.Line], comment) } - return pathToLineToComment, nil + return comments[:n], nil +} + +// FetchCodeCommentsByLine fetches the code comments for a given treePath and line number +func FetchCodeCommentsByLine(issue *Issue, currentUser *User, treePath string, line int64) ([]*Comment, error) { + opts := FindCommentsOptions{ + Type: CommentTypeCode, + IssueID: issue.ID, + TreePath: treePath, + Line: line, + } + return findCodeComments(x, opts, issue, currentUser, nil) } // FetchCodeComments will return a 2d-map: ["Path"]["Line"] = Comments at line diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 87f2a53510679..78b2197a2dda8 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -548,6 +548,7 @@ func (f *MergePullRequestForm) Validate(ctx *macaron.Context, errs binding.Error // CodeCommentForm form for adding code comments for PRs type CodeCommentForm struct { + Origin string `binding:"Required;In(timeline,diff)"` Content string `binding:"Required"` Side string `binding:"Required;In(previous,proposed)"` Line int64 diff --git a/routers/repo/pull_review.go b/routers/repo/pull_review.go index 730074b7f3f08..0bacc682324b3 100644 --- a/routers/repo/pull_review.go +++ b/routers/repo/pull_review.go @@ -9,11 +9,40 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/auth" + "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" pull_service "code.gitea.io/gitea/services/pull" ) +const ( + tplConversation base.TplName = "repo/diff/conversation" + tplNewComment base.TplName = "repo/diff/new_comment" +) + +// RenderNewCodeCommentForm will render the form for creating a new review comment +func RenderNewCodeCommentForm(ctx *context.Context) { + issue := GetActionIssue(ctx) + if !issue.IsPull { + return + } + currentReview, err := models.GetCurrentReview(ctx.User, issue) + if err != nil && !models.IsErrReviewNotExist(err) { + ctx.ServerError("GetCurrentReview", err) + return + } + ctx.Data["PageIsPullFiles"] = true + ctx.Data["Issue"] = issue + ctx.Data["CurrentReview"] = currentReview + pullHeadCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(issue.PullRequest.GetGitRefName()) + if err != nil { + ctx.ServerError("GetRefCommitID", err) + return + } + ctx.Data["AfterCommitID"] = pullHeadCommitID + ctx.HTML(200, tplNewComment) +} + // CreateCodeComment will create a code comment including an pending review if required func CreateCodeComment(ctx *context.Context, form auth.CodeCommentForm) { issue := GetActionIssue(ctx) @@ -58,11 +87,17 @@ func CreateCodeComment(ctx *context.Context, form auth.CodeCommentForm) { } log.Trace("Comment created: %-v #%d[%d] Comment[%d]", ctx.Repo.Repository, issue.Index, issue.ID, comment.ID) + + if form.Origin == "diff" { + renderConversation(ctx, comment) + return + } ctx.Redirect(comment.HTMLURL()) } // UpdateResolveConversation add or remove an Conversation resolved mark func UpdateResolveConversation(ctx *context.Context) { + origin := ctx.Query("origin") action := ctx.Query("action") commentID := ctx.QueryInt64("comment_id") @@ -103,11 +138,38 @@ func UpdateResolveConversation(ctx *context.Context) { return } + if origin == "diff" { + renderConversation(ctx, comment) + return + } ctx.JSON(200, map[string]interface{}{ "ok": true, }) } +func renderConversation(ctx *context.Context, comment *models.Comment) { + comments, err := models.FetchCodeCommentsByLine(comment.Issue, ctx.User, comment.TreePath, comment.Line) + if err != nil { + ctx.ServerError("FetchCodeCommentsByLine", err) + return + } + ctx.Data["PageIsPullFiles"] = true + ctx.Data["comments"] = comments + ctx.Data["CanMarkConversation"] = true + ctx.Data["Issue"] = comment.Issue + if err = comment.Issue.LoadPullRequest(); err != nil { + ctx.ServerError("comment.Issue.LoadPullRequest", err) + return + } + pullHeadCommitID, err := ctx.Repo.GitRepo.GetRefCommitID(comment.Issue.PullRequest.GetGitRefName()) + if err != nil { + ctx.ServerError("GetRefCommitID", err) + return + } + ctx.Data["AfterCommitID"] = pullHeadCommitID + ctx.HTML(200, tplConversation) +} + // SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) { issue := GetActionIssue(ctx) diff --git a/routers/routes/macaron.go b/routers/routes/macaron.go index ca3599b7a0a52..d54cd580d3cf9 100644 --- a/routers/routes/macaron.go +++ b/routers/routes/macaron.go @@ -856,6 +856,7 @@ func RegisterMacaronRoutes(m *macaron.Macaron) { m.Group("/files", func() { m.Get("", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.ViewPullFiles) m.Group("/reviews", func() { + m.Get("/new_comment", repo.RenderNewCodeCommentForm) m.Post("/comments", bindIgnErr(auth.CodeCommentForm{}), repo.CreateCodeComment) m.Post("/submit", bindIgnErr(auth.SubmitReviewForm{}), repo.SubmitReview) }, context.RepoMustNotBeArchived()) diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index ef152fa58b1f5..8533f9b910c37 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -144,28 +144,25 @@ {{end}} {{if not $.Repository.IsArchived}} -
- {{template "repo/diff/new_comment" dict "root" .}} +
+
+
+ {{$.i18n.Tr "write"}} + {{$.i18n.Tr "preview"}}
-
-
-
- {{$.i18n.Tr "write"}} - {{$.i18n.Tr "preview"}} -
-
- -
-
- {{$.i18n.Tr "loading"}} -
-
-
{{.i18n.Tr "repo.issues.cancel"}}
-
{{.i18n.Tr "repo.issues.save"}}
-
-
+
+ +
+
+ {{$.i18n.Tr "loading"}} +
+
+
{{.i18n.Tr "repo.issues.cancel"}}
+
{{.i18n.Tr "repo.issues.save"}}
- {{end}} +
+
+ {{end}} {{if .IsSplitStyle}}