Add dismiss review feature (#12674)

* Add dismiss review feature

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

* change modal ui and error message

* Add unDismissReview api

Signed-off-by: a1012112796 <1012112796@qq.com>
Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: 6543 <6543@obermui.de>
tokarchuk/v1.17
a1012112796 4 years ago committed by GitHub
parent c69c01d2b6
commit ac701637b4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 16
      integrations/api_pull_review_test.go
  2. 51
      models/action.go
  3. 4
      models/branches.go
  4. 2
      models/issue_comment.go
  5. 2
      models/issue_list.go
  6. 2
      models/migrations/migrations.go
  7. 22
      models/migrations/v170.go
  8. 2
      models/pull.go
  9. 24
      models/review.go
  10. 10
      models/review_test.go
  11. 1
      modules/convert/pull_review.go
  12. 6
      modules/forms/repo_form.go
  13. 20
      modules/notification/action/action.go
  14. 1
      modules/notification/base/notifier.go
  15. 4
      modules/notification/base/null.go
  16. 6
      modules/notification/mail/mail.go
  17. 7
      modules/notification/notification.go
  18. 9
      modules/notification/ui/ui.go
  19. 6
      modules/structs/pull_review.go
  20. 2
      modules/templates/helper.go
  21. 7
      options/locale/locale_en-US.ini
  22. 2
      routers/api/v1/api.go
  23. 126
      routers/api/v1/repo/pull_review.go
  24. 3
      routers/api/v1/swagger/options.go
  25. 2
      routers/repo/issue.go
  26. 12
      routers/repo/pull_review.go
  27. 1
      routers/routes/web.go
  28. 2
      services/mailer/mail.go
  29. 51
      services/pull/review.go
  30. 2
      templates/mail/issue/default.tmpl
  31. 45
      templates/repo/issue/view_content/comments.tmpl
  32. 33
      templates/repo/issue/view_content/pull.tmpl
  33. 133
      templates/swagger/v1_json.tmpl
  34. 7
      templates/user/dashboard/feeds.tmpl
  35. 7
      web_src/js/index.js

@ -111,6 +111,22 @@ func TestAPIPullReview(t *testing.T) {
assert.EqualValues(t, "APPROVED", review.State) assert.EqualValues(t, "APPROVED", review.State)
assert.EqualValues(t, 3, review.CodeCommentsCount) 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 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 // 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{ 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", Body: "just a comment",

@ -26,30 +26,31 @@ type ActionType int
// Possible action types. // Possible action types.
const ( const (
ActionCreateRepo ActionType = iota + 1 // 1 ActionCreateRepo ActionType = iota + 1 // 1
ActionRenameRepo // 2 ActionRenameRepo // 2
ActionStarRepo // 3 ActionStarRepo // 3
ActionWatchRepo // 4 ActionWatchRepo // 4
ActionCommitRepo // 5 ActionCommitRepo // 5
ActionCreateIssue // 6 ActionCreateIssue // 6
ActionCreatePullRequest // 7 ActionCreatePullRequest // 7
ActionTransferRepo // 8 ActionTransferRepo // 8
ActionPushTag // 9 ActionPushTag // 9
ActionCommentIssue // 10 ActionCommentIssue // 10
ActionMergePullRequest // 11 ActionMergePullRequest // 11
ActionCloseIssue // 12 ActionCloseIssue // 12
ActionReopenIssue // 13 ActionReopenIssue // 13
ActionClosePullRequest // 14 ActionClosePullRequest // 14
ActionReopenPullRequest // 15 ActionReopenPullRequest // 15
ActionDeleteTag // 16 ActionDeleteTag // 16
ActionDeleteBranch // 17 ActionDeleteBranch // 17
ActionMirrorSyncPush // 18 ActionMirrorSyncPush // 18
ActionMirrorSyncCreate // 19 ActionMirrorSyncCreate // 19
ActionMirrorSyncDelete // 20 ActionMirrorSyncDelete // 20
ActionApprovePullRequest // 21 ActionApprovePullRequest // 21
ActionRejectPullRequest // 22 ActionRejectPullRequest // 22
ActionCommentPull // 23 ActionCommentPull // 23
ActionPublishRelease // 24 ActionPublishRelease // 24
ActionPullReviewDismissed // 25
) )
// Action represents user operation type and other information to // 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 // GetIssueInfos returns a list of issues associated with
// the action. // the action.
func (a *Action) GetIssueInfos() []string { 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 // GetIssueTitle returns the title of first issue associated

@ -157,7 +157,8 @@ func (protectBranch *ProtectedBranch) HasEnoughApprovals(pr *PullRequest) bool {
func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest) int64 { func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest) int64 {
sess := x.Where("issue_id = ?", pr.IssueID). sess := x.Where("issue_id = ?", pr.IssueID).
And("type = ?", ReviewTypeApprove). And("type = ?", ReviewTypeApprove).
And("official = ?", true) And("official = ?", true).
And("dismissed = ?", false)
if protectBranch.DismissStaleApprovals { if protectBranch.DismissStaleApprovals {
sess = sess.And("stale = ?", false) sess = sess.And("stale = ?", false)
} }
@ -178,6 +179,7 @@ func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullReque
rejectExist, err := x.Where("issue_id = ?", pr.IssueID). rejectExist, err := x.Where("issue_id = ?", pr.IssueID).
And("type = ?", ReviewTypeReject). And("type = ?", ReviewTypeReject).
And("official = ?", true). And("official = ?", true).
And("dismissed = ?", false).
Exist(new(Review)) Exist(new(Review))
if err != nil { if err != nil {
log.Error("MergeBlockedByRejectedReview: %v", err) log.Error("MergeBlockedByRejectedReview: %v", err)

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

@ -530,7 +530,7 @@ func (issues IssueList) getApprovalCounts(e Engine) (map[int64][]*ReviewCount, e
} }
sess := e.In("issue_id", ids) sess := e.In("issue_id", ids)
err := sess.Select("issue_id, type, count(id) as `count`"). err := sess.Select("issue_id, type, count(id) as `count`").
Where("official = ?", true). Where("official = ? AND dismissed = ?", true, false).
GroupBy("issue_id, type"). GroupBy("issue_id, type").
OrderBy("issue_id"). OrderBy("issue_id").
Table("review"). Table("review").

@ -286,6 +286,8 @@ var migrations = []Migration{
NewMigration("Recreate user table to fix default values", recreateUserTableToFixDefaultValues), NewMigration("Recreate user table to fix default values", recreateUserTableToFixDefaultValues),
// v169 -> v170 // v169 -> v170
NewMigration("Update DeleteBranch comments to set the old_ref to the commit_sha", commentTypeDeleteBranchUseOldRef), NewMigration("Update DeleteBranch comments to set the old_ref to the commit_sha", commentTypeDeleteBranchUseOldRef),
// v170 -> v171
NewMigration("Add Dismissed to Review table", addDismissedReviewColumn),
} }
// GetCurrentDBVersion returns the current db version // GetCurrentDBVersion returns the current db version

@ -0,0 +1,22 @@
// Copyright 2021 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
}

@ -234,7 +234,7 @@ func (pr *PullRequest) GetApprovalCounts() ([]*ReviewCount, error) {
func (pr *PullRequest) getApprovalCounts(e Engine) ([]*ReviewCount, error) { func (pr *PullRequest) getApprovalCounts(e Engine) ([]*ReviewCount, error) {
rCounts := make([]*ReviewCount, 0, 6) rCounts := make([]*ReviewCount, 0, 6)
sess := e.Where("issue_id = ?", pr.IssueID) 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 // GetApprovers returns the approvers of the pull request

@ -63,9 +63,10 @@ type Review struct {
IssueID int64 `xorm:"index"` IssueID int64 `xorm:"index"`
Content string `xorm:"TEXT"` Content string `xorm:"TEXT"`
// Official is a review made by an assigned approver (counts towards approval) // Official is a review made by an assigned approver (counts towards approval)
Official bool `xorm:"NOT NULL DEFAULT false"` Official bool `xorm:"NOT NULL DEFAULT false"`
CommitID string `xorm:"VARCHAR(40)"` CommitID string `xorm:"VARCHAR(40)"`
Stale bool `xorm:"NOT NULL DEFAULT false"` Stale bool `xorm:"NOT NULL DEFAULT false"`
Dismissed bool `xorm:"NOT NULL DEFAULT false"`
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
@ -466,8 +467,8 @@ func GetReviewersByIssueID(issueID int64) ([]*Review, error) {
} }
// Get latest review of each reviwer, sorted in order they were made // 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", 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). issueID, ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest, false).
Find(&reviews); err != nil { Find(&reviews); err != nil {
return nil, err return nil, err
} }
@ -558,6 +559,19 @@ func MarkReviewsAsNotStale(issueID int64, commitID string) (err error) {
return return
} }
// DismissReview change the dismiss status of a review
func DismissReview(review *Review, isDismiss bool) (err error) {
if review.Dismissed == isDismiss || (review.Type != ReviewTypeApprove && review.Type != ReviewTypeReject) {
return nil
}
review.Dismissed = isDismiss
_, err = x.Cols("dismissed").Update(review)
return
}
// InsertReviews inserts review and review comments // InsertReviews inserts review and review comments
func InsertReviews(reviews []*Review) error { func InsertReviews(reviews []*Review) error {
sess := x.NewSession() sess := x.NewSession()

@ -142,3 +142,13 @@ func TestGetReviewersByIssueID(t *testing.T) {
} }
} }
} }
func TestDismissReview(t *testing.T) {
review1 := AssertExistsAndLoadBean(t, &Review{ID: 9}).(*Review)
review2 := AssertExistsAndLoadBean(t, &Review{ID: 11}).(*Review)
assert.NoError(t, DismissReview(review1, true))
assert.NoError(t, DismissReview(review2, true))
assert.NoError(t, DismissReview(review2, true))
assert.NoError(t, DismissReview(review2, false))
assert.NoError(t, DismissReview(review2, false))
}

@ -34,6 +34,7 @@ func ToPullReview(r *models.Review, doer *models.User) (*api.PullReview, error)
CommitID: r.CommitID, CommitID: r.CommitID,
Stale: r.Stale, Stale: r.Stale,
Official: r.Official, Official: r.Official,
Dismissed: r.Dismissed,
CodeCommentsCount: r.GetCodeCommentsCount(), CodeCommentsCount: r.GetCodeCommentsCount(),
Submitted: r.CreatedUnix.AsTime(), Submitted: r.CreatedUnix.AsTime(),
HTMLURL: r.HTMLURL(), HTMLURL: r.HTMLURL(),

@ -622,6 +622,12 @@ func (f SubmitReviewForm) HasEmptyContent() bool {
len(strings.TrimSpace(f.Content)) == 0 len(strings.TrimSpace(f.Content)) == 0
} }
// DismissReviewForm for dismissing stale review by repo admin
type DismissReviewForm struct {
ReviewID int64 `binding:"Required"`
Message string
}
// __________ .__ // __________ .__
// \______ \ ____ | | ____ _____ ______ ____ // \______ \ ____ | | ____ _____ ______ ____
// | _// __ \| | _/ __ \\__ \ / ___// __ \ // | _// __ \| | _/ __ \\__ \ / ___// __ \

@ -275,6 +275,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) NotifyPushCommits(pusher *models.User, repo *models.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits) { func (a *actionNotifier) NotifyPushCommits(pusher *models.User, repo *models.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits) {
data, err := json.Marshal(commits) data, err := json.Marshal(commits)
if err != nil { if err != nil {

@ -39,6 +39,7 @@ type Notifier interface {
NotifyPullRequestCodeComment(pr *models.PullRequest, comment *models.Comment, mentions []*models.User) NotifyPullRequestCodeComment(pr *models.PullRequest, comment *models.Comment, mentions []*models.User)
NotifyPullRequestChangeTargetBranch(doer *models.User, pr *models.PullRequest, oldBranch string) NotifyPullRequestChangeTargetBranch(doer *models.User, pr *models.PullRequest, oldBranch string)
NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment) NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment)
NotifyPullRevieweDismiss(doer *models.User, review *models.Review, comment *models.Comment)
NotifyCreateIssueComment(doer *models.User, repo *models.Repository, NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
issue *models.Issue, comment *models.Comment, mentions []*models.User) issue *models.Issue, comment *models.Comment, mentions []*models.User)

@ -62,6 +62,10 @@ func (*NullNotifier) NotifyPullRequestChangeTargetBranch(doer *models.User, pr *
func (*NullNotifier) NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment) { 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 // NotifyUpdateComment places a place holder function
func (*NullNotifier) NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) { func (*NullNotifier) NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) {
} }

@ -152,6 +152,12 @@ func (m *mailNotifier) NotifyPullRequestPushCommits(doer *models.User, pr *model
m.NotifyCreateIssueComment(doer, comment.Issue.Repo, comment.Issue, comment, nil) m.NotifyCreateIssueComment(doer, comment.Issue.Repo, comment.Issue, comment, nil)
} }
func (m *mailNotifier) NotifyPullRevieweDismiss(doer *models.User, review *models.Review, comment *models.Comment) {
if err := mailer.MailParticipantsComment(comment, models.ActionPullReviewDismissed, review.Issue, []*models.User{}); err != nil {
log.Error("MailParticipantsComment: %v", err)
}
}
func (m *mailNotifier) NotifyNewRelease(rel *models.Release) { func (m *mailNotifier) NotifyNewRelease(rel *models.Release) {
if err := rel.LoadAttributes(); err != nil { if err := rel.LoadAttributes(); err != nil {
log.Error("NotifyNewRelease: %v", err) log.Error("NotifyNewRelease: %v", err)

@ -108,6 +108,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 // NotifyUpdateComment notifies update comment to notifiers
func NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) { func NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) {
for _, notifier := range notifiers { for _, notifier := range notifiers {

@ -161,6 +161,15 @@ func (ns *notificationService) NotifyPullRequestPushCommits(doer *models.User, p
_ = ns.issueQueue.Push(opts) _ = 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) { func (ns *notificationService) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment) {
if !removed { if !removed {
var opts = issueNotificationOpts{ var opts = issueNotificationOpts{

@ -36,6 +36,7 @@ type PullReview struct {
CommitID string `json:"commit_id"` CommitID string `json:"commit_id"`
Stale bool `json:"stale"` Stale bool `json:"stale"`
Official bool `json:"official"` Official bool `json:"official"`
Dismissed bool `json:"dismissed"`
CodeCommentsCount int `json:"comments_count"` CodeCommentsCount int `json:"comments_count"`
// swagger:strfmt date-time // swagger:strfmt date-time
Submitted time.Time `json:"submitted_at"` Submitted time.Time `json:"submitted_at"`
@ -92,6 +93,11 @@ type SubmitPullReviewOptions struct {
Body string `json:"body"` Body string `json:"body"`
} }
// DismissPullReviewOptions are options to dismiss a pull review
type DismissPullReviewOptions struct {
Message string `json:"message"`
}
// PullReviewRequestOptions are options to add or remove pull review requests // PullReviewRequestOptions are options to add or remove pull review requests
type PullReviewRequestOptions struct { type PullReviewRequestOptions struct {
Reviewers []string `json:"reviewers"` Reviewers []string `json:"reviewers"`

@ -798,6 +798,8 @@ func ActionIcon(opType models.ActionType) string {
return "diff" return "diff"
case models.ActionPublishRelease: case models.ActionPublishRelease:
return "tag" return "tag"
case models.ActionPullReviewDismissed:
return "x"
default: default:
return "question" return "question"
} }

@ -76,6 +76,7 @@ pull_requests = Pull Requests
issues = Issues issues = Issues
milestones = Milestones milestones = Milestones
ok = OK
cancel = Cancel cancel = Cancel
save = Save save = Save
add = Add add = Add
@ -1104,6 +1105,8 @@ issues.re_request_review=Re-request review
issues.is_stale = There have been changes to this PR since this review issues.is_stale = There have been changes to this PR since this review
issues.remove_request_review=Remove review request issues.remove_request_review=Remove review request
issues.remove_request_review_block=Can't remove review request issues.remove_request_review_block=Can't remove review request
issues.dismiss_review = Dismiss Review
issues.dismiss_review_warning = Are you sure you want to dismiss this review?
issues.sign_in_require_desc = <a href="%s">Sign in</a> to join this conversation. issues.sign_in_require_desc = <a href="%s">Sign in</a> to join this conversation.
issues.edit = Edit issues.edit = Edit
issues.cancel = Cancel issues.cancel = Cancel
@ -1216,6 +1219,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.self.rejection = You cannot request changes on your own pull request.
issues.review.approve = "approved these changes %s" issues.review.approve = "approved these changes %s"
issues.review.comment = "reviewed %s" issues.review.comment = "reviewed %s"
issues.review.dismissed = "dismissed %s’s review %s"
issues.review.dismissed_label = Dismissed
issues.review.left_comment = left a comment issues.review.left_comment = left a comment
issues.review.content.empty = You need to leave a comment indicating the requested change(s). issues.review.content.empty = You need to leave a comment indicating the requested change(s).
issues.review.reject = "requested changes %s" issues.review.reject = "requested changes %s"
@ -2519,6 +2524,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>` 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>` 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>` 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:
create_branch = created branch <a href="%[1]s/src/branch/%[2]s">%[3]s</a> in <a href="%[1]s">%[4]s</a> create_branch = created branch <a href="%[1]s/src/branch/%[2]s">%[3]s</a> in <a href="%[1]s">%[4]s</a>
[tool] [tool]

@ -891,6 +891,8 @@ func Routes() *web.Route {
Post(reqToken(), bind(api.SubmitPullReviewOptions{}), repo.SubmitPullReview) Post(reqToken(), bind(api.SubmitPullReviewOptions{}), repo.SubmitPullReview)
m.Combo("/comments"). m.Combo("/comments").
Get(repo.GetPullReviewComments) Get(repo.GetPullReviewComments)
m.Post("/dismissals", reqToken(), bind(api.DismissPullReviewOptions{}), repo.DismissPullReview)
m.Post("/undismissals", reqToken(), repo.UnDismissPullReview)
}) })
}) })
m.Combo("/requested_reviewers"). m.Combo("/requested_reviewers").

@ -757,3 +757,129 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions
return return
} }
} }
// DismissPullReview dismiss a review for a pull request
func DismissPullReview(ctx *context.APIContext) {
// 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"
opts := web.GetForm(ctx).(*api.DismissPullReviewOptions)
dismissReview(ctx, opts.Message, true)
}
// 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"
dismissReview(ctx, "", false)
}
func dismissReview(ctx *context.APIContext, msg string, isDismiss bool) {
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
}
_, err := pull_service.DismissReview(review.ID, msg, ctx.User, isDismiss)
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)
}

@ -150,6 +150,9 @@ type swaggerParameterBodies struct {
// in:body // in:body
SubmitPullReviewOptions api.SubmitPullReviewOptions SubmitPullReviewOptions api.SubmitPullReviewOptions
// in:body
DismissPullReviewOptions api.DismissPullReviewOptions
// in:body // in:body
MigrateRepoOptions api.MigrateRepoOptions MigrateRepoOptions api.MigrateRepoOptions

@ -1364,7 +1364,7 @@ func ViewIssue(ctx *context.Context) {
return 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, comment.RenderedContent = string(markdown.Render([]byte(comment.Content), ctx.Repo.RepoLink,
ctx.Repo.Repository.ComposeMetas())) ctx.Repo.Repository.ComposeMetas()))
if err = comment.LoadReview(); err != nil && !models.IsErrReviewNotExist(err) { if err = comment.LoadReview(); err != nil && !models.IsErrReviewNotExist(err) {

@ -223,3 +223,15 @@ func SubmitReview(ctx *context.Context) {
ctx.Redirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag())) 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 := web.GetForm(ctx).(*auth.DismissReviewForm)
comm, err := pull_service.DismissReview(form.ReviewID, form.Message, ctx.User, true)
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()))
}

@ -734,6 +734,7 @@ func RegisterRoutes(m *web.Route) {
m.Post("/projects", reqRepoIssuesOrPullsWriter, repo.UpdateIssueProject) m.Post("/projects", reqRepoIssuesOrPullsWriter, repo.UpdateIssueProject)
m.Post("/assignee", reqRepoIssuesOrPullsWriter, repo.UpdateIssueAssignee) m.Post("/assignee", reqRepoIssuesOrPullsWriter, repo.UpdateIssueAssignee)
m.Post("/request_review", reqRepoIssuesOrPullsReader, repo.UpdatePullReviewRequest) 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("/status", reqRepoIssuesOrPullsWriter, repo.UpdateIssueStatus)
m.Post("/resolve_conversation", reqRepoIssuesOrPullsReader, repo.UpdateResolveConversation) m.Post("/resolve_conversation", reqRepoIssuesOrPullsReader, repo.UpdateResolveConversation)
m.Post("/attachments", repo.UploadIssueAttachment) m.Post("/attachments", repo.UploadIssueAttachment)

@ -304,6 +304,8 @@ func actionToTemplate(issue *models.Issue, actionType models.ActionType,
name = "reopen" name = "reopen"
case models.ActionMergePullRequest: case models.ActionMergePullRequest:
name = "merge" name = "merge"
case models.ActionPullReviewDismissed:
name = "review_dismissed"
default: default:
switch commentType { switch commentType {
case models.CommentTypeReview: case models.CommentTypeReview:

@ -253,3 +253,54 @@ func SubmitReview(doer *models.User, gitRepo *git.Repository, issue *models.Issu
return review, comm, nil return review, comm, nil
} }
// DismissReview dismissing stale review by repo admin
func DismissReview(reviewID int64, message string, doer *models.User, isDismiss bool) (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("not need to dismiss this review because it's type is not Approve or change request")
}
if err = models.DismissReview(review, isDismiss); err != nil {
return
}
if !isDismiss {
return nil, nil
}
// 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
}

@ -49,6 +49,8 @@
<b>@{{.Doer.Name}}</b> requested changes on this pull request. <b>@{{.Doer.Name}}</b> requested changes on this pull request.
{{else if eq .ActionName "review"}} {{else if eq .ActionName "review"}}
<b>@{{.Doer.Name}}</b> commented on this pull request. <b>@{{.Doer.Name}}</b> commented on this pull request.
{{else if eq .ActionName "review_dismissed"}}
<b>@{{.Doer.Name}}</b> dismissed last review from {{.Comment.Review.Reviewer.Name}} for this pull request.
{{end}} {{end}}
{{- if eq .Body ""}} {{- if eq .Body ""}}

@ -8,7 +8,8 @@
18 = REMOVED_DEADLINE, 19 = ADD_DEPENDENCY, 20 = REMOVE_DEPENDENCY, 21 = CODE, 18 = REMOVED_DEADLINE, 19 = ADD_DEPENDENCY, 20 = REMOVE_DEPENDENCY, 21 = CODE,
22 = REVIEW, 23 = ISSUE_LOCKED, 24 = ISSUE_UNLOCKED, 25 = TARGET_BRANCH_CHANGED, 22 = REVIEW, 23 = ISSUE_LOCKED, 24 = ISSUE_UNLOCKED, 25 = TARGET_BRANCH_CHANGED,
26 = DELETE_TIME_MANUAL, 27 = REVIEW_REQUEST, 28 = MERGE_PULL_REQUEST, 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 eq .Type 0}}
<div class="timeline-item comment" id="{{.HashTag}}"> <div class="timeline-item comment" id="{{.HashTag}}">
{{if .OriginalAuthor }} {{if .OriginalAuthor }}
@ -415,6 +416,9 @@
{{else}} {{else}}
{{$.i18n.Tr "repo.issues.review.comment" $createdStr | Safe}} {{$.i18n.Tr "repo.issues.review.comment" $createdStr | Safe}}
{{end}} {{end}}
{{if .Review.Dismissed}}
<div class="ui small label">{{$.i18n.Tr "repo.issues.review.dismissed_label"}}</div>
{{end}}
</span> </span>
</div> </div>
{{if .Content}} {{if .Content}}
@ -698,5 +702,44 @@
</span> </span>
</div> </div>
{{end}} {{end}}
{{else if eq .Type 32}}
<div class="timeline-item-group">
<div class="timeline-item event" id="{{.HashTag}}">
<a class="timeline-avatar"{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>
<img src="{{.Poster.RelAvatarLink}}">
</a>
<span class="badge grey">{{svg "octicon-x" 16}}</span>
<span class="text grey">
<a class="author"{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>{{.Poster.GetDisplayName}}</a>
{{$reviewerName := ""}}
{{if eq .Review.OriginalAuthor ""}}
{{$reviewerName = .Review.Reviewer.Name}}
{{else}}
{{$reviewerName = .Review.OriginalAuthor}}
{{end}}
{{$.i18n.Tr "repo.issues.review.dismissed" $reviewerName $createdStr | Safe}}
</span>
</div>
{{if .Content}}
<div class="timeline-item comment">
<div class="content">
<div class="ui top attached header arrow-top">
<span class="text grey">
{{$.i18n.Tr "action.review_dismissed_reason"}}
</span>
</div>
<div class="ui attached segment">
<div class="render-content markdown">
{{if .RenderedContent}}
{{.RenderedContent|Str2html}}
{{else}}
<span class="no-content">{{$.i18n.Tr "repo.issues.no_content"}}</span>
{{end}}
</div>
</div>
</div>
</div>
{{end}}
</div>
{{end}} {{end}}
{{end}} {{end}}

@ -34,9 +34,36 @@
</div> </div>
<div class="review-item-right"> <div class="review-item-right">
{{if .Review.Stale}} {{if .Review.Stale}}
<span class="ui poping up type-icon text grey" data-content="{{$.i18n.Tr "repo.issues.is_stale"}}"> <span class="ui poping up type-icon text grey" data-content="{{$.i18n.Tr "repo.issues.is_stale"}}">
<i class="octicon icon fa-hourglass-end"></i> <i class="octicon icon fa-hourglass-end"></i>
</span> </span>
{{end}}
{{if (and $.Permission.IsAdmin (or (eq .Review.Type 1) (eq .Review.Type 3)) (not $.Issue.IsClosed))}}
<a href="#" class="ui grey poping up icon dismiss-review-btn" data-review-id="dismiss-review-{{.Review.ID}}" data-content="{{$.i18n.Tr "repo.issues.dismiss_review"}}">
{{svg "octicon-x" 16}}
</a>
<div class="ui small modal" id="dismiss-review-modal">
<div class="header">
{{$.i18n.Tr "repo.issues.dismiss_review"}}
</div>
<div class="content">
<div class="ui warning message text left">
{{$.i18n.Tr "repo.issues.dismiss_review_warning"}}
</div>
<form class="ui form dismiss-review-form" id="dismiss-review-{{.Review.ID}}" action="{{$.RepoLink}}/issues/dismiss_review" method="post">
{{$.CsrfTokenHtml}}
<input type="hidden" name="review_id" value="{{.Review.ID}}">
<div class="field">
<label for="message">{{$.i18n.Tr "action.review_dismissed_reason"}}</label>
<input id="message" name="message">
</div>
<div class="text right actions">
<div class="ui cancel button">{{$.i18n.Tr "settings.cancel"}}</div>
<button class="ui red button" type="submit">{{$.i18n.Tr "ok"}}</button>
</div>
</form>
</div>
</div>
{{end}} {{end}}
<span class="type-icon text {{if eq .Review.Type 1}}green <span class="type-icon text {{if eq .Review.Type 1}}green
{{- else if eq .Review.Type 2}}grey {{- else if eq .Review.Type 2}}grey

@ -7761,6 +7761,124 @@
} }
} }
}, },
"/repos/{owner}/{repo}/pulls/{index}/reviews/{id}/dismissals": {
"post": {
"produces": [
"application/json"
],
"tags": [
"repository"
],
"summary": "Dismiss a review for a pull request",
"operationId": "repoDismissPullReview",
"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
},
{
"name": "body",
"in": "body",
"required": true,
"schema": {
"$ref": "#/definitions/DismissPullReviewOptions"
}
}
],
"responses": {
"200": {
"$ref": "#/responses/PullReview"
},
"403": {
"$ref": "#/responses/forbidden"
},
"422": {
"$ref": "#/responses/validationError"
}
}
}
},
"/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": { "/repos/{owner}/{repo}/pulls/{index}/update": {
"post": { "post": {
"produces": [ "produces": [
@ -13036,6 +13154,17 @@
}, },
"x-go-package": "code.gitea.io/gitea/modules/structs" "x-go-package": "code.gitea.io/gitea/modules/structs"
}, },
"DismissPullReviewOptions": {
"description": "DismissPullReviewOptions are options to dismiss a pull review",
"type": "object",
"properties": {
"message": {
"type": "string",
"x-go-name": "Message"
}
},
"x-go-package": "code.gitea.io/gitea/modules/structs"
},
"EditAttachmentOptions": { "EditAttachmentOptions": {
"description": "EditAttachmentOptions options for editing attachments", "description": "EditAttachmentOptions options for editing attachments",
"type": "object", "type": "object",
@ -15199,6 +15328,10 @@
"type": "string", "type": "string",
"x-go-name": "CommitID" "x-go-name": "CommitID"
}, },
"dismissed": {
"type": "boolean",
"x-go-name": "Dismissed"
},
"html_url": { "html_url": {
"type": "string", "type": "string",
"x-go-name": "HTMLURL" "x-go-name": "HTMLURL"

@ -78,6 +78,10 @@
{{ $branchLink := .GetBranch | EscapePound | Escape}} {{ $branchLink := .GetBranch | EscapePound | Escape}}
{{ $linkText := .Content | RenderEmoji }} {{ $linkText := .Content | RenderEmoji }}
{{$.i18n.Tr "action.publish_release" .GetRepoLink $branchLink .ShortRepoPath $linkText | Str2html}} {{$.i18n.Tr "action.publish_release" .GetRepoLink $branchLink .ShortRepoPath $linkText | Str2html}}
{{else if eq .GetOpType 25}}
{{ $index := index .GetIssueInfos 0}}
{{ $reviewer := index .GetIssueInfos 1}}
{{$.i18n.Tr "action.review_dismissed" .GetRepoLink $index .ShortRepoPath $reviewer | Str2html}}
{{end}} {{end}}
</p> </p>
{{if or (eq .GetOpType 5) (eq .GetOpType 18)}} {{if or (eq .GetOpType 5) (eq .GetOpType 18)}}
@ -111,6 +115,9 @@
<p class="text light grey">{{index .GetIssueInfos 1}}</p> <p class="text light grey">{{index .GetIssueInfos 1}}</p>
{{else if or (eq .GetOpType 12) (eq .GetOpType 13) (eq .GetOpType 14) (eq .GetOpType 15)}} {{else if or (eq .GetOpType 12) (eq .GetOpType 13) (eq .GetOpType 14) (eq .GetOpType 15)}}
<span class="text truncate issue title">{{.GetIssueTitle | RenderEmoji}}</span> <span class="text truncate issue title">{{.GetIssueTitle | RenderEmoji}}</span>
{{else if eq .GetOpType 25}}
<p class="text light grey">{{$.i18n.Tr "action.review_dismissed_reason"}}</p>
<p class="text light grey">{{index .GetIssueInfos 2 | RenderEmoji}}</p>
{{end}} {{end}}
<p class="text italic light grey">{{TimeSince .GetCreate $.i18n.Lang}}</p> <p class="text italic light grey">{{TimeSince .GetCreate $.i18n.Lang}}</p>
</div> </div>

@ -677,6 +677,13 @@ function initIssueComments() {
return false; return false;
}); });
$('.dismiss-review-btn').on('click', function (e) {
e.preventDefault();
const $this = $(this);
const $dismissReviewModal = $this.next();
$dismissReviewModal.modal('show');
});
$(document).on('click', (event) => { $(document).on('click', (event) => {
const urlTarget = $(':target'); const urlTarget = $(':target');
if (urlTarget.length === 0) return; if (urlTarget.length === 0) return;

Loading…
Cancel
Save