From 4c5a98fd54b79d756e212a04148453e79e47e744 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Tue, 1 Sep 2020 23:29:59 +0800 Subject: [PATCH 01/15] Add dismiss review feature It's same with github refs: https://github.blog/2016-10-12-dismissing-reviews-on-pull-requests/ https://developer.github.com/v3/pulls/reviews/#dismiss-a-review-for-a-pull-request Signed-off-by: a1012112796 <1012112796@qq.com> --- integrations/api_pull_review_test.go | 9 +++ models/action.go | 51 ++++++------ models/branches.go | 4 +- models/fixtures/review.yml | 11 +++ models/issue_comment.go | 2 + models/issue_list.go | 2 +- models/migrations/migrations.go | 2 + models/migrations/v148.go | 22 +++++ models/pull.go | 2 +- models/review.go | 24 ++++-- models/review_test.go | 7 ++ modules/auth/repo_form.go | 6 ++ modules/convert/pull_review.go | 1 + modules/notification/action/action.go | 20 +++++ modules/notification/base/notifier.go | 1 + modules/notification/base/null.go | 4 + modules/notification/mail/mail.go | 6 ++ modules/notification/notification.go | 7 ++ modules/notification/ui/ui.go | 9 +++ modules/structs/pull_review.go | 6 ++ modules/templates/helper.go | 2 + options/locale/locale_en-US.ini | 6 ++ routers/api/v1/api.go | 1 + routers/api/v1/repo/pull_review.go | 76 ++++++++++++++++++ routers/api/v1/swagger/options.go | 3 + routers/repo/issue.go | 2 +- routers/repo/pull_review.go | 11 +++ routers/routes/routes.go | 1 + services/mailer/mail.go | 2 + services/pull/review.go | 47 +++++++++++ templates/mail/issue/default.tmpl | 2 + .../repo/issue/view_content/comments.tmpl | 45 ++++++++++- templates/repo/issue/view_content/pull.tmpl | 30 ++++++- templates/swagger/v1_json.tmpl | 80 ++++++++++++++++++- templates/user/dashboard/feeds.tmpl | 7 ++ web_src/js/index.js | 7 ++ 36 files changed, 479 insertions(+), 39 deletions(-) create mode 100644 models/migrations/v148.go diff --git a/integrations/api_pull_review_test.go b/integrations/api_pull_review_test.go index 611b34712caed..abb6e04d75c40 100644 --- a/integrations/api_pull_review_test.go +++ b/integrations/api_pull_review_test.go @@ -111,6 +111,15 @@ func TestAPIPullReview(t *testing.T) { assert.EqualValues(t, "APPROVED", review.State) assert.EqualValues(t, 3, review.CodeCommentsCount) + // test dismiss review + req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/reviews/%d/dismissals?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, review.ID, token), &api.DismissPullReviewOptions{ + Message: "test", + }) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &review) + assert.EqualValues(t, 6, review.ID) + assert.EqualValues(t, true, review.Dismissed) + // test DeletePullReview req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/reviews?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, token), &api.CreatePullReviewOptions{ Body: "just a comment", diff --git a/models/action.go b/models/action.go index f4d163afa3f53..06946ac3d6563 100644 --- a/models/action.go +++ b/models/action.go @@ -26,30 +26,31 @@ type ActionType int // Possible action types. const ( - ActionCreateRepo ActionType = iota + 1 // 1 - ActionRenameRepo // 2 - ActionStarRepo // 3 - ActionWatchRepo // 4 - ActionCommitRepo // 5 - ActionCreateIssue // 6 - ActionCreatePullRequest // 7 - ActionTransferRepo // 8 - ActionPushTag // 9 - ActionCommentIssue // 10 - ActionMergePullRequest // 11 - ActionCloseIssue // 12 - ActionReopenIssue // 13 - ActionClosePullRequest // 14 - ActionReopenPullRequest // 15 - ActionDeleteTag // 16 - ActionDeleteBranch // 17 - ActionMirrorSyncPush // 18 - ActionMirrorSyncCreate // 19 - ActionMirrorSyncDelete // 20 - ActionApprovePullRequest // 21 - ActionRejectPullRequest // 22 - ActionCommentPull // 23 - ActionPublishRelease // 24 + ActionCreateRepo ActionType = iota + 1 // 1 + ActionRenameRepo // 2 + ActionStarRepo // 3 + ActionWatchRepo // 4 + ActionCommitRepo // 5 + ActionCreateIssue // 6 + ActionCreatePullRequest // 7 + ActionTransferRepo // 8 + ActionPushTag // 9 + ActionCommentIssue // 10 + ActionMergePullRequest // 11 + ActionCloseIssue // 12 + ActionReopenIssue // 13 + ActionClosePullRequest // 14 + ActionReopenPullRequest // 15 + ActionDeleteTag // 16 + ActionDeleteBranch // 17 + ActionMirrorSyncPush // 18 + ActionMirrorSyncCreate // 19 + ActionMirrorSyncDelete // 20 + ActionApprovePullRequest // 21 + ActionRejectPullRequest // 22 + ActionCommentPull // 23 + ActionPublishRelease // 24 + ActionPullReviewDismissed // 25 ) // Action represents user operation type and other information to @@ -259,7 +260,7 @@ func (a *Action) GetCreate() time.Time { // GetIssueInfos returns a list of issues associated with // the action. func (a *Action) GetIssueInfos() []string { - return strings.SplitN(a.Content, "|", 2) + return strings.SplitN(a.Content, "|", 3) } // GetIssueTitle returns the title of first issue associated diff --git a/models/branches.go b/models/branches.go index 20988deed712d..f11edc4c5ef4d 100644 --- a/models/branches.go +++ b/models/branches.go @@ -157,7 +157,8 @@ func (protectBranch *ProtectedBranch) HasEnoughApprovals(pr *PullRequest) bool { func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest) int64 { sess := x.Where("issue_id = ?", pr.IssueID). And("type = ?", ReviewTypeApprove). - And("official = ?", true) + And("official = ?", true). + And("dismissed = ?", false) if protectBranch.DismissStaleApprovals { sess = sess.And("stale = ?", false) } @@ -179,6 +180,7 @@ func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullReque rejectExist, err := x.Where("issue_id = ?", pr.IssueID). And("type in ( ?, ?)", ReviewTypeReject, ReviewTypeRequest). And("official = ?", true). + And("dismissed = ?", false). Exist(new(Review)) if err != nil { log.Error("MergeBlockedByRejectedReview: %v", err) diff --git a/models/fixtures/review.yml b/models/fixtures/review.yml index 35d3dee2e6b06..7751b13b0644c 100644 --- a/models/fixtures/review.yml +++ b/models/fixtures/review.yml @@ -82,3 +82,14 @@ official: true updated_unix: 946684815 created_unix: 946684815 + + - + id: 11 + type: 1 + reviewer_id: 7 + issue_id: 3 + content: "Invalid Review #2" + updated_unix: 946684822 + created_unix: 946684822 + dismissed: true + \ No newline at end of file diff --git a/models/issue_comment.go b/models/issue_comment.go index 726ed7472bcf5..045d06476481c 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -101,6 +101,8 @@ const ( CommentTypeProject // Project board changed CommentTypeProjectBoard + // Dismiss Review + CommentTypeDismissReview ) // CommentTag defines comment tag type diff --git a/models/issue_list.go b/models/issue_list.go index 628058eb35903..5789ad84ae486 100644 --- a/models/issue_list.go +++ b/models/issue_list.go @@ -530,7 +530,7 @@ func (issues IssueList) getApprovalCounts(e Engine) (map[int64][]*ReviewCount, e } sess := e.In("issue_id", ids) err := sess.Select("issue_id, type, count(id) as `count`"). - Where("official = ?", true). + Where("official = ? AND dismissed = ?", true, false). GroupBy("issue_id, type"). OrderBy("issue_id"). Table("review"). diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 721b045fdce6c..d6ac01700597c 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -228,6 +228,8 @@ var migrations = []Migration{ NewMigration("Add projects info to repository table", addProjectsInfo), // v147 -> v148 NewMigration("create review for 0 review id code comments", createReviewsForCodeComments), + // v148 -> v149 + NewMigration("Add Dismissed to Review table", addDismissedReviewColumn), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v148.go b/models/migrations/v148.go new file mode 100644 index 0000000000000..dfb419e8f11d6 --- /dev/null +++ b/models/migrations/v148.go @@ -0,0 +1,22 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "fmt" + + "xorm.io/xorm" +) + +func addDismissedReviewColumn(x *xorm.Engine) error { + type Review struct { + Dismissed bool `xorm:"NOT NULL DEFAULT false"` + } + + if err := x.Sync2(new(Review)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + return nil +} diff --git a/models/pull.go b/models/pull.go index 9f1f485266a59..e56447004ac6b 100644 --- a/models/pull.go +++ b/models/pull.go @@ -232,7 +232,7 @@ func (pr *PullRequest) GetApprovalCounts() ([]*ReviewCount, error) { func (pr *PullRequest) getApprovalCounts(e Engine) ([]*ReviewCount, error) { rCounts := make([]*ReviewCount, 0, 6) sess := e.Where("issue_id = ?", pr.IssueID) - return rCounts, sess.Select("issue_id, type, count(id) as `count`").Where("official = ?", true).GroupBy("issue_id, type").Table("review").Find(&rCounts) + return rCounts, sess.Select("issue_id, type, count(id) as `count`").Where("official = ? AND dismissed = ?", true, false).GroupBy("issue_id, type").Table("review").Find(&rCounts) } // GetApprovers returns the approvers of the pull request diff --git a/models/review.go b/models/review.go index 5f27e2b7fd20e..34fd39abbcffd 100644 --- a/models/review.go +++ b/models/review.go @@ -60,9 +60,10 @@ type Review struct { IssueID int64 `xorm:"index"` Content string `xorm:"TEXT"` // Official is a review made by an assigned approver (counts towards approval) - Official bool `xorm:"NOT NULL DEFAULT false"` - CommitID string `xorm:"VARCHAR(40)"` - Stale bool `xorm:"NOT NULL DEFAULT false"` + Official bool `xorm:"NOT NULL DEFAULT false"` + CommitID string `xorm:"VARCHAR(40)"` + Stale bool `xorm:"NOT NULL DEFAULT false"` + Dismissed bool `xorm:"NOT NULL DEFAULT false"` CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` @@ -388,8 +389,8 @@ func GetReviewersByIssueID(issueID int64) (reviews []*Review, err error) { } // Get latest review of each reviwer, sorted in order they were made - if err := sess.SQL("SELECT * FROM review WHERE id IN (SELECT max(id) as id FROM review WHERE issue_id = ? AND type in (?, ?, ?) GROUP BY issue_id, reviewer_id) ORDER BY review.updated_unix ASC", - issueID, ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest). + if err := sess.SQL("SELECT * FROM review WHERE id IN (SELECT max(id) as id FROM review WHERE issue_id = ? AND type in (?, ?, ?) AND dismissed = ? GROUP BY issue_id, reviewer_id) ORDER BY review.updated_unix ASC", + issueID, ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest, false). Find(&reviewsUnfiltered); err != nil { return nil, err } @@ -439,6 +440,19 @@ func MarkReviewsAsNotStale(issueID int64, commitID string) (err error) { return } +// MarkReviewAsDismissed marks existing reviews as stale +func MarkReviewAsDismissed(review *Review) (err error) { + if review.Dismissed || (review.Type != ReviewTypeApprove && review.Type != ReviewTypeReject) { + return nil + } + + review.Dismissed = true + + _, err = x.Cols("dismissed").Update(review) + + return +} + // InsertReviews inserts review and review comments func InsertReviews(reviews []*Review) error { sess := x.NewSession() diff --git a/models/review_test.go b/models/review_test.go index 7103962ce9a98..41c602524b6f0 100644 --- a/models/review_test.go +++ b/models/review_test.go @@ -139,3 +139,10 @@ func TestGetReviewersByIssueID(t *testing.T) { } } } + +func TestMarkReviewAsDismissed(t *testing.T) { + review1 := AssertExistsAndLoadBean(t, &Review{ID: 9}).(*Review) + review2 := AssertExistsAndLoadBean(t, &Review{ID: 11}).(*Review) + assert.NoError(t, MarkReviewAsDismissed(review1)) + assert.NoError(t, MarkReviewAsDismissed(review2)) +} diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index b3fead7da97ac..0c29933a25ef1 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -577,6 +577,12 @@ func (f SubmitReviewForm) HasEmptyContent() bool { len(strings.TrimSpace(f.Content)) == 0 } +// DismissReviewForm for dismissing stale review by repo admin +type DismissReviewForm struct { + ReviewID int64 `binding:"Required"` + Message string +} + // __________ .__ // \______ \ ____ | | ____ _____ ______ ____ // | _// __ \| | _/ __ \\__ \ / ___// __ \ diff --git a/modules/convert/pull_review.go b/modules/convert/pull_review.go index 032d3617fc234..6fa17fe294f8a 100644 --- a/modules/convert/pull_review.go +++ b/modules/convert/pull_review.go @@ -33,6 +33,7 @@ func ToPullReview(r *models.Review, doer *models.User) (*api.PullReview, error) CommitID: r.CommitID, Stale: r.Stale, Official: r.Official, + Dismissed: r.Dismissed, CodeCommentsCount: r.GetCodeCommentsCount(), Submitted: r.CreatedUnix.AsTime(), HTMLURL: r.HTMLURL(), diff --git a/modules/notification/action/action.go b/modules/notification/action/action.go index 040cf3df1030d..af4ca16294b3d 100644 --- a/modules/notification/action/action.go +++ b/modules/notification/action/action.go @@ -266,6 +266,26 @@ func (*actionNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *mode } } +func (*actionNotifier) NotifyPullRevieweDismiss(doer *models.User, review *models.Review, comment *models.Comment) { + reviewerName := review.Reviewer.Name + if len(review.OriginalAuthor) > 0 { + reviewerName = review.OriginalAuthor + } + if err := models.NotifyWatchers(&models.Action{ + ActUserID: doer.ID, + ActUser: doer, + OpType: models.ActionPullReviewDismissed, + Content: fmt.Sprintf("%d|%s|%s", review.Issue.Index, reviewerName, comment.Content), + RepoID: review.Issue.Repo.ID, + Repo: review.Issue.Repo, + IsPrivate: review.Issue.Repo.IsPrivate, + CommentID: comment.ID, + Comment: comment, + }); err != nil { + log.Error("NotifyWatchers [%d]: %v", review.Issue.ID, err) + } +} + func (a *actionNotifier) NotifySyncPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repository.PushCommits) { data, err := json.Marshal(commits) if err != nil { diff --git a/modules/notification/base/notifier.go b/modules/notification/base/notifier.go index 428f9a9544f3c..709f0a635e359 100644 --- a/modules/notification/base/notifier.go +++ b/modules/notification/base/notifier.go @@ -37,6 +37,7 @@ type Notifier interface { NotifyPullRequestReview(*models.PullRequest, *models.Review, *models.Comment) NotifyPullRequestChangeTargetBranch(doer *models.User, pr *models.PullRequest, oldBranch string) NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment) + NotifyPullRevieweDismiss(doer *models.User, review *models.Review, comment *models.Comment) NotifyCreateIssueComment(*models.User, *models.Repository, *models.Issue, *models.Comment) diff --git a/modules/notification/base/null.go b/modules/notification/base/null.go index b2ce0742b6c16..59cca5605e01b 100644 --- a/modules/notification/base/null.go +++ b/modules/notification/base/null.go @@ -58,6 +58,10 @@ func (*NullNotifier) NotifyPullRequestChangeTargetBranch(doer *models.User, pr * func (*NullNotifier) NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment) { } +// NotifyPullRevieweDismiss notifies when a review was dismissed by repo admin +func (*NullNotifier) NotifyPullRevieweDismiss(doer *models.User, review *models.Review, comment *models.Comment) { +} + // NotifyUpdateComment places a place holder function func (*NullNotifier) NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) { } diff --git a/modules/notification/mail/mail.go b/modules/notification/mail/mail.go index 9b2c27280f7a4..36371d5e2b84e 100644 --- a/modules/notification/mail/mail.go +++ b/modules/notification/mail/mail.go @@ -146,6 +146,12 @@ func (m *mailNotifier) NotifyPullRequestPushCommits(doer *models.User, pr *model m.NotifyCreateIssueComment(doer, comment.Issue.Repo, comment.Issue, comment) } +func (m *mailNotifier) NotifyPullRevieweDismiss(doer *models.User, review *models.Review, comment *models.Comment) { + if err := mailer.MailParticipantsComment(comment, models.ActionPullReviewDismissed, review.Issue); err != nil { + log.Error("MailParticipantsComment: %v", err) + } +} + func (m *mailNotifier) NotifyNewRelease(rel *models.Release) { if err := rel.LoadAttributes(); err != nil { log.Error("NotifyNewRelease: %v", err) diff --git a/modules/notification/notification.go b/modules/notification/notification.go index d17b13b9e53f3..6dca5da67981f 100644 --- a/modules/notification/notification.go +++ b/modules/notification/notification.go @@ -101,6 +101,13 @@ func NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, com } } +// NotifyPullRevieweDismiss notifies when a review was dismissed by repo admin +func NotifyPullRevieweDismiss(doer *models.User, review *models.Review, comment *models.Comment) { + for _, notifier := range notifiers { + notifier.NotifyPullRevieweDismiss(doer, review, comment) + } +} + // NotifyUpdateComment notifies update comment to notifiers func NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) { for _, notifier := range notifiers { diff --git a/modules/notification/ui/ui.go b/modules/notification/ui/ui.go index cadc9720d5d1b..58ff4cef18788 100644 --- a/modules/notification/ui/ui.go +++ b/modules/notification/ui/ui.go @@ -114,6 +114,15 @@ func (ns *notificationService) NotifyPullRequestPushCommits(doer *models.User, p _ = ns.issueQueue.Push(opts) } +func (ns *notificationService) NotifyPullRevieweDismiss(doer *models.User, review *models.Review, comment *models.Comment) { + var opts = issueNotificationOpts{ + IssueID: review.IssueID, + NotificationAuthorID: doer.ID, + CommentID: comment.ID, + } + _ = ns.issueQueue.Push(opts) +} + func (ns *notificationService) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment) { if !removed { var opts = issueNotificationOpts{ diff --git a/modules/structs/pull_review.go b/modules/structs/pull_review.go index bf9eafc243640..e41b4107fce26 100644 --- a/modules/structs/pull_review.go +++ b/modules/structs/pull_review.go @@ -35,6 +35,7 @@ type PullReview struct { CommitID string `json:"commit_id"` Stale bool `json:"stale"` Official bool `json:"official"` + Dismissed bool `json:"dismissed"` CodeCommentsCount int `json:"comments_count"` // swagger:strfmt date-time Submitted time.Time `json:"submitted_at"` @@ -90,3 +91,8 @@ type SubmitPullReviewOptions struct { Event ReviewStateType `json:"event"` Body string `json:"body"` } + +// DismissPullReviewOptions are options to dismiss a pull review +type DismissPullReviewOptions struct { + Message string `json:"message"` +} diff --git a/modules/templates/helper.go b/modules/templates/helper.go index f86287f10bef9..231877e4a5292 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -667,6 +667,8 @@ func ActionIcon(opType models.ActionType) string { return "diff" case models.ActionPublishRelease: return "tag" + case models.ActionPullReviewDismissed: + return "x" default: return "question" } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 94d7ab27fb1b5..d52f90bfb9411 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1029,6 +1029,8 @@ issues.owner = Owner issues.re_request_review=Re-request review issues.remove_request_review=Remove review request issues.remove_request_review_block=Can't remove review request +issues.dismiss_review_btn = Dismiss review +issues.dismiss_review_confirm = Dismiss review confirm issues.sign_in_require_desc = Sign in to join this conversation. issues.edit = Edit issues.cancel = Cancel @@ -1141,6 +1143,8 @@ issues.review.self.approval = You cannot approve your own pull request. issues.review.self.rejection = You cannot request changes on your own pull request. issues.review.approve = "approved these changes %s" issues.review.comment = "reviewed %s" +issues.review.dismissed = "dismissed %s’s stale review %s" +issues.review.dismissed_label = Dismissed issues.review.left_comment = left a comment issues.review.content.empty = You need to leave a comment indicating the requested change(s). issues.review.reject = "requested changes %s" @@ -2388,6 +2392,8 @@ mirror_sync_delete = synced and deleted reference %[2]s at %s#%[2]s` reject_pull_request = `suggested changes for %s#%[2]s` publish_release = `released "%[4]s" at %[3]s` +review_dismissed = `dismissed review from %[4]s for %[3]s#%[2]s` +review_dismissed_reason = Reason: [tool] ago = %s ago diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index ab7ef6d6f714a..5ba7158b5e2aa 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -820,6 +820,7 @@ func RegisterRoutes(m *macaron.Macaron) { Post(reqToken(), bind(api.SubmitPullReviewOptions{}), repo.SubmitPullReview) m.Combo("/comments"). Get(repo.GetPullReviewComments) + m.Post("/dismissals", bind(api.DismissPullReviewOptions{}), repo.DismissPullReview) }) }) diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index 3f2cb011d8437..0cd847cbf4a14 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -539,3 +539,79 @@ func prepareSingleReview(ctx *context.APIContext) (*models.Review, *models.PullR return review, pr, false } + +// DismissPullReview dismiss a review for a pull request +func DismissPullReview(ctx *context.APIContext, opts api.DismissPullReviewOptions) { + // swagger:operation POST /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/dismissals repository repoDismissPullReview + // --- + // summary: Dismiss a review for a pull request + // 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: index + // in: path + // description: index of the pull request + // type: integer + // format: int64 + // required: true + // - name: id + // in: path + // description: id of the review + // type: integer + // format: int64 + // required: true + // - name: body + // in: body + // required: true + // schema: + // "$ref": "#/definitions/DismissPullReviewOptions" + // responses: + // "200": + // "$ref": "#/responses/PullReview" + // "403": + // "$ref": "#/responses/forbidden" + // "422": + // "$ref": "#/responses/validationError" + if !ctx.Repo.IsAdmin() { + ctx.Error(http.StatusForbidden, "", "Must be repo admin") + return + } + review, pr, isWrong := prepareSingleReview(ctx) + if isWrong { + return + } + + if (review.Type != models.ReviewTypeApprove && review.Type != models.ReviewTypeReject) || pr.Issue.IsClosed { + ctx.Error(http.StatusForbidden, "", "Wrong using") + return + } + + _, err := pull_service.DismissReview(review.ID, opts.Message, ctx.User) + if err != nil { + ctx.Error(http.StatusInternalServerError, "pull_service.DismissReview", err) + return + } + + if review, err = models.GetReviewByID(review.ID); err != nil { + ctx.Error(http.StatusInternalServerError, "GetReviewByID", err) + return + } + + // convert response + apiReview, err := convert.ToPullReview(review, ctx.User) + if err != nil { + ctx.Error(http.StatusInternalServerError, "convertToPullReview", err) + return + } + ctx.JSON(http.StatusOK, apiReview) +} diff --git a/routers/api/v1/swagger/options.go b/routers/api/v1/swagger/options.go index d9ef05c335991..13024afdba034 100644 --- a/routers/api/v1/swagger/options.go +++ b/routers/api/v1/swagger/options.go @@ -149,4 +149,7 @@ type swaggerParameterBodies struct { // in:body SubmitPullReviewOptions api.SubmitPullReviewOptions + + // in:body + DismissPullReviewOptions api.DismissPullReviewOptions } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index dabe0f6b0fe17..0d9b8c2a2307a 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -1082,7 +1082,7 @@ func ViewIssue(ctx *context.Context) { ctx.ServerError("LoadDepIssueDetails", err) return } - } else if comment.Type == models.CommentTypeCode || comment.Type == models.CommentTypeReview { + } else if comment.Type == models.CommentTypeCode || comment.Type == models.CommentTypeReview || comment.Type == models.CommentTypeDismissReview { comment.RenderedContent = string(markdown.Render([]byte(comment.Content), ctx.Repo.RepoLink, ctx.Repo.Repository.ComposeMetas())) if err = comment.LoadReview(); err != nil && !models.IsErrReviewNotExist(err) { diff --git a/routers/repo/pull_review.go b/routers/repo/pull_review.go index 730074b7f3f08..fdd5998fcb650 100644 --- a/routers/repo/pull_review.go +++ b/routers/repo/pull_review.go @@ -158,3 +158,14 @@ func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) { ctx.Redirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag())) } + +// DismissReview dismissing stale review by repo admin +func DismissReview(ctx *context.Context, form auth.DismissReviewForm) { + comm, err := pull_service.DismissReview(form.ReviewID, form.Message, ctx.User) + if err != nil { + ctx.ServerError("pull_service.DismissReview", err) + return + } + + ctx.Redirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, comm.Issue.Index, comm.HashTag())) +} diff --git a/routers/routes/routes.go b/routers/routes/routes.go index bdb82db6f5042..68388156dc72e 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -756,6 +756,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Post("/projects", reqRepoIssuesOrPullsWriter, repo.UpdateIssueProject) m.Post("/assignee", reqRepoIssuesOrPullsWriter, repo.UpdateIssueAssignee) m.Post("/request_review", reqRepoIssuesOrPullsReader, repo.UpdatePullReviewRequest) + m.Post("/dismiss_review", reqRepoAdmin, bindIgnErr(auth.DismissReviewForm{}), repo.DismissReview) m.Post("/status", reqRepoIssuesOrPullsWriter, repo.UpdateIssueStatus) m.Post("/resolve_conversation", reqRepoIssuesOrPullsReader, repo.UpdateResolveConversation) }, context.RepoMustNotBeArchived()) diff --git a/services/mailer/mail.go b/services/mailer/mail.go index b4217c046612b..e87d34ab29521 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -304,6 +304,8 @@ func actionToTemplate(issue *models.Issue, actionType models.ActionType, name = "reopen" case models.ActionMergePullRequest: name = "merge" + case models.ActionPullReviewDismissed: + name = "review_dismissed" default: switch commentType { case models.CommentTypeReview: diff --git a/services/pull/review.go b/services/pull/review.go index 5a77a4da16842..fbb1f9a7bd1f2 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -193,3 +193,50 @@ func SubmitReview(doer *models.User, gitRepo *git.Repository, issue *models.Issu return review, comm, nil } + +// DismissReview dismissing stale review by repo admin +func DismissReview(reviewID int64, message string, doer *models.User) (comment *models.Comment, err error) { + review, err := models.GetReviewByID(reviewID) + if err != nil { + return + } + + if review.Type != models.ReviewTypeApprove && review.Type != models.ReviewTypeReject { + return nil, fmt.Errorf("Wrong using") + } + + if err = models.MarkReviewAsDismissed(review); err != nil { + return + } + + // load data for notify + if err = review.LoadAttributes(); err != nil { + return + } + if err = review.Issue.LoadPullRequest(); err != nil { + return + } + if err = review.Issue.LoadAttributes(); err != nil { + return + } + + comment, err = models.CreateComment(&models.CreateCommentOptions{ + Doer: doer, + Content: message, + Type: models.CommentTypeDismissReview, + ReviewID: review.ID, + Issue: review.Issue, + Repo: review.Issue.Repo, + }) + if err != nil { + return + } + + comment.Review = review + comment.Poster = doer + comment.Issue = review.Issue + + notification.NotifyPullRevieweDismiss(doer, review, comment) + + return +} diff --git a/templates/mail/issue/default.tmpl b/templates/mail/issue/default.tmpl index e062dca7f1b5d..b7d576bef4adf 100644 --- a/templates/mail/issue/default.tmpl +++ b/templates/mail/issue/default.tmpl @@ -49,6 +49,8 @@ @{{.Doer.Name}} requested changes on this pull request. {{else if eq .ActionName "review"}} @{{.Doer.Name}} commented on this pull request. + {{else if eq .ActionName "review_dismissed"}} + @{{.Doer.Name}} dismissed last review from {{.Comment.Review.Reviewer.Name}} for this pull request. {{end}} {{- if eq .Body ""}} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index c222e6cec2451..54c6f0f094627 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -8,7 +8,8 @@ 18 = REMOVED_DEADLINE, 19 = ADD_DEPENDENCY, 20 = REMOVE_DEPENDENCY, 21 = CODE, 22 = REVIEW, 23 = ISSUE_LOCKED, 24 = ISSUE_UNLOCKED, 25 = TARGET_BRANCH_CHANGED, 26 = DELETE_TIME_MANUAL, 27 = REVIEW_REQUEST, 28 = MERGE_PULL_REQUEST, - 29 = PULL_PUSH_EVENT, 30 = PROJECT_CHANGED, 31 = PROJECT_BOARD_CHANGED --> + 29 = PULL_PUSH_EVENT, 30 = PROJECT_CHANGED, 31 = PROJECT_BOARD_CHANGED + 32 = DISMISSED_REVIEW --> {{if eq .Type 0}}
{{if .OriginalAuthor }} @@ -396,6 +397,9 @@ {{else}} {{$.i18n.Tr "repo.issues.review.comment" $createdStr | Safe}} {{end}} + {{if .Review.Dismissed}} +
{{$.i18n.Tr "repo.issues.review.dismissed_label"}}
+ {{end}}
{{if .Content}} @@ -637,5 +641,44 @@ {{end}} + {{else if eq .Type 32}} +
+
+ + + + {{svg "octicon-x" 16}} + + {{.Poster.GetDisplayName}} + {{$reviewerName := ""}} + {{if eq .Review.OriginalAuthor ""}} + {{$reviewerName = .Review.Reviewer.Name}} + {{else}} + {{$reviewerName = .Review.OriginalAuthor}} + {{end}} + {{$.i18n.Tr "repo.issues.review.dismissed" $reviewerName $createdStr | Safe}} + +
+ {{if .Content}} +
+
+
+ + {{$.i18n.Tr "action.review_dismissed_reason"}} + +
+
+
+ {{if .RenderedContent}} + {{.RenderedContent|Str2html}} + {{else}} + {{$.i18n.Tr "repo.issues.no_content"}} + {{end}} +
+
+
+
+ {{end}} +
{{end}} {{end}} diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 94edc8b12662b..b76a09e8e2655 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -27,9 +27,33 @@
{{if .Stale}} - - - + + + + {{end}} + {{if (and $.Permission.IsAdmin (or (eq .Type 1) (eq .Type 3)) (not $.Issue.IsClosed))}} + + {{svg "octicon-x" 16}} + + {{end}} {{if or (eq .GetOpType 5) (eq .GetOpType 18)}} @@ -101,6 +105,9 @@

