Add resolve/unresolve review comment API endpoints (#36441)

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

models/issues/comment.go

@@ -1034,6 +1034,20 @@ func GetCommentByID(ctx context.Context, id int64) (*Comment, error) {
 	return c, nil
 }
 
+func GetCommentWithRepoID(ctx context.Context, repoID, commentID int64) (*Comment, error) {
+	c, err := GetCommentByID(ctx, commentID)
+	if err != nil {
+		return nil, err
+	}
+	if err := c.LoadIssue(ctx); err != nil {
+		return nil, err
+	}
+	if c.Issue.RepoID != repoID {
+		return nil, ErrCommentNotExist{commentID, 0}
+	}
+	return c, nil
+}
+
 // FindCommentsOptions describes the conditions to Find comments
 type FindCommentsOptions struct {
 	db.ListOptions

models/issues/comment_code.go

@@ -102,6 +102,7 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu
 				continue
 			}
 			comment.Review = re
+			comment.Issue = issue
 		}
 		comments[n] = comment
 		n++

modules/structs/pull_review.go

@@ -72,7 +72,7 @@ type PullReviewComment struct {
 	HTMLPullURL string `json:"pull_request_url"`
 }
 
-// CreatePullReviewOptions are options to create a pull review
+// CreatePullReviewOptions are options to create a pull request review
 type CreatePullReviewOptions struct {
 	Event    ReviewStateType           `json:"event"`
 	Body     string                    `json:"body"`
@@ -91,19 +91,19 @@ type CreatePullReviewComment struct {
 	NewLineNum int64 `json:"new_position"`
 }
 
-// SubmitPullReviewOptions are options to submit a pending pull review
+// SubmitPullReviewOptions are options to submit a pending pull request review
 type SubmitPullReviewOptions struct {
 	Event ReviewStateType `json:"event"`
 	Body  string          `json:"body"`
 }
 
-// DismissPullReviewOptions are options to dismiss a pull review
+// DismissPullReviewOptions are options to dismiss a pull request review
 type DismissPullReviewOptions struct {
 	Message string `json:"message"`
 	Priors  bool   `json:"priors"`
 }
 
-// PullReviewRequestOptions are options to add or remove pull review requests
+// PullReviewRequestOptions are options to add or remove pull request review requests
 type PullReviewRequestOptions struct {
 	Reviewers     []string `json:"reviewers"`
 	TeamReviewers []string `json:"team_reviewers"`

routers/api/v1/api.go

@@ -1348,6 +1348,8 @@ func Routes() *web.Router {
 					m.Combo("").Get(repo.ListPullRequests).
 						Post(reqToken(), mustNotBeArchived, bind(api.CreatePullRequestOption{}), repo.CreatePullRequest)
 					m.Get("/pinned", repo.ListPinnedPullRequests)
+					m.Post("/comments/{id}/resolve", reqToken(), mustNotBeArchived, repo.ResolvePullReviewComment)
+					m.Post("/comments/{id}/unresolve", reqToken(), mustNotBeArchived, repo.UnresolvePullReviewComment)
 					m.Group("/{index}", func() {
 						m.Combo("").Get(repo.GetPullRequest).
 							Patch(reqToken(), bind(api.EditPullRequestOption{}), repo.EditPullRequest)

routers/api/v1/repo/issue_comment.go

@@ -445,7 +445,7 @@ func GetIssueComment(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	comment, err := issues_model.GetCommentByID(ctx, ctx.PathParamInt64("id"))
+	comment, err := issues_model.GetCommentWithRepoID(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("id"))
 	if err != nil {
 		if issues_model.IsErrCommentNotExist(err) {
 			ctx.APIErrorNotFound(err)
@@ -455,15 +455,6 @@ func GetIssueComment(ctx *context.APIContext) {
 		return
 	}
 
-	if err = comment.LoadIssue(ctx); err != nil {
-		ctx.APIErrorInternal(err)
-		return
-	}
-	if comment.Issue.RepoID != ctx.Repo.Repository.ID {
-		ctx.Status(http.StatusNotFound)
-		return
-	}
-
 	if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
 		ctx.APIErrorNotFound()
 		return
@@ -579,7 +570,7 @@ func EditIssueCommentDeprecated(ctx *context.APIContext) {
 }
 
 func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption) {
-	comment, err := issues_model.GetCommentByID(ctx, ctx.PathParamInt64("id"))
+	comment, err := issues_model.GetCommentWithRepoID(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("id"))
 	if err != nil {
 		if issues_model.IsErrCommentNotExist(err) {
 			ctx.APIErrorNotFound(err)
@@ -589,16 +580,6 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption)
 		return
 	}
 
-	if err := comment.LoadIssue(ctx); err != nil {
-		ctx.APIErrorInternal(err)
-		return
-	}
-
-	if comment.Issue.RepoID != ctx.Repo.Repository.ID {
-		ctx.Status(http.StatusNotFound)
-		return
-	}
-
 	if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
 		ctx.Status(http.StatusForbidden)
 		return
@@ -698,7 +679,7 @@ func DeleteIssueCommentDeprecated(ctx *context.APIContext) {
 }
 
 func deleteIssueComment(ctx *context.APIContext) {
-	comment, err := issues_model.GetCommentByID(ctx, ctx.PathParamInt64("id"))
+	comment, err := issues_model.GetCommentWithRepoID(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("id"))
 	if err != nil {
 		if issues_model.IsErrCommentNotExist(err) {
 			ctx.APIErrorNotFound(err)
@@ -708,16 +689,6 @@ func deleteIssueComment(ctx *context.APIContext) {
 		return
 	}
 
-	if err := comment.LoadIssue(ctx); err != nil {
-		ctx.APIErrorInternal(err)
-		return
-	}
-
-	if comment.Issue.RepoID != ctx.Repo.Repository.ID {
-		ctx.Status(http.StatusNotFound)
-		return
-	}
-
 	if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
 		ctx.Status(http.StatusForbidden)
 		return

routers/api/v1/repo/pull_review.go

@@ -208,6 +208,126 @@ func GetPullReviewComments(ctx *context.APIContext) {
 	ctx.JSON(http.StatusOK, apiComments)
 }
 
+// ResolvePullReviewComment resolves a review comment in a pull request
+func ResolvePullReviewComment(ctx *context.APIContext) {
+	// swagger:operation POST /repos/{owner}/{repo}/pulls/comments/{id}/resolve repository repoResolvePullReviewComment
+	// ---
+	// summary: Resolve a pull request review comment
+	// produces:
+	// - application/json
+	// parameters:
+	// - name: owner
+	//   in: path
+	//   description: owner of the repo
+	//   type: string
+	//   required: true
+	// - name: repo
+	//   in: path
+	//   description: name of the repo
+	//   type: string
+	//   required: true
+	// - name: id
+	//   in: path
+	//   description: id of the review comment
+	//   type: integer
+	//   format: int64
+	//   required: true
+	// responses:
+	//   "204":
+	//     "$ref": "#/responses/empty"
+	//   "400":
+	//     "$ref": "#/responses/validationError"
+	//   "403":
+	//     "$ref": "#/responses/forbidden"
+	//   "404":
+	//     "$ref": "#/responses/notFound"
+	updatePullReviewCommentResolve(ctx, true)
+}
+
+// UnresolvePullReviewComment unresolves a review comment in a pull request
+func UnresolvePullReviewComment(ctx *context.APIContext) {
+	// swagger:operation POST /repos/{owner}/{repo}/pulls/comments/{id}/unresolve repository repoUnresolvePullReviewComment
+	// ---
+	// summary: Unresolve a pull request review comment
+	// produces:
+	// - application/json
+	// parameters:
+	// - name: owner
+	//   in: path
+	//   description: owner of the repo
+	//   type: string
+	//   required: true
+	// - name: repo
+	//   in: path
+	//   description: name of the repo
+	//   type: string
+	//   required: true
+	// - name: id
+	//   in: path
+	//   description: id of the review comment
+	//   type: integer
+	//   format: int64
+	//   required: true
+	// responses:
+	//   "204":
+	//     "$ref": "#/responses/empty"
+	//   "400":
+	//     "$ref": "#/responses/validationError"
+	//   "403":
+	//     "$ref": "#/responses/forbidden"
+	//   "404":
+	//     "$ref": "#/responses/notFound"
+	updatePullReviewCommentResolve(ctx, false)
+}
+
+func updatePullReviewCommentResolve(ctx *context.APIContext, isResolve bool) {
+	comment := getPullReviewCommentToResolve(ctx)
+	if comment == nil {
+		return
+	}
+
+	canMarkConv, err := issues_model.CanMarkConversation(ctx, comment.Issue, ctx.Doer)
+	if err != nil {
+		ctx.APIErrorInternal(err)
+		return
+	}
+	if !canMarkConv {
+		ctx.APIError(http.StatusForbidden, "user should have permission to resolve comment")
+		return
+	}
+
+	if err = issues_model.MarkConversation(ctx, comment, ctx.Doer, isResolve); err != nil {
+		ctx.APIErrorInternal(err)
+		return
+	}
+
+	ctx.Status(http.StatusNoContent)
+}
+
+func getPullReviewCommentToResolve(ctx *context.APIContext) *issues_model.Comment {
+	comment, err := issues_model.GetCommentWithRepoID(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("id"))
+	if err != nil {
+		if issues_model.IsErrCommentNotExist(err) {
+			ctx.APIErrorNotFound("GetCommentByID", err)
+		} else {
+			ctx.APIErrorInternal(err)
+		}
+		return nil
+	}
+
+	if !comment.Issue.IsPull {
+		ctx.APIError(http.StatusBadRequest, "comment does not belong to a pull request")
+		return nil
+	}
+
+	if comment.Type != issues_model.CommentTypeCode {
+		ctx.APIError(http.StatusBadRequest, "comment is not a review comment")
+		return nil
+	}
+
+	return comment
+}
+
 // DeletePullReview delete a specific review from a pull request
 func DeletePullReview(ctx *context.APIContext) {
 	// swagger:operation DELETE /repos/{owner}/{repo}/pulls/{index}/reviews/{id} repository repoDeletePullReview

services/convert/pull_review.go

@@ -92,12 +92,21 @@ func ToPullReviewCommentList(ctx context.Context, review *issues_model.Review, d
 	for _, lines := range review.CodeComments {
 		for _, comments := range lines {
 			for _, comment := range comments {
+				apiComments = append(apiComments, ToPullReviewComment(ctx, comment, doer))
+			}
+		}
+	}
+	return apiComments, nil
+}
+
+// ToPullReviewComment convert a single code review comment to api format
+func ToPullReviewComment(ctx context.Context, comment *issues_model.Comment, doer *user_model.User) *api.PullReviewComment {
 	apiComment := &api.PullReviewComment{
 		ID:           comment.ID,
 		Body:         comment.Content,
 		Poster:       ToUser(ctx, comment.Poster, doer),
 		Resolver:     ToUser(ctx, comment.ResolveDoer, doer),
-					ReviewID:     review.ID,
+		ReviewID:     comment.ReviewID,
 		Created:      comment.CreatedUnix.AsTime(),
 		Updated:      comment.UpdatedUnix.AsTime(),
 		Path:         comment.TreePath,
@@ -105,7 +114,7 @@ func ToPullReviewCommentList(ctx context.Context, review *issues_model.Review, d
 		OrigCommitID: comment.OldRef,
 		DiffHunk:     patch2diff(comment.Patch),
 		HTMLURL:      comment.HTMLURL(ctx),
-					HTMLPullURL:  review.Issue.HTMLURL(ctx),
+		HTMLPullURL:  comment.Issue.HTMLURL(ctx),
 	}
 
 	if comment.Line < 0 {
@@ -113,11 +122,8 @@ func ToPullReviewCommentList(ctx context.Context, review *issues_model.Review, d
 	} else {
 		apiComment.LineNum = comment.UnsignedLine()
 	}
-				apiComments = append(apiComments, apiComment)
+
-			}
+	return apiComment
-		}
-	}
-	return apiComments, nil
 }
 
 func patch2diff(patch string) string {

templates/swagger/v1_json.tmpl

@@ -13707,6 +13707,106 @@
         }
       }
     },
+    "/repos/{owner}/{repo}/pulls/comments/{id}/resolve": {
+      "post": {
+        "produces": [
+          "application/json"
+        ],
+        "tags": [
+          "repository"
+        ],
+        "summary": "Resolve a pull request review comment",
+        "operationId": "repoResolvePullReviewComment",
+        "parameters": [
+          {
+            "type": "string",
+            "description": "owner of the repo",
+            "name": "owner",
+            "in": "path",
+            "required": true
+          },
+          {
+            "type": "string",
+            "description": "name of the repo",
+            "name": "repo",
+            "in": "path",
+            "required": true
+          },
+          {
+            "type": "integer",
+            "format": "int64",
+            "description": "id of the review comment",
+            "name": "id",
+            "in": "path",
+            "required": true
+          }
+        ],
+        "responses": {
+          "204": {
+            "$ref": "#/responses/empty"
+          },
+          "400": {
+            "$ref": "#/responses/validationError"
+          },
+          "403": {
+            "$ref": "#/responses/forbidden"
+          },
+          "404": {
+            "$ref": "#/responses/notFound"
+          }
+        }
+      }
+    },
+    "/repos/{owner}/{repo}/pulls/comments/{id}/unresolve": {
+      "post": {
+        "produces": [
+          "application/json"
+        ],
+        "tags": [
+          "repository"
+        ],
+        "summary": "Unresolve a pull request review comment",
+        "operationId": "repoUnresolvePullReviewComment",
+        "parameters": [
+          {
+            "type": "string",
+            "description": "owner of the repo",
+            "name": "owner",
+            "in": "path",
+            "required": true
+          },
+          {
+            "type": "string",
+            "description": "name of the repo",
+            "name": "repo",
+            "in": "path",
+            "required": true
+          },
+          {
+            "type": "integer",
+            "format": "int64",
+            "description": "id of the review comment",
+            "name": "id",
+            "in": "path",
+            "required": true
+          }
+        ],
+        "responses": {
+          "204": {
+            "$ref": "#/responses/empty"
+          },
+          "400": {
+            "$ref": "#/responses/validationError"
+          },
+          "403": {
+            "$ref": "#/responses/forbidden"
+          },
+          "404": {
+            "$ref": "#/responses/notFound"
+          }
+        }
+      }
+    },
     "/repos/{owner}/{repo}/pulls/pinned": {
       "get": {
         "produces": [
@@ -23536,7 +23636,7 @@
       "x-go-package": "code.gitea.io/gitea/modules/structs"
     },
     "CreatePullReviewOptions": {
-      "description": "CreatePullReviewOptions are options to create a pull review",
+      "description": "CreatePullReviewOptions are options to create a pull request review",
       "type": "object",
       "properties": {
         "body": {
@@ -24133,7 +24233,7 @@
       "x-go-package": "code.gitea.io/gitea/modules/structs"
     },
     "DismissPullReviewOptions": {
-      "description": "DismissPullReviewOptions are options to dismiss a pull review",
+      "description": "DismissPullReviewOptions are options to dismiss a pull request review",
       "type": "object",
       "properties": {
         "message": {
@@ -27645,7 +27745,7 @@
       "x-go-package": "code.gitea.io/gitea/modules/structs"
     },
     "PullReviewRequestOptions": {
-      "description": "PullReviewRequestOptions are options to add or remove pull review requests",
+      "description": "PullReviewRequestOptions are options to add or remove pull request review requests",
       "type": "object",
       "properties": {
         "reviewers": {
@@ -28389,7 +28489,7 @@
       "x-go-package": "code.gitea.io/gitea/modules/structs"
     },
     "SubmitPullReviewOptions": {
-      "description": "SubmitPullReviewOptions are options to submit a pending pull review",
+      "description": "SubmitPullReviewOptions are options to submit a pending pull request review",
       "type": "object",
       "properties": {
         "body": {

tests/integration/api_pull_review_test.go

@@ -15,9 +15,11 @@ import (
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/json"
 	api "code.gitea.io/gitea/modules/structs"
 	issue_service "code.gitea.io/gitea/services/issue"
+	pull_service "code.gitea.io/gitea/services/pull"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
@@ -362,6 +364,79 @@ func TestAPIPullReviewRequest(t *testing.T) {
 	MakeRequest(t, req, http.StatusNoContent)
 }
 
+func TestAPIPullReviewCommentResolveEndpoints(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	ctx := t.Context()
+	pullIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 3})
+	require.NoError(t, pullIssue.LoadAttributes(ctx))
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: pullIssue.RepoID})
+
+	doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: pullIssue.PosterID})
+	require.NoError(t, pullIssue.LoadPullRequest(ctx))
+	gitRepo, err := gitrepo.OpenRepository(ctx, repo)
+	require.NoError(t, err)
+	defer gitRepo.Close()
+
+	latestCommitID, err := gitRepo.GetRefCommitID(pullIssue.PullRequest.GetGitHeadRefName())
+	require.NoError(t, err)
+
+	codeComment, err := pull_service.CreateCodeComment(ctx, doer, gitRepo, pullIssue, 1, "resolve comment", "README.md", false, 0, latestCommitID, nil)
+	require.NoError(t, err)
+	require.NotNil(t, codeComment)
+
+	session := loginUser(t, doer.Name)
+	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+
+	resolveURL := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/comments/%d/resolve", repo.OwnerName, repo.Name, codeComment.ID)
+	unresolveURL := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/comments/%d/unresolve", repo.OwnerName, repo.Name, codeComment.ID)
+
+	req := NewRequest(t, http.MethodPost, resolveURL).AddTokenAuth(token)
+	MakeRequest(t, req, http.StatusNoContent)
+
+	// Verify comment is resolved
+	updatedComment, err := issues_model.GetCommentByID(ctx, codeComment.ID)
+	require.NoError(t, err)
+	assert.NotZero(t, updatedComment.ResolveDoerID)
+	assert.Equal(t, doer.ID, updatedComment.ResolveDoerID)
+
+	// Resolving again should be idempotent
+	MakeRequest(t, req, http.StatusNoContent)
+
+	req = NewRequest(t, http.MethodPost, unresolveURL).AddTokenAuth(token)
+	MakeRequest(t, req, http.StatusNoContent)
+
+	// Verify comment is unresolved
+	updatedComment, err = issues_model.GetCommentByID(ctx, codeComment.ID)
+	require.NoError(t, err)
+	assert.Zero(t, updatedComment.ResolveDoerID)
+
+	// Unresolving again should be idempotent
+	MakeRequest(t, req, http.StatusNoContent)
+
+	// Non-existing comment ID
+	req = NewRequest(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/comments/999999/resolve", repo.OwnerName, repo.Name)).AddTokenAuth(token)
+	MakeRequest(t, req, http.StatusNotFound)
+
+	// Non-code-comment
+	plainComment, err := issue_service.CreateIssueComment(ctx, doer, repo, pullIssue, "not a review comment", nil)
+	require.NoError(t, err)
+	req = NewRequest(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/comments/%d/resolve", repo.OwnerName, repo.Name, plainComment.ID)).AddTokenAuth(token)
+	MakeRequest(t, req, http.StatusBadRequest)
+
+	// Test permission check: use a user without write access for target repo to test 403 response
+	unauthorizedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
+	require.NotEqual(t, pullIssue.PosterID, unauthorizedUser.ID)
+
+	unauthorizedSession := loginUser(t, unauthorizedUser.Name)
+	unauthorizedToken := getTokenForLoggedInUser(t, unauthorizedSession, auth_model.AccessTokenScopeWriteIssue, auth_model.AccessTokenScopeWriteRepository)
+
+	req = NewRequest(t, http.MethodGet, fmt.Sprintf("/api/v1/repos/%s/%s/issues/comments/%d", repo.OwnerName, repo.Name, plainComment.ID)).AddTokenAuth(unauthorizedToken)
+	MakeRequest(t, req, http.StatusOK)
+	req = NewRequest(t, http.MethodPost, resolveURL).AddTokenAuth(unauthorizedToken)
+	MakeRequest(t, req, http.StatusForbidden)
+}
+
 func TestAPIPullReviewStayDismissed(t *testing.T) {
 	// This test against issue https://github.com/go-gitea/gitea/issues/28542
 	// where old reviews surface after a review request got dismissed.