diff --git a/models/issues/comment.go b/models/issues/comment.go index 9c249d2c05..f15618bf50 100644 --- a/models/issues/comment.go +++ b/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 diff --git a/models/issues/comment_code.go b/models/issues/comment_code.go index 55e67a1243..8de52f815a 100644 --- a/models/issues/comment_code.go +++ b/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++ diff --git a/modules/structs/pull_review.go b/modules/structs/pull_review.go index e93e4e9720..f44d2f84f5 100644 --- a/modules/structs/pull_review.go +++ b/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"` diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index cba96aa08a..359d5af4c4 100644 --- a/routers/api/v1/api.go +++ b/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) diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index 4db1e878b1..37b5836e1d 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/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 diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index 3c00193fac..d4b268c009 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/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 diff --git a/services/convert/pull_review.go b/services/convert/pull_review.go index 574f34fa17..ba0102cf1f 100644 --- a/services/convert/pull_review.go +++ b/services/convert/pull_review.go @@ -92,34 +92,40 @@ func ToPullReviewCommentList(ctx context.Context, review *issues_model.Review, d 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(ctx), - } - - if comment.Line < 0 { - apiComment.OldLineNum = comment.UnsignedLine() - } else { - apiComment.LineNum = comment.UnsignedLine() - } - apiComments = append(apiComments, apiComment) + 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: comment.ReviewID, + 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: comment.Issue.HTMLURL(ctx), + } + + if comment.Line < 0 { + apiComment.OldLineNum = comment.UnsignedLine() + } else { + apiComment.LineNum = comment.UnsignedLine() + } + + return apiComment +} + func patch2diff(patch string) string { split := strings.Split(patch, "\n@@") if len(split) == 2 { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 6ed21e0451..45f4a84316 100644 --- a/templates/swagger/v1_json.tmpl +++ b/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": { diff --git a/tests/integration/api_pull_review_test.go b/tests/integration/api_pull_review_test.go index 9ffbd9a267..bdec278426 100644 --- a/tests/integration/api_pull_review_test.go +++ b/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.