{{index .GetIssueInfos 1}}

{{else if or (eq .GetOpType 12) (eq .GetOpType 13) (eq .GetOpType 14) (eq .GetOpType 15)}} {{.GetIssueTitle | RenderEmoji}} + {{else if eq .GetOpType 25}} +

{{$.i18n.Tr "action.review_dismissed_reason"}}

+

{{index .GetIssueInfos 2 | RenderEmoji}}

{{end}}

{{TimeSince .GetCreate $.i18n.Lang}}

diff --git a/web_src/js/index.js b/web_src/js/index.js index a8965a437f003..0e34a525a99ec 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -679,6 +679,13 @@ function initIssueComments() { ).then(reload); }); + $('.dismiss-review-btn').on('click', function (e) { + e.preventDefault(); + const $this = $(this); + const $dismissReviewModal = $this.next(); + $dismissReviewModal.modal('show'); + }); + $(document).on('click', (event) => { const urlTarget = $(':target'); if (urlTarget.length === 0) return; From 58a6148fcef48eb3eaddecba69ee0ef2a156ff34 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Tue, 1 Sep 2020 23:51:04 +0800 Subject: [PATCH 02/15] make fmt --- routers/repo/issue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 0d9b8c2a2307a..addbb8fb8864d 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -1082,7 +1082,7 @@ func ViewIssue(ctx *context.Context) { ctx.ServerError("LoadDepIssueDetails", err) return } - } else if comment.Type == models.CommentTypeCode || comment.Type == models.CommentTypeReview || comment.Type == models.CommentTypeDismissReview { + } else if comment.Type == models.CommentTypeCode || comment.Type == models.CommentTypeReview || comment.Type == models.CommentTypeDismissReview { comment.RenderedContent = string(markdown.Render([]byte(comment.Content), ctx.Repo.RepoLink, ctx.Repo.Repository.ComposeMetas())) if err = comment.LoadReview(); err != nil && !models.IsErrReviewNotExist(err) { From 2b84a0a269327e3ddc3ce54e28e406b219a5fc45 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Wed, 2 Sep 2020 00:10:01 +0800 Subject: [PATCH 03/15] fix test --- models/fixtures/review.yml | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/models/fixtures/review.yml b/models/fixtures/review.yml index 7751b13b0644c..8e715654f5197 100644 --- a/models/fixtures/review.yml +++ b/models/fixtures/review.yml @@ -83,13 +83,12 @@ updated_unix: 946684815 created_unix: 946684815 - - - id: 11 - type: 1 - reviewer_id: 7 - issue_id: 3 - content: "Invalid Review #2" - updated_unix: 946684822 - created_unix: 946684822 - dismissed: true - \ No newline at end of file +- + id: 11 + type: 1 + reviewer_id: 7 + issue_id: 3 + content: "Invalid Review #2" + updated_unix: 946684822 + created_unix: 946684822 + dismissed: true From 365fc83f68c85fbb07fabf9337830c3a18c13735 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Wed, 2 Sep 2020 00:32:30 +0800 Subject: [PATCH 04/15] fix test --- integrations/api_pull_review_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/api_pull_review_test.go b/integrations/api_pull_review_test.go index abb6e04d75c40..01515fbb88de2 100644 --- a/integrations/api_pull_review_test.go +++ b/integrations/api_pull_review_test.go @@ -28,7 +28,7 @@ func TestAPIPullReview(t *testing.T) { var reviews []*api.PullReview DecodeJSON(t, resp, &reviews) - if !assert.Len(t, reviews, 6) { + if !assert.Len(t, reviews, 7) { return } for _, r := range reviews { From 8e57c491e56435723e340ebc5aac8d5da30a9b7f Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Thu, 3 Sep 2020 20:56:37 +0800 Subject: [PATCH 05/15] change modal ui and error message --- options/locale/locale_en-US.ini | 1 + routers/api/v1/repo/pull_review.go | 9 +++++++-- templates/repo/issue/view_content/pull.tmpl | 5 ++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index d52f90bfb9411..df1de20f90f30 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1031,6 +1031,7 @@ issues.remove_request_review=Remove review request issues.remove_request_review_block=Can't remove review request issues.dismiss_review_btn = Dismiss review issues.dismiss_review_confirm = Dismiss review confirm +issues.dismiss_review_warning = Are you sure you want to dismiss this review? issues.sign_in_require_desc = Sign in to join this conversation. issues.edit = Edit issues.cancel = Cancel diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index 0cd847cbf4a14..2714861fd1c1b 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -591,8 +591,13 @@ func DismissPullReview(ctx *context.APIContext, opts api.DismissPullReviewOption return } - if (review.Type != models.ReviewTypeApprove && review.Type != models.ReviewTypeReject) || pr.Issue.IsClosed { - ctx.Error(http.StatusForbidden, "", "Wrong using") + if review.Type != models.ReviewTypeApprove && review.Type != models.ReviewTypeReject { + ctx.Error(http.StatusForbidden, "", "not need to dismiss this review because it's type is not Approve or change request") + return + } + + if pr.Issue.IsClosed { + ctx.Error(http.StatusForbidden, "", "not need to dismiss this review because this pr is closed") return } diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index b76a09e8e2655..d23cb2e1ac89d 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -40,6 +40,9 @@ {{$.i18n.Tr "repo.issues.dismiss_review_confirm"}}
+
+ {{$.i18n.Tr "repo.issues.dismiss_review_warning"}} +
{{$.CsrfTokenHtml}} @@ -48,7 +51,7 @@
-
{{$.i18n.Tr "settings.cancel"}}
+
{{$.i18n.Tr "settings.cancel"}}
From ca6282d649ec319c19037843b0c2631b330b24a0 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Thu, 3 Sep 2020 21:47:06 +0800 Subject: [PATCH 06/15] Add unDismissReview api --- integrations/api_pull_review_test.go | 7 +++ models/review.go | 13 +++++ models/review_test.go | 1 + routers/api/v1/api.go | 3 +- routers/api/v1/repo/pull_review.go | 76 ++++++++++++++++++++++++++++ services/pull/review.go | 15 ++++++ templates/swagger/v1_json.tmpl | 55 ++++++++++++++++++++ 7 files changed, 169 insertions(+), 1 deletion(-) diff --git a/integrations/api_pull_review_test.go b/integrations/api_pull_review_test.go index 01515fbb88de2..5e6a4a97e69c4 100644 --- a/integrations/api_pull_review_test.go +++ b/integrations/api_pull_review_test.go @@ -120,6 +120,13 @@ func TestAPIPullReview(t *testing.T) { assert.EqualValues(t, 6, review.ID) assert.EqualValues(t, true, review.Dismissed) + // test dismiss review + req = NewRequest(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/reviews/%d/undismissals?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, review.ID, token)) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &review) + assert.EqualValues(t, 6, review.ID) + assert.EqualValues(t, false, review.Dismissed) + // test DeletePullReview req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/reviews?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, token), &api.CreatePullReviewOptions{ Body: "just a comment", diff --git a/models/review.go b/models/review.go index 34fd39abbcffd..fbb35a24fc7f8 100644 --- a/models/review.go +++ b/models/review.go @@ -453,6 +453,19 @@ func MarkReviewAsDismissed(review *Review) (err error) { return } +// MarkReviewAsUnDismissed marks dismissed reviews as not dismissed +func MarkReviewAsUnDismissed(review *Review) (err error) { + if !review.Dismissed || (review.Type != ReviewTypeApprove && review.Type != ReviewTypeReject) { + return nil + } + + review.Dismissed = false + + _, err = x.Cols("dismissed").Update(review) + + return +} + // InsertReviews inserts review and review comments func InsertReviews(reviews []*Review) error { sess := x.NewSession() diff --git a/models/review_test.go b/models/review_test.go index 41c602524b6f0..51914e946c776 100644 --- a/models/review_test.go +++ b/models/review_test.go @@ -145,4 +145,5 @@ func TestMarkReviewAsDismissed(t *testing.T) { review2 := AssertExistsAndLoadBean(t, &Review{ID: 11}).(*Review) assert.NoError(t, MarkReviewAsDismissed(review1)) assert.NoError(t, MarkReviewAsDismissed(review2)) + assert.NoError(t, MarkReviewAsUnDismissed(review2)) } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 5ba7158b5e2aa..9be745077a525 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -820,7 +820,8 @@ func RegisterRoutes(m *macaron.Macaron) { Post(reqToken(), bind(api.SubmitPullReviewOptions{}), repo.SubmitPullReview) m.Combo("/comments"). Get(repo.GetPullReviewComments) - m.Post("/dismissals", bind(api.DismissPullReviewOptions{}), repo.DismissPullReview) + m.Post("/dismissals", reqToken(), bind(api.DismissPullReviewOptions{}), repo.DismissPullReview) + m.Post("/undismissals", reqToken(), repo.UnDismissPullReview) }) }) diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index 2714861fd1c1b..149d998c481f8 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -620,3 +620,79 @@ func DismissPullReview(ctx *context.APIContext, opts api.DismissPullReviewOption } ctx.JSON(http.StatusOK, apiReview) } + +// UnDismissPullReview cancel to dismiss a review for a pull request +func UnDismissPullReview(ctx *context.APIContext) { + // swagger:operation POST /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/undismissals repository repoUnDismissPullReview + // --- + // summary: Cancel to dismiss a review for a pull request + // 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: index + // in: path + // description: index of the pull request + // type: integer + // format: int64 + // required: true + // - name: id + // in: path + // description: id of the review + // type: integer + // format: int64 + // required: true + // responses: + // "200": + // "$ref": "#/responses/PullReview" + // "403": + // "$ref": "#/responses/forbidden" + // "422": + // "$ref": "#/responses/validationError" + if !ctx.Repo.IsAdmin() { + ctx.Error(http.StatusForbidden, "", "Must be repo admin") + return + } + review, pr, isWrong := prepareSingleReview(ctx) + if isWrong { + return + } + + if review.Type != models.ReviewTypeApprove && review.Type != models.ReviewTypeReject { + ctx.Error(http.StatusForbidden, "", "not need to dismiss this review because it's type is not Approve or change request") + return + } + + if pr.Issue.IsClosed { + ctx.Error(http.StatusForbidden, "", "not need to dismiss this review because this pr is closed") + return + } + + if err := pull_service.UnDismissReview(review.ID); err != nil { + ctx.Error(http.StatusInternalServerError, "pull_service.DismissReview", err) + return + } + + var err error + if review, err = models.GetReviewByID(review.ID); err != nil { + ctx.Error(http.StatusInternalServerError, "GetReviewByID", err) + return + } + + // convert response + apiReview, err := convert.ToPullReview(review, ctx.User) + if err != nil { + ctx.Error(http.StatusInternalServerError, "convertToPullReview", err) + return + } + ctx.JSON(http.StatusOK, apiReview) +} diff --git a/services/pull/review.go b/services/pull/review.go index fbb1f9a7bd1f2..f6fd46ea647c2 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -240,3 +240,18 @@ func DismissReview(reviewID int64, message string, doer *models.User) (comment * return } + +// UnDismissReview cancel dismissed stale review by repo admin +func UnDismissReview(reviewID int64) (err error) { + review, err := models.GetReviewByID(reviewID) + if err != nil { + return + } + + if review.Type != models.ReviewTypeApprove && review.Type != models.ReviewTypeReject { + return fmt.Errorf("Wrong using") + } + + err = models.MarkReviewAsUnDismissed(review) + return +} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 37455178e43a8..ccf3533b04fcc 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7411,6 +7411,61 @@ } } }, + "/repos/{owner}/{repo}/pulls/{index}/reviews/{id}/undismissals": { + "post": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Cancel to dismiss a review for a pull request", + "operationId": "repoUnDismissPullReview", + "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": "index of the pull request", + "name": "index", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the review", + "name": "id", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/PullReview" + }, + "403": { + "$ref": "#/responses/forbidden" + }, + "422": { + "$ref": "#/responses/validationError" + } + } + } + }, "/repos/{owner}/{repo}/pulls/{index}/update": { "post": { "produces": [ From dbaebf7268e5b45982119de17bcd1c1cb19e55a5 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Sun, 8 Nov 2020 12:11:36 +0800 Subject: [PATCH 07/15] fix bugs --- models/review.go | 4 ++-- templates/repo/issue/view_content/pull.tmpl | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/models/review.go b/models/review.go index 9c9281bff3a4d..c169c16b78fa7 100644 --- a/models/review.go +++ b/models/review.go @@ -467,8 +467,8 @@ func GetReviewersByIssueID(issueID int64) ([]*Review, error) { } // Get latest review of each reviwer, sorted in order they were made - if err := sess.SQL("SELECT * FROM review WHERE id IN (SELECT max(id) as id FROM review WHERE issue_id = ? AND reviewer_team_id = 0 AND type in (?, ?, ?) AND original_author_id = 0 GROUP BY issue_id, reviewer_id) ORDER BY review.updated_unix ASC", - issueID, ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest). + if err := sess.SQL("SELECT * FROM review WHERE id IN (SELECT max(id) as id FROM review WHERE issue_id = ? AND reviewer_team_id = 0 AND type in (?, ?, ?) AND dismissed = ? AND original_author_id = 0 GROUP BY issue_id, reviewer_id) ORDER BY review.updated_unix ASC", + issueID, ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest, false). Find(&reviews); err != nil { return nil, err } diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 719642f36012b..3d67d8f05d217 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -38,8 +38,8 @@ {{end}} - {{if (and $.Permission.IsAdmin (or (eq .Type 1) (eq .Type 3)) (not $.Issue.IsClosed))}} - + {{if (and $.Permission.IsAdmin (or (eq .Review.Type 1) (eq .Review.Type 3)) (not $.Issue.IsClosed))}} + {{svg "octicon-x" 16}}