From e96ba12ad068a81757c8fa97f9b0220fc9353148 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 24 Jun 2022 15:11:10 +0200 Subject: [PATCH 1/5] Dismiss antecessors pull reviews if done via web in review dismiss --- models/issues/review.go | 30 ++++++++++++++++++++++++++++++ routers/api/v1/repo/pull_review.go | 2 +- routers/web/repo/pull_review.go | 2 +- services/pull/review.go | 19 ++++++++++++++++++- 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/models/issues/review.go b/models/issues/review.go index 1cb99dc3373ff..5e2627cd3b986 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -19,6 +19,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "xorm.io/builder" ) @@ -474,6 +475,35 @@ func SubmitReview(doer *user_model.User, issue *Issue, reviewType ReviewType, co return review, comm, committer.Commit() } +// GetReviewOptions represent filter options for GetReviewByOpts +type GetReviewOptions struct { + IssueID int64 + ReviewerID int64 + Dismissed util.OptionalBool +} + +// GetReviewByOpts return reviews based on GetReviewOptions +func GetReviewByOpts(ctx context.Context, opts *GetReviewOptions) ([]*Review, error) { + if opts == nil { + return nil, fmt.Errorf("opts are nil") + } + + sess := db.GetEngine(ctx) + + if opts.IssueID != 0 { + sess = sess.Where("issue_id=?", opts.IssueID) + } + if opts.ReviewerID != 0 { + sess = sess.Where("reviewer_id=?", opts.ReviewerID) + } + if !opts.Dismissed.IsNone() { + sess = sess.Where("dismissed=?", opts.Dismissed.IsTrue()) + } + + reviews := make([]*Review, 0, 4) + return reviews, sess.Find(&reviews) +} + // GetReviewersByIssueID gets the latest review of each reviewer for a pull request func GetReviewersByIssueID(issueID int64) ([]*Review, error) { reviews := make([]*Review, 0, 10) diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index 1b61a40222d69..c37687e1e4d31 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -886,7 +886,7 @@ func dismissReview(ctx *context.APIContext, msg string, isDismiss bool) { return } - _, err := pull_service.DismissReview(ctx, review.ID, ctx.Repo.Repository.ID, msg, ctx.Doer, isDismiss) + _, err := pull_service.DismissReview(ctx, review.ID, ctx.Repo.Repository.ID, msg, ctx.Doer, isDismiss, false) if err != nil { ctx.Error(http.StatusInternalServerError, "pull_service.DismissReview", err) return diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 5a9f7a8138909..bc64f35472ea4 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -242,7 +242,7 @@ func SubmitReview(ctx *context.Context) { // DismissReview dismissing stale review by repo admin func DismissReview(ctx *context.Context) { form := web.GetForm(ctx).(*forms.DismissReviewForm) - comm, err := pull_service.DismissReview(ctx, form.ReviewID, ctx.Repo.Repository.ID, form.Message, ctx.Doer, true) + comm, err := pull_service.DismissReview(ctx, form.ReviewID, ctx.Repo.Repository.ID, form.Message, ctx.Doer, true, true) if err != nil { ctx.ServerError("pull_service.DismissReview", err) return diff --git a/services/pull/review.go b/services/pull/review.go index 22e0ae9853955..69d27c51f0a7e 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -20,6 +20,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) // CreateCodeComment creates a comment on the code line @@ -271,7 +272,7 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos } // DismissReview dismissing stale review by repo admin -func DismissReview(ctx context.Context, reviewID, repoID int64, message string, doer *user_model.User, isDismiss bool) (comment *issues_model.Comment, err error) { +func DismissReview(ctx context.Context, reviewID, repoID int64, message string, doer *user_model.User, isDismiss, dismissAntecessors bool) (comment *issues_model.Comment, err error) { review, err := issues_model.GetReviewByID(ctx, reviewID) if err != nil { return @@ -295,6 +296,22 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string, return } + if dismissAntecessors { + reviews, err := issues_model.GetReviewByOpts(ctx, &issues_model.GetReviewOptions{ + IssueID: review.IssueID, + ReviewerID: review.ReviewerID, + Dismissed: util.OptionalBoolFalse, + }) + if err != nil { + return nil, err + } + for _, oldReview := range reviews { + if err = issues_model.DismissReview(oldReview, true); err != nil { + return nil, err + } + } + } + if !isDismiss { return nil, nil } From 3a8afa02092f8ec0d06698b535595450af3f677e Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 4 Jul 2022 10:25:02 +0200 Subject: [PATCH 2/5] add option to api too --- modules/structs/pull_review.go | 3 ++- routers/api/v1/repo/pull_review.go | 8 ++++---- templates/swagger/v1_json.tmpl | 4 ++++ 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/modules/structs/pull_review.go b/modules/structs/pull_review.go index 6544604acbaed..6fb6bf077b508 100644 --- a/modules/structs/pull_review.go +++ b/modules/structs/pull_review.go @@ -96,7 +96,8 @@ type SubmitPullReviewOptions struct { // DismissPullReviewOptions are options to dismiss a pull review type DismissPullReviewOptions struct { - Message string `json:"message"` + Message string `json:"message"` + Antecessors bool `json:"antecessors"` } // PullReviewRequestOptions are options to add or remove pull review requests diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index c37687e1e4d31..f6625eeb3a996 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -823,7 +823,7 @@ func DismissPullReview(ctx *context.APIContext) { // "422": // "$ref": "#/responses/validationError" opts := web.GetForm(ctx).(*api.DismissPullReviewOptions) - dismissReview(ctx, opts.Message, true) + dismissReview(ctx, opts.Message, true, opts.Antecessors) } // UnDismissPullReview cancel to dismiss a review for a pull request @@ -863,10 +863,10 @@ func UnDismissPullReview(ctx *context.APIContext) { // "$ref": "#/responses/forbidden" // "422": // "$ref": "#/responses/validationError" - dismissReview(ctx, "", false) + dismissReview(ctx, "", false, false) } -func dismissReview(ctx *context.APIContext, msg string, isDismiss bool) { +func dismissReview(ctx *context.APIContext, msg string, isDismiss, dismissAntecessors bool) { if !ctx.Repo.IsAdmin() { ctx.Error(http.StatusForbidden, "", "Must be repo admin") return @@ -886,7 +886,7 @@ func dismissReview(ctx *context.APIContext, msg string, isDismiss bool) { return } - _, err := pull_service.DismissReview(ctx, review.ID, ctx.Repo.Repository.ID, msg, ctx.Doer, isDismiss, false) + _, err := pull_service.DismissReview(ctx, review.ID, ctx.Repo.Repository.ID, msg, ctx.Doer, isDismiss, dismissAntecessors) if err != nil { ctx.Error(http.StatusInternalServerError, "pull_service.DismissReview", err) return diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index f3f9a33672253..71d29cc9a1ab6 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -14863,6 +14863,10 @@ "description": "DismissPullReviewOptions are options to dismiss a pull review", "type": "object", "properties": { + "antecessors": { + "type": "boolean", + "x-go-name": "Antecessors" + }, "message": { "type": "string", "x-go-name": "Message" From a3c39e89a8b4947b51b149678f3d68585abac6ae Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 4 Jul 2022 10:29:08 +0200 Subject: [PATCH 3/5] rename --- models/issues/review.go | 6 +++--- services/pull/review.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/models/issues/review.go b/models/issues/review.go index 5e2627cd3b986..5835900801778 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -475,15 +475,15 @@ func SubmitReview(doer *user_model.User, issue *Issue, reviewType ReviewType, co return review, comm, committer.Commit() } -// GetReviewOptions represent filter options for GetReviewByOpts +// GetReviewOptions represent filter options for GetReviews type GetReviewOptions struct { IssueID int64 ReviewerID int64 Dismissed util.OptionalBool } -// GetReviewByOpts return reviews based on GetReviewOptions -func GetReviewByOpts(ctx context.Context, opts *GetReviewOptions) ([]*Review, error) { +// GetReviews return reviews based on GetReviewOptions +func GetReviews(ctx context.Context, opts *GetReviewOptions) ([]*Review, error) { if opts == nil { return nil, fmt.Errorf("opts are nil") } diff --git a/services/pull/review.go b/services/pull/review.go index 69d27c51f0a7e..51ddb6af9d193 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -297,7 +297,7 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string, } if dismissAntecessors { - reviews, err := issues_model.GetReviewByOpts(ctx, &issues_model.GetReviewOptions{ + reviews, err := issues_model.GetReviews(ctx, &issues_model.GetReviewOptions{ IssueID: review.IssueID, ReviewerID: review.ReviewerID, Dismissed: util.OptionalBoolFalse, From 7ec70d9c2c29b86b553658dc311e08db7cfa2122 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 13 Jul 2022 01:37:03 +0200 Subject: [PATCH 4/5] Antecessors -> Prior --- modules/structs/pull_review.go | 4 ++-- routers/api/v1/repo/pull_review.go | 6 +++--- services/pull/review.go | 4 ++-- templates/swagger/v1_json.tmpl | 8 ++++---- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/modules/structs/pull_review.go b/modules/structs/pull_review.go index 6fb6bf077b508..76bb2c29337a0 100644 --- a/modules/structs/pull_review.go +++ b/modules/structs/pull_review.go @@ -96,8 +96,8 @@ type SubmitPullReviewOptions struct { // DismissPullReviewOptions are options to dismiss a pull review type DismissPullReviewOptions struct { - Message string `json:"message"` - Antecessors bool `json:"antecessors"` + Message string `json:"message"` + Prior bool `json:"prior"` } // PullReviewRequestOptions are options to add or remove pull review requests diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index f6625eeb3a996..f5c6e86154795 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -823,7 +823,7 @@ func DismissPullReview(ctx *context.APIContext) { // "422": // "$ref": "#/responses/validationError" opts := web.GetForm(ctx).(*api.DismissPullReviewOptions) - dismissReview(ctx, opts.Message, true, opts.Antecessors) + dismissReview(ctx, opts.Message, true, opts.Prior) } // UnDismissPullReview cancel to dismiss a review for a pull request @@ -866,7 +866,7 @@ func UnDismissPullReview(ctx *context.APIContext) { dismissReview(ctx, "", false, false) } -func dismissReview(ctx *context.APIContext, msg string, isDismiss, dismissAntecessors bool) { +func dismissReview(ctx *context.APIContext, msg string, isDismiss, dismissPrior bool) { if !ctx.Repo.IsAdmin() { ctx.Error(http.StatusForbidden, "", "Must be repo admin") return @@ -886,7 +886,7 @@ func dismissReview(ctx *context.APIContext, msg string, isDismiss, dismissAntece return } - _, err := pull_service.DismissReview(ctx, review.ID, ctx.Repo.Repository.ID, msg, ctx.Doer, isDismiss, dismissAntecessors) + _, err := pull_service.DismissReview(ctx, review.ID, ctx.Repo.Repository.ID, msg, ctx.Doer, isDismiss, dismissPrior) if err != nil { ctx.Error(http.StatusInternalServerError, "pull_service.DismissReview", err) return diff --git a/services/pull/review.go b/services/pull/review.go index 51ddb6af9d193..2b232ad8728d3 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -272,7 +272,7 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos } // DismissReview dismissing stale review by repo admin -func DismissReview(ctx context.Context, reviewID, repoID int64, message string, doer *user_model.User, isDismiss, dismissAntecessors bool) (comment *issues_model.Comment, err error) { +func DismissReview(ctx context.Context, reviewID, repoID int64, message string, doer *user_model.User, isDismiss, dismissPrior bool) (comment *issues_model.Comment, err error) { review, err := issues_model.GetReviewByID(ctx, reviewID) if err != nil { return @@ -296,7 +296,7 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string, return } - if dismissAntecessors { + if dismissPrior { reviews, err := issues_model.GetReviews(ctx, &issues_model.GetReviewOptions{ IssueID: review.IssueID, ReviewerID: review.ReviewerID, diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index a76e69b465e64..177e642e79165 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -14861,13 +14861,13 @@ "description": "DismissPullReviewOptions are options to dismiss a pull review", "type": "object", "properties": { - "antecessors": { - "type": "boolean", - "x-go-name": "Antecessors" - }, "message": { "type": "string", "x-go-name": "Message" + }, + "prior": { + "type": "boolean", + "x-go-name": "Prior" } }, "x-go-package": "code.gitea.io/gitea/modules/structs" From dd43969b213416b63e528057e3f929e9ca6cef47 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 19 Jul 2022 10:54:56 +0100 Subject: [PATCH 5/5] Prior -> Priors Signed-off-by: Andrew Thornton --- modules/structs/pull_review.go | 2 +- routers/api/v1/repo/pull_review.go | 6 +++--- services/pull/review.go | 4 ++-- templates/swagger/v1_json.tmpl | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/modules/structs/pull_review.go b/modules/structs/pull_review.go index 76bb2c29337a0..7c9360a0c2200 100644 --- a/modules/structs/pull_review.go +++ b/modules/structs/pull_review.go @@ -97,7 +97,7 @@ type SubmitPullReviewOptions struct { // DismissPullReviewOptions are options to dismiss a pull review type DismissPullReviewOptions struct { Message string `json:"message"` - Prior bool `json:"prior"` + Priors bool `json:"priors"` } // PullReviewRequestOptions are options to add or remove pull review requests diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index f5c6e86154795..f36d0586ab6b9 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -823,7 +823,7 @@ func DismissPullReview(ctx *context.APIContext) { // "422": // "$ref": "#/responses/validationError" opts := web.GetForm(ctx).(*api.DismissPullReviewOptions) - dismissReview(ctx, opts.Message, true, opts.Prior) + dismissReview(ctx, opts.Message, true, opts.Priors) } // UnDismissPullReview cancel to dismiss a review for a pull request @@ -866,7 +866,7 @@ func UnDismissPullReview(ctx *context.APIContext) { dismissReview(ctx, "", false, false) } -func dismissReview(ctx *context.APIContext, msg string, isDismiss, dismissPrior bool) { +func dismissReview(ctx *context.APIContext, msg string, isDismiss, dismissPriors bool) { if !ctx.Repo.IsAdmin() { ctx.Error(http.StatusForbidden, "", "Must be repo admin") return @@ -886,7 +886,7 @@ func dismissReview(ctx *context.APIContext, msg string, isDismiss, dismissPrior return } - _, err := pull_service.DismissReview(ctx, review.ID, ctx.Repo.Repository.ID, msg, ctx.Doer, isDismiss, dismissPrior) + _, err := pull_service.DismissReview(ctx, review.ID, ctx.Repo.Repository.ID, msg, ctx.Doer, isDismiss, dismissPriors) if err != nil { ctx.Error(http.StatusInternalServerError, "pull_service.DismissReview", err) return diff --git a/services/pull/review.go b/services/pull/review.go index 2b232ad8728d3..8d8903c6a9521 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -272,7 +272,7 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos } // DismissReview dismissing stale review by repo admin -func DismissReview(ctx context.Context, reviewID, repoID int64, message string, doer *user_model.User, isDismiss, dismissPrior bool) (comment *issues_model.Comment, err error) { +func DismissReview(ctx context.Context, reviewID, repoID int64, message string, doer *user_model.User, isDismiss, dismissPriors bool) (comment *issues_model.Comment, err error) { review, err := issues_model.GetReviewByID(ctx, reviewID) if err != nil { return @@ -296,7 +296,7 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string, return } - if dismissPrior { + if dismissPriors { reviews, err := issues_model.GetReviews(ctx, &issues_model.GetReviewOptions{ IssueID: review.IssueID, ReviewerID: review.ReviewerID, diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 3ac4c8553e6c2..ff0b4c6468aea 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -14865,9 +14865,9 @@ "type": "string", "x-go-name": "Message" }, - "prior": { + "priors": { "type": "boolean", - "x-go-name": "Prior" + "x-go-name": "Priors" } }, "x-go-package": "code.gitea.io/gitea/modules/structs"