Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add request review from specific reviewers feature in pull request #10756

Merged
merged 10 commits into from
Apr 6, 2020
Prev Previous commit
Next Next commit
new change
  • Loading branch information
a1012112796 committed Apr 2, 2020
commit 29a6d24e192e812b9e9deeb12624b928105fd005
51 changes: 13 additions & 38 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ func (repo *Repository) getReviewersPrivate(e Engine, doerID, posterID int64) (u
users = make([]*User, 0, 20)

if err = e.
SQL("SELECT * FROM user WHERE id in (SELECT user_id FROM access WHERE repo_id = ? AND mode >= ? AND user_id NOT IN ( ?, ?))",
SQL("SELECT * FROM `user` WHERE id in (SELECT user_id FROM `access` WHERE repo_id = ? AND mode >= ? AND user_id NOT IN ( ?, ?)) ORDER BY name",
repo.ID, AccessModeRead,
doerID, posterID).
Find(&users); err != nil {
Expand All @@ -636,46 +636,21 @@ func (repo *Repository) getReviewersPrivate(e Engine, doerID, posterID int64) (u
return users, nil
}

func (repo *Repository) getReviewersPublic(e Engine, doerID, posterID int64) (users []*User, err error) {
func (repo *Repository) getReviewersPublic(e Engine, doerID, posterID int64) (_ []*User, err error) {

userIDs := make([]int64, 0, 20)
users := make([]*User, 0)

if err = e.
SQL("SELECT user_id FROM access WHERE repo_id = ? AND mode >= ? AND user_id NOT IN ( ?, ?)",
repo.ID, AccessModeRead,
doerID, posterID).
Find(&userIDs); err != nil {
return nil, err
}

getNum := len(userIDs)

watcherIDs := make([]int64, 0)

if err = e.SQL("SELECT user_id FROM watch WHERE repo_id = ? AND user_id NOT IN ( ?, ?)",
repo.ID, doerID, posterID).Find(&watcherIDs); err != nil {
return nil, err
}
const SQLCmd = "SELECT * FROM `user` WHERE id IN ( " +
"SELECT user_id FROM `access` WHERE repo_id = ? AND mode >= ? AND user_id NOT IN ( ?, ?) " +
"UNION " +
"SELECT user_id FROM `watch` WHERE repo_id = ? AND user_id NOT IN ( ?, ?) AND mode IN (?, ?) " +
") ORDER BY name"

for _, watcherID := range watcherIDs {
isHave := false
for index, userID := range userIDs {
if index >= getNum {
break
}
if userID == watcherID {
isHave = true
break
}
}

if !isHave {
userIDs = append(userIDs, watcherID)
}
}

users = make([]*User, 0, len(userIDs))
if err = e.In("id", userIDs).Find(&users); err != nil {
if err = e.
SQL(SQLCmd,
repo.ID, AccessModeRead, doerID, posterID,
repo.ID, doerID, posterID, RepoWatchModeNormal, RepoWatchModeAuto).
Find(&users); err != nil {
return nil, err
}

Expand Down
29 changes: 27 additions & 2 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -818,9 +818,28 @@ func ViewIssue(ctx *context.Context) {
// Check milestone and assignee.
if ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
RetrieveRepoMilestonesAndAssignees(ctx, repo)
if issue.IsPull {
if ctx.Written() {
return
}
}

if issue.IsPull {
canChooseReviewer := ctx.Repo.CanWrite(models.UnitTypePullRequests)
if !canChooseReviewer && ctx.User != nil && ctx.IsSigned {
canChooseReviewer, err = models.IsOfficialReviewer(issue, ctx.User)
if err != nil {
ctx.ServerError("IsOfficialReviewer", err)
return
}
}

if canChooseReviewer {
RetrieveRepoReviewers(ctx, repo, issue.PosterID)
ctx.Data["CanChooseReviewer"] = true
} else {
ctx.Data["CanChooseReviewer"] = false
}

if ctx.Written() {
return
}
Expand Down Expand Up @@ -1320,7 +1339,13 @@ func isLegalReviewRequest(reviewer, doer *models.User, isAdd bool, issue *models

pemResult = permDoer.CanAccessAny(models.AccessModeWrite, models.UnitTypePullRequests)
if !pemResult {
return fmt.Errorf("Doer can't write [user_id: %d, repo_name: %s]", doer.ID, issue.Repo.Name)
pemResult, err = models.IsOfficialReviewer(issue, doer)
if err != nil {
return err
}
if !pemResult {
return fmt.Errorf("Doer can't choose reviewer [user_id: %d, repo_name: %s, issue_id: %d]", doer.ID, issue.Repo.Name, issue.ID)
}
}

if doer.ID == reviewer.ID {
Expand Down
3 changes: 1 addition & 2 deletions routers/routes/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -738,8 +738,7 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Post("/labels", reqRepoIssuesOrPullsWriter, repo.UpdateIssueLabel)
m.Post("/milestone", reqRepoIssuesOrPullsWriter, repo.UpdateIssueMilestone)
m.Post("/assignee", reqRepoIssuesOrPullsWriter, repo.UpdateIssueAssignee)
m.Post("/request_review", reqRepoIssuesOrPullsWriter, repo.UpdatePullReviewRequest)
m.Post("/re_request_review", reqRepoIssuesOrPullsReader, repo.UpdatePullReviewRequest)
m.Post("/request_review", reqRepoIssuesOrPullsReader, repo.UpdatePullReviewRequest)
m.Post("/status", reqRepoIssuesOrPullsWriter, repo.UpdateIssueStatus)
}, context.RepoMustNotBeArchived())
m.Group("/comments/:id", func() {
Expand Down
4 changes: 2 additions & 2 deletions templates/repo/issue/view_content/pull.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@
{{$canChoose = true}}
{{end}}
{{else}}
{{if and (or $.IsIssuePoster $.IsIssueWriter) (not (eq $.SignedUserID .ReviewerID))}}
{{if and (or $.IsIssuePoster $.CanChooseReviewer) (not (eq $.SignedUserID .ReviewerID))}}
{{$canChoose = true}}
{{end}}
{{end}}

{{if $canChoose }}
<a href="#" class="ui poping up icon re-request-review" data-is-checked="{{if eq .Type 4}}remove{{else}}add{{end}}" data-issue-id="{{$.Issue.ID}}" data-content="{{ if eq .Type 4 }} {{$.i18n.Tr "repo.issues.remove_request_review"}} {{else}} {{$.i18n.Tr "repo.issues.re_request_review"}} {{end}}" data-id="{{.ReviewerID}}" data-update-url="{{$.RepoLink}}/issues/re_request_review">
<a href="#" class="ui poping up icon re-request-review" data-is-checked="{{if eq .Type 4}}remove{{else}}add{{end}}" data-issue-id="{{$.Issue.ID}}" data-content="{{ if eq .Type 4 }} {{$.i18n.Tr "repo.issues.remove_request_review"}} {{else}} {{$.i18n.Tr "repo.issues.re_request_review"}} {{end}}" data-id="{{.ReviewerID}}" data-update-url="{{$.RepoLink}}/issues/request_review">
{{svg "octicon-sync" 16}}
</a>
{{end}}
Expand Down
8 changes: 4 additions & 4 deletions templates/repo/issue/view_content/sidebar.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
{{if .Issue.IsPull }}

<input id="reviewer_id" name="reviewer_id" type="hidden" value="{{.reviewer_id}}">
<div class="ui {{if or (not .IsIssueWriter) .Repository.IsArchived}}disabled{{end}} floating jump select-reviewers-modify dropdown">
<div class="ui {{if or (not .CanChooseReviewer) .Repository.IsArchived}}disabled{{end}} floating jump select-reviewers-modify dropdown">
<span class="text">
<strong>{{.i18n.Tr "repo.issues.review.reviewers"}}</strong>
{{if and .IsIssueWriter (not .Repository.IsArchived)}}
{{if and .CanChooseReviewer (not .Repository.IsArchived)}}
{{svg "octicon-gear" 16}}
{{end}}
</span>
Expand Down Expand Up @@ -72,13 +72,13 @@
{{$canChoose = true}}
{{end}}
{{else}}
{{if and (or $.IsIssuePoster $.IsIssueWriter) (not (eq $.SignedUserID .ReviewerID))}}
{{if and (or $.IsIssuePoster $.CanChooseReviewer) (not (eq $.SignedUserID .ReviewerID))}}
{{$canChoose = true}}
{{end}}
{{end}}

{{if $canChoose}}
<a href="#" class="ui poping up icon re-request-review" data-is-checked="{{if eq .Type 4}}remove{{else}}add{{end}}" data-content="{{ if eq .Type 4 }} {{$.i18n.Tr "repo.issues.remove_request_review"}} {{else}} {{$.i18n.Tr "repo.issues.re_request_review"}} {{end}}" data-issue-id="{{$.Issue.ID}}" data-id="{{.ReviewerID}}" data-update-url="{{$.RepoLink}}/issues/re_request_review">
<a href="#" class="ui poping up icon re-request-review" data-is-checked="{{if eq .Type 4}}remove{{else}}add{{end}}" data-content="{{ if eq .Type 4 }} {{$.i18n.Tr "repo.issues.remove_request_review"}} {{else}} {{$.i18n.Tr "repo.issues.re_request_review"}} {{end}}" data-issue-id="{{$.Issue.ID}}" data-id="{{.ReviewerID}}" data-update-url="{{$.RepoLink}}/issues/request_review">
{{svg "octicon-sync" 16}}
</a>
{{end}}
Expand Down