Skip to content

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

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

Merged
merged 3 commits into from
Jan 30, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion models/activities/notification.go
Original file line number Diff line number Diff line change
@@ -157,7 +157,7 @@ func CreateRepoTransferNotification(doer, newOwner *user_model.User, repo *repo_
}
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
@@ -396,13 +396,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) {
// Use a map, in order to de-duplicate users.
users := make(map[int64]*user_model.User)
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
@@ -92,11 +92,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)
}
}

@@ -293,7 +294,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{})
}
@@ -448,7 +449,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) {