mirror of
https://github.com/go-gitea/gitea.git
synced 2026-04-27 06:44:29 +00:00
Fix org team assignee/reviewer lookups for team member permissions (#37365)
Fix team members missing from assignee list when `team_unit.access_mode` is 0 but the doer is owner. Fix #34871 1. Use `GetTeamUserIDsWithAccessToAnyRepoUnit` for repo assignee list 2. Load assignee list for project issues directly 3. Use `GetTeamUserIDsWithAccessToAnyRepoUnit` for repo reviewer list Signed-off-by: Jakub Pisarczyk <pisarz77@gmail.com> Signed-off-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: silverwind <me@silverwind.io> Co-authored-by: Claude (Opus 4.7) <noreply@anthropic.com> Co-authored-by: Giteabot <teabot@gitea.io>
This commit is contained in:
@@ -8,10 +8,7 @@ import (
|
||||
"fmt"
|
||||
|
||||
"code.gitea.io/gitea/models/db"
|
||||
"code.gitea.io/gitea/models/perm"
|
||||
"code.gitea.io/gitea/models/unit"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
"code.gitea.io/gitea/modules/container"
|
||||
"code.gitea.io/gitea/modules/log"
|
||||
|
||||
"xorm.io/builder"
|
||||
@@ -129,49 +126,6 @@ func IsUserOrgOwner(ctx context.Context, users user_model.UserList, orgID int64)
|
||||
return results
|
||||
}
|
||||
|
||||
// GetOrgAssignees returns all users that have write access and can be assigned to issues
|
||||
// of the any repository in the organization.
|
||||
func GetOrgAssignees(ctx context.Context, orgID int64) (_ []*user_model.User, err error) {
|
||||
e := db.GetEngine(ctx)
|
||||
userIDs := make([]int64, 0, 10)
|
||||
if err = e.Table("access").
|
||||
Join("INNER", "repository", "`repository`.id = `access`.repo_id").
|
||||
Where("`repository`.owner_id = ? AND `access`.mode >= ?", orgID, perm.AccessModeWrite).
|
||||
Select("user_id").
|
||||
Find(&userIDs); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
additionalUserIDs := make([]int64, 0, 10)
|
||||
if err = e.Table("team_user").
|
||||
Join("INNER", "team_repo", "`team_repo`.team_id = `team_user`.team_id").
|
||||
Join("INNER", "team_unit", "`team_unit`.team_id = `team_user`.team_id").
|
||||
Join("INNER", "repository", "`repository`.id = `team_repo`.repo_id").
|
||||
Where("`repository`.owner_id = ? AND (`team_unit`.access_mode >= ? OR (`team_unit`.access_mode = ? AND `team_unit`.`type` = ?))",
|
||||
orgID, perm.AccessModeWrite, perm.AccessModeRead, unit.TypePullRequests).
|
||||
Distinct("`team_user`.uid").
|
||||
Select("`team_user`.uid").
|
||||
Find(&additionalUserIDs); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
uniqueUserIDs := make(container.Set[int64])
|
||||
uniqueUserIDs.AddMultiple(userIDs...)
|
||||
uniqueUserIDs.AddMultiple(additionalUserIDs...)
|
||||
|
||||
users := make([]*user_model.User, 0, len(uniqueUserIDs))
|
||||
if len(userIDs) > 0 {
|
||||
if err = e.In("id", uniqueUserIDs.Values()).
|
||||
Where(builder.Eq{"`user`.is_active": true}).
|
||||
OrderBy(user_model.GetOrderByName()).
|
||||
Find(&users); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
return users, nil
|
||||
}
|
||||
|
||||
func loadOrganizationOwners(ctx context.Context, users user_model.UserList, orgID int64) (map[int64]*TeamUser, error) {
|
||||
if len(users) == 0 {
|
||||
return nil, nil //nolint:nilnil // return nil when there are no users
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"strings"
|
||||
|
||||
"code.gitea.io/gitea/models/db"
|
||||
"code.gitea.io/gitea/models/organization"
|
||||
"code.gitea.io/gitea/models/perm"
|
||||
"code.gitea.io/gitea/models/unit"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
@@ -94,8 +95,7 @@ func GetWatchedRepos(ctx context.Context, opts *WatchedReposOptions) ([]*Reposit
|
||||
return db.FindAndCount[Repository](ctx, opts)
|
||||
}
|
||||
|
||||
// GetRepoAssignees returns all users that have write access and can be assigned to issues
|
||||
// of the repository,
|
||||
// GetRepoAssignees returns all users that have write access and can be assigned to issues or pull-requests of the repository,
|
||||
func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.User, err error) {
|
||||
if err = repo.LoadOwner(ctx); err != nil {
|
||||
return nil, err
|
||||
@@ -114,15 +114,9 @@ func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.Us
|
||||
uniqueUserIDs.AddMultiple(userIDs...)
|
||||
|
||||
if repo.Owner.IsOrganization() {
|
||||
additionalUserIDs := make([]int64, 0, 10)
|
||||
if err = e.Table("team_user").
|
||||
Join("INNER", "team_repo", "`team_repo`.team_id = `team_user`.team_id").
|
||||
Join("INNER", "team_unit", "`team_unit`.team_id = `team_user`.team_id").
|
||||
Where("`team_repo`.repo_id = ? AND (`team_unit`.access_mode >= ? OR (`team_unit`.access_mode = ? AND `team_unit`.`type` = ?))",
|
||||
repo.ID, perm.AccessModeWrite, perm.AccessModeRead, unit.TypePullRequests).
|
||||
Distinct("`team_user`.uid").
|
||||
Select("`team_user`.uid").
|
||||
Find(&additionalUserIDs); err != nil {
|
||||
// issues and pull requests both need "assignee list"
|
||||
additionalUserIDs, err := organization.GetTeamUserIDsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypeIssues, unit.TypePullRequests)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
uniqueUserIDs.AddMultiple(additionalUserIDs...)
|
||||
|
||||
@@ -6,7 +6,12 @@ package repo_test
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"code.gitea.io/gitea/models/db"
|
||||
"code.gitea.io/gitea/models/organization"
|
||||
perm_model "code.gitea.io/gitea/models/perm"
|
||||
access_model "code.gitea.io/gitea/models/perm/access"
|
||||
repo_model "code.gitea.io/gitea/models/repo"
|
||||
"code.gitea.io/gitea/models/unit"
|
||||
"code.gitea.io/gitea/models/unittest"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
|
||||
@@ -14,9 +19,14 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestRepoAssignees(t *testing.T) {
|
||||
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||
func TestUserRepo(t *testing.T) {
|
||||
require.NoError(t, unittest.PrepareTestDatabase())
|
||||
t.Run("GetIssuePostersWithSearch", testUserRepoGetIssuePostersWithSearch)
|
||||
t.Run("Assignees", testUserRepoAssignees)
|
||||
t.Run("AssigneesNoTeamUnit", testRepoAssigneesNoTeamUnit)
|
||||
}
|
||||
|
||||
func testUserRepoAssignees(t *testing.T) {
|
||||
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
|
||||
users, err := repo_model.GetRepoAssignees(t.Context(), repo2)
|
||||
assert.NoError(t, err)
|
||||
@@ -39,9 +49,29 @@ func TestRepoAssignees(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetIssuePostersWithSearch(t *testing.T) {
|
||||
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||
func testRepoAssigneesNoTeamUnit(t *testing.T) {
|
||||
ctx := t.Context()
|
||||
|
||||
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 32})
|
||||
require.NoError(t, repo.LoadOwner(ctx))
|
||||
require.True(t, repo.Owner.IsOrganization())
|
||||
|
||||
require.NoError(t, db.TruncateBeans(ctx, &organization.Team{}, &organization.TeamUser{}, &organization.TeamRepo{}, &organization.TeamUnit{}, &access_model.Access{}))
|
||||
|
||||
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
|
||||
team := &organization.Team{OrgID: repo.OwnerID, LowerName: "admin-team", AccessMode: perm_model.AccessModeAdmin}
|
||||
require.NoError(t, db.Insert(ctx, team))
|
||||
require.NoError(t, db.Insert(ctx, &organization.TeamUser{OrgID: repo.OwnerID, TeamID: team.ID, UID: user.ID}))
|
||||
require.NoError(t, db.Insert(ctx, &organization.TeamRepo{OrgID: repo.OwnerID, TeamID: team.ID, RepoID: repo.ID}))
|
||||
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: repo.OwnerID, TeamID: team.ID, Type: unit.TypePullRequests, AccessMode: perm_model.AccessModeNone}))
|
||||
|
||||
users, err := repo_model.GetRepoAssignees(ctx, repo)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, users, 1)
|
||||
assert.ElementsMatch(t, []int64{4}, []int64{users[0].ID})
|
||||
}
|
||||
|
||||
func testUserRepoGetIssuePostersWithSearch(t *testing.T) {
|
||||
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
|
||||
|
||||
users, err := repo_model.GetIssuePostersWithSearch(t.Context(), repo2, false, "USER")
|
||||
|
||||
@@ -11,7 +11,6 @@ import (
|
||||
|
||||
"code.gitea.io/gitea/models/db"
|
||||
issues_model "code.gitea.io/gitea/models/issues"
|
||||
org_model "code.gitea.io/gitea/models/organization"
|
||||
project_model "code.gitea.io/gitea/models/project"
|
||||
repo_model "code.gitea.io/gitea/models/repo"
|
||||
"code.gitea.io/gitea/models/unit"
|
||||
@@ -451,9 +450,9 @@ func ViewProject(ctx *context.Context) {
|
||||
ctx.Data["MilestoneID"] = milestoneID
|
||||
|
||||
// Get assignees.
|
||||
assigneeUsers, err := org_model.GetOrgAssignees(ctx, project.OwnerID)
|
||||
assigneeUsers, err := project_service.LoadIssuesAssigneesForProject(ctx, issuesMap)
|
||||
if err != nil {
|
||||
ctx.ServerError("GetRepoAssignees", err)
|
||||
ctx.ServerError("LoadIssuesAssigneesForProject", err)
|
||||
return
|
||||
}
|
||||
ctx.Data["Assignees"] = shared_user.MakeSelfOnTop(ctx.Doer, assigneeUsers)
|
||||
|
||||
@@ -6,11 +6,14 @@ package project
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"slices"
|
||||
"strings"
|
||||
|
||||
"code.gitea.io/gitea/models/db"
|
||||
issues_model "code.gitea.io/gitea/models/issues"
|
||||
project_model "code.gitea.io/gitea/models/project"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
"code.gitea.io/gitea/modules/container"
|
||||
"code.gitea.io/gitea/modules/optional"
|
||||
)
|
||||
|
||||
@@ -86,6 +89,31 @@ func MoveIssuesOnProjectColumn(ctx context.Context, doer *user_model.User, colum
|
||||
})
|
||||
}
|
||||
|
||||
func LoadIssuesAssigneesForProject(ctx context.Context, issuesMap map[int64]issues_model.IssueList) ([]*user_model.User, error) {
|
||||
var issueList issues_model.IssueList
|
||||
for _, colIssues := range issuesMap {
|
||||
issueList = append(issueList, colIssues...)
|
||||
}
|
||||
err := issueList.LoadAssignees(ctx)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
users := make([]*user_model.User, 0, len(issueList))
|
||||
usersAdded := container.Set[int64]{}
|
||||
for _, issue := range issueList {
|
||||
for _, assignee := range issue.Assignees {
|
||||
if !usersAdded.Contains(assignee.ID) {
|
||||
usersAdded.Add(assignee.ID)
|
||||
users = append(users, assignee)
|
||||
}
|
||||
}
|
||||
}
|
||||
slices.SortFunc(users, func(a, b *user_model.User) int {
|
||||
return strings.Compare(a.Name, b.Name)
|
||||
})
|
||||
return users, nil
|
||||
}
|
||||
|
||||
// LoadIssuesFromProject load issues assigned to each project column inside the given project
|
||||
func LoadIssuesFromProject(ctx context.Context, project *project_model.Project, opts *issues_model.IssuesOptions) (results map[int64]issues_model.IssueList, _ error) {
|
||||
issueList, err := issues_model.Issues(ctx, opts.Copy(func(o *issues_model.IssuesOptions) {
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func Test_Projects(t *testing.T) {
|
||||
@@ -207,4 +208,18 @@ func Test_Projects(t *testing.T) {
|
||||
assert.Len(t, columnIssues[3], 1)
|
||||
})
|
||||
})
|
||||
|
||||
t.Run("LoadIssuesAssigneesForProject", func(t *testing.T) {
|
||||
issuesMap := map[int64]issues_model.IssueList{}
|
||||
issue1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
|
||||
issue6 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 6})
|
||||
issuesMap[1] = issues_model.IssueList{issue1}
|
||||
issuesMap[2] = issues_model.IssueList{issue6}
|
||||
assignees, err := LoadIssuesAssigneesForProject(t.Context(), issuesMap)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, assignees, 3)
|
||||
require.Equal(t, "user1", assignees[0].Name)
|
||||
require.Equal(t, "user10", assignees[1].Name)
|
||||
require.Equal(t, "user2", assignees[2].Name)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -40,15 +40,8 @@ func GetReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post
|
||||
uniqueUserIDs.AddMultiple(collaboratorIDs...)
|
||||
|
||||
if repo.Owner.IsOrganization() {
|
||||
additionalUserIDs := make([]int64, 0, 10)
|
||||
if err := e.Table("team_user").
|
||||
Join("INNER", "team_repo", "`team_repo`.team_id = `team_user`.team_id").
|
||||
Join("INNER", "team_unit", "`team_unit`.team_id = `team_user`.team_id").
|
||||
Where("`team_repo`.repo_id = ? AND (`team_unit`.access_mode >= ? AND `team_unit`.`type` = ?)",
|
||||
repo.ID, perm.AccessModeRead, unit.TypePullRequests).
|
||||
Distinct("`team_user`.uid").
|
||||
Select("`team_user`.uid").
|
||||
Find(&additionalUserIDs); err != nil {
|
||||
additionalUserIDs, err := organization.GetTeamUserIDsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
uniqueUserIDs.AddMultiple(additionalUserIDs...)
|
||||
|
||||
Reference in New Issue
Block a user