From 3d553deb2b098bc7316c866920205e8b6baea514 Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Tue, 5 Jan 2021 20:39:15 +0100 Subject: [PATCH 1/5] For public repos, all organization members should be assignable as reviewer --- models/repo.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/models/repo.go b/models/repo.go index f2453cc5c73a9..23d3ee2cd1a21 100644 --- a/models/repo.go +++ b/models/repo.go @@ -593,15 +593,19 @@ func (repo *Repository) getReviewers(e Engine, doerID, posterID int64) ([]*User, } // This is a "public" repository: - // Any user that has write access or who is a watcher can be requested to review + // Any user that has write access, is a watcher or organization member can be requested to review 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 ( ?, ?) "+ + "SELECT user_id FROM `access` WHERE repo_id = ? AND mode >= ? "+ "UNION "+ - "SELECT user_id FROM `watch` WHERE repo_id = ? AND user_id NOT IN ( ?, ?) AND mode IN (?, ?) "+ - ") ORDER BY name", - repo.ID, AccessModeRead, doerID, posterID, - repo.ID, doerID, posterID, RepoWatchModeNormal, RepoWatchModeAuto). + "SELECT user_id FROM `watch` WHERE repo_id = ? AND mode IN (?, ?) "+ + "UNION "+ + "SELECT user_id FROM `org_user` WHERE org_id = ? "+ + ") AND user_id NOT IN (?, ?) ORDER BY name", + repo.ID, AccessModeRead, + repo.ID, RepoWatchModeNormal, RepoWatchModeAuto, + repo.OwnerID, + doerID, posterID). Find(&users); err != nil { return nil, err } @@ -612,7 +616,7 @@ func (repo *Repository) getReviewers(e Engine, doerID, posterID int64) ([]*User, // GetReviewers get all users can be requested to review: // * for private repositories this returns all users that have read access or higher to the repository. // * for public repositories this returns all users that have write access or higher to the repository, -// and all repo watchers. +// all repo watchers and all organization members. // TODO: may be we should hava a busy choice for users to block review request to them. func (repo *Repository) GetReviewers(doerID, posterID int64) ([]*User, error) { return repo.getReviewers(x, doerID, posterID) From 1858fa2037f48c2941690cb9974986a47419509a Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Tue, 5 Jan 2021 21:22:38 +0100 Subject: [PATCH 2/5] Update models/repo.go Co-authored-by: zeripath --- models/repo.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/repo.go b/models/repo.go index 23d3ee2cd1a21..1db8d1bf47313 100644 --- a/models/repo.go +++ b/models/repo.go @@ -600,8 +600,8 @@ func (repo *Repository) getReviewers(e Engine, doerID, posterID int64) ([]*User, "UNION "+ "SELECT user_id FROM `watch` WHERE repo_id = ? AND mode IN (?, ?) "+ "UNION "+ - "SELECT user_id FROM `org_user` WHERE org_id = ? "+ - ") AND user_id NOT IN (?, ?) ORDER BY name", + "SELECT uid AS user_id FROM `org_user` WHERE org_id = ? "+ + ") AND uid NOT IN (?, ?) ORDER BY name", repo.ID, AccessModeRead, repo.ID, RepoWatchModeNormal, RepoWatchModeAuto, repo.OwnerID, From 49df82ae81c720e91a8be394c9e9d1d50a746e77 Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Tue, 5 Jan 2021 21:32:30 +0100 Subject: [PATCH 3/5] fix query --- models/repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/repo.go b/models/repo.go index 1db8d1bf47313..bcf513ef2cb62 100644 --- a/models/repo.go +++ b/models/repo.go @@ -601,7 +601,7 @@ func (repo *Repository) getReviewers(e Engine, doerID, posterID int64) ([]*User, "SELECT user_id FROM `watch` WHERE repo_id = ? AND mode IN (?, ?) "+ "UNION "+ "SELECT uid AS user_id FROM `org_user` WHERE org_id = ? "+ - ") AND uid NOT IN (?, ?) ORDER BY name", + ") AND id NOT IN (?, ?) ORDER BY name", repo.ID, AccessModeRead, repo.ID, RepoWatchModeNormal, RepoWatchModeAuto, repo.OwnerID, From 679c192cb73ece37c4f1d918a1f7b5ff3a3a9f3c Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Thu, 28 Jan 2021 21:26:42 +0100 Subject: [PATCH 4/5] fix comments --- models/repo.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/repo.go b/models/repo.go index 4bcdac34160fe..28fe0e1bb436f 100644 --- a/models/repo.go +++ b/models/repo.go @@ -593,7 +593,7 @@ func (repo *Repository) getReviewers(e Engine, doerID, posterID int64) ([]*User, } // This is a "public" repository: - // Any user that has write access, is a watcher or organization member can be requested to review + // Any user that has read access, is a watcher or organization member can be requested to review if err := e. SQL("SELECT * FROM `user` WHERE id IN ( "+ "SELECT user_id FROM `access` WHERE repo_id = ? AND mode >= ? "+ @@ -615,7 +615,7 @@ func (repo *Repository) getReviewers(e Engine, doerID, posterID int64) ([]*User, // GetReviewers get all users can be requested to review: // * for private repositories this returns all users that have read access or higher to the repository. -// * for public repositories this returns all users that have write access or higher to the repository, +// * for public repositories this returns all users that have read access or higher to the repository, // all repo watchers and all organization members. // TODO: may be we should hava a busy choice for users to block review request to them. func (repo *Repository) GetReviewers(doerID, posterID int64) ([]*User, error) { From 98f55633fc613354e68489561ef848b1a92d726c Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Thu, 25 Feb 2021 19:45:12 +0100 Subject: [PATCH 5/5] Restore typo fix that got lost in merge conflict --- models/repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/repo.go b/models/repo.go index ca66a71fc68aa..6cb6f38a48d14 100644 --- a/models/repo.go +++ b/models/repo.go @@ -617,7 +617,7 @@ func (repo *Repository) getReviewers(e Engine, doerID, posterID int64) ([]*User, // * for private repositories this returns all users that have read access or higher to the repository. // * for public repositories this returns all users that have read access or higher to the repository, // all repo watchers and all organization members. -// TODO: may be we should hava a busy choice for users to block review request to them. +// TODO: may be we should have a busy choice for users to block review request to them. func (repo *Repository) GetReviewers(doerID, posterID int64) ([]*User, error) { return repo.getReviewers(x, doerID, posterID) }