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

Don't return duplicated users who can create org repo #22560

Merged
merged 11 commits into from
Jan 30, 2023
2 changes: 1 addition & 1 deletion models/activities/notification.go
Original file line number Diff line number Diff line change
@@ -151,7 +151,7 @@ func CreateRepoTransferNotification(ctx context.Context, doer, newOwner *user_mo
}
for i := range users {
notify = append(notify, &Notification{
UserID: users[i].ID,
UserID: i,
RepoID: repo.ID,
Status: NotificationStatusUnread,
UpdatedBy: doer.ID,
11 changes: 11 additions & 0 deletions models/fixtures/team.yml
Original file line number Diff line number Diff line change
@@ -140,3 +140,14 @@
num_members: 1
includes_all_repositories: false
can_create_org_repo: false

-
id: 14
org_id: 3
lower_name: teamcreaterepo
name: teamCreateRepo
authorize: 2 # write
num_repos: 0
num_members: 1
includes_all_repositories: false
can_create_org_repo: true
6 changes: 6 additions & 0 deletions models/fixtures/team_user.yml
Original file line number Diff line number Diff line change
@@ -93,3 +93,9 @@
org_id: 19
team_id: 6
uid: 31

-
id: 17
org_id: 3
team_id: 14
uid: 2
2 changes: 1 addition & 1 deletion models/fixtures/user.yml
Original file line number Diff line number Diff line change
@@ -104,7 +104,7 @@
num_following: 0
num_stars: 0
num_repos: 3
num_teams: 4
num_teams: 5
num_members: 3
visibility: 0
repo_admin_change_team_access: false
7 changes: 4 additions & 3 deletions models/organization/org.go
Original file line number Diff line number Diff line change
@@ -397,13 +397,14 @@ func (org *Organization) GetOrgUserMaxAuthorizeLevel(uid int64) (perm.AccessMode
}

// GetUsersWhoCanCreateOrgRepo returns users which are able to create repo in organization
func GetUsersWhoCanCreateOrgRepo(ctx context.Context, orgID int64) ([]*user_model.User, error) {
users := make([]*user_model.User, 0, 10)
func GetUsersWhoCanCreateOrgRepo(ctx context.Context, orgID int64) (map[int64]*user_model.User, error) {
Gusted marked this conversation as resolved.
Show resolved Hide resolved
// Use a map, in order to de-duplicate users.
users := make(map[int64]*user_model.User)
lunny marked this conversation as resolved.
Show resolved Hide resolved
return users, db.GetEngine(ctx).
Join("INNER", "`team_user`", "`team_user`.uid=`user`.id").
Join("INNER", "`team`", "`team`.id=`team_user`.team_id").
Where(builder.Eq{"team.can_create_org_repo": true}.Or(builder.Eq{"team.authorize": perm.AccessModeOwner})).
And("team_user.org_id = ?", orgID).Asc("`user`.name").Find(&users)
And("team_user.org_id = ?", orgID).Find(&users)
}

// SearchOrganizationsOptions options to filter organizations
9 changes: 5 additions & 4 deletions models/organization/org_test.go
Original file line number Diff line number Diff line change
@@ -91,11 +91,12 @@ func TestUser_GetTeams(t *testing.T) {
org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})
teams, err := org.LoadTeams()
assert.NoError(t, err)
if assert.Len(t, teams, 4) {
if assert.Len(t, teams, 5) {
assert.Equal(t, int64(1), teams[0].ID)
assert.Equal(t, int64(2), teams[1].ID)
assert.Equal(t, int64(12), teams[2].ID)
assert.Equal(t, int64(7), teams[3].ID)
assert.Equal(t, int64(14), teams[3].ID)
assert.Equal(t, int64(7), teams[4].ID)
}
}

@@ -292,7 +293,7 @@ func TestUser_GetUserTeamIDs(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, expected, teamIDs)
}
testSuccess(2, []int64{1, 2})
testSuccess(2, []int64{1, 2, 14})
testSuccess(4, []int64{2})
testSuccess(unittest.NonexistentID, []int64{})
}
@@ -447,7 +448,7 @@ func TestGetUsersWhoCanCreateOrgRepo(t *testing.T) {
users, err = organization.GetUsersWhoCanCreateOrgRepo(db.DefaultContext, 7)
assert.NoError(t, err)
assert.Len(t, users, 1)
assert.EqualValues(t, 5, users[0].ID)
assert.NotNil(t, users[5])
}

func TestUser_RemoveOrgRepo(t *testing.T) {