Add dismiss review feature

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 {
@@ -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",

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

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)

models/fixtures/review.yml

@@ -82,3 +82,13 @@
   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

models/issue_comment.go

@@ -101,6 +101,8 @@ const (
 	CommentTypeProject
 	// Project board changed
 	CommentTypeProjectBoard
+	// Dismiss Review
+	CommentTypeDismissReview
 )
 
 // CommentTag defines comment tag type

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").

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

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
+}

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

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()

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))
+}

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
+}
+
 // __________       .__
 // \______   \ ____ |  |   ____ _____    ______ ____
 //  |       _// __ \|  | _/ __ \\__  \  /  ___// __ \

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(),

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 {

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)

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) {
 }

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)

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 {

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{

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"`
+}

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"
 	}

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 = <a href="%s">Sign in</a> 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 <code>%[2]s</code> at <a href=
 approve_pull_request = `approved <a href="%s/pulls/%s">%s#%[2]s</a>`
 reject_pull_request = `suggested changes for <a href="%s/pulls/%s">%s#%[2]s</a>`
 publish_release  = `released <a href="%s/releases/tag/%s"> "%[4]s" </a> at <a href="%[1]s">%[3]s</a>`
+review_dismissed = `dismissed review from <b>%[4]s</b> for <a href="%[1]s/pulls/%[2]s">%[3]s#%[2]s</a>`
+review_dismissed_reason = Reason:
 
 [tool]
 ago = %s ago