diff --git a/models/org_team.go b/models/org_team.go index de54c61745..187ea1560d 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -23,34 +23,44 @@ import ( "xorm.io/builder" ) -func AddRepository(ctx context.Context, t *organization.Team, repo *repo_model.Repository) (err error) { - if err = organization.AddTeamRepo(ctx, t.OrgID, t.ID, repo.ID); err != nil { - return err +func AddRepository(ctx context.Context, t *organization.Team, repo *repo_model.Repository) error { + _, err := InsertTeamRepository(ctx, t, repo) + return err +} + +func InsertTeamRepository(ctx context.Context, t *organization.Team, repo *repo_model.Repository) (teamRepo *organization.TeamRepo, err error) { + teamRepo = &organization.TeamRepo{ + OrgID: t.OrgID, + TeamID: t.ID, + RepoID: repo.ID, + } + if _, err = db.GetEngine(ctx).Insert(teamRepo); err != nil { + return nil, err } if err = organization.IncrTeamRepoNum(ctx, t.ID); err != nil { - return fmt.Errorf("update team: %w", err) + return nil, fmt.Errorf("update team: %w", err) } t.NumRepos++ if err = access_model.RecalculateTeamAccesses(ctx, repo, 0); err != nil { - return fmt.Errorf("recalculateAccesses: %w", err) + return nil, fmt.Errorf("recalculateAccesses: %w", err) } // Make all team members watch this repo if enabled in global settings if setting.Service.AutoWatchNewRepos { if err = t.LoadMembers(ctx); err != nil { - return fmt.Errorf("getMembers: %w", err) + return nil, fmt.Errorf("getMembers: %w", err) } for _, u := range t.Members { if err = repo_model.WatchRepo(ctx, u.ID, repo.ID, true); err != nil { - return fmt.Errorf("watchRepo: %w", err) + return nil, fmt.Errorf("watchRepo: %w", err) } } } - return nil + return teamRepo, nil } // addAllRepositories adds all repositories to the team. @@ -354,16 +364,27 @@ func DeleteTeam(ctx context.Context, t *organization.Team) error { return committer.Commit() } +func AddTeamMember(ctx context.Context, team *organization.Team, userID int64) error { + _, err := InsertTeamMember(ctx, team, userID) + return err +} + // AddTeamMember adds new membership of given team to given organization, // the user will have membership to given organization automatically when needed. -func AddTeamMember(ctx context.Context, team *organization.Team, userID int64) error { +func InsertTeamMember(ctx context.Context, team *organization.Team, userID int64) (*organization.TeamUser, error) { isAlreadyMember, err := organization.IsTeamMember(ctx, team.OrgID, team.ID, userID) if err != nil || isAlreadyMember { - return err + return nil, err } if err := organization.AddOrgUser(ctx, team.OrgID, userID); err != nil { - return err + return nil, err + } + + teamUser := &organization.TeamUser{ + UID: userID, + OrgID: team.OrgID, + TeamID: team.ID, } err = db.WithTx(ctx, func(ctx context.Context) error { @@ -375,11 +396,7 @@ func AddTeamMember(ctx context.Context, team *organization.Team, userID int64) e sess := db.GetEngine(ctx) - if err := db.Insert(ctx, &organization.TeamUser{ - UID: userID, - OrgID: team.OrgID, - TeamID: team.ID, - }); err != nil { + if err := db.Insert(ctx, teamUser); err != nil { return err } else if _, err := sess.Incr("num_members").ID(team.ID).Update(new(organization.Team)); err != nil { return err @@ -420,7 +437,7 @@ func AddTeamMember(ctx context.Context, team *organization.Team, userID int64) e return nil }) if err != nil { - return err + return nil, err } // this behaviour may spend much time so run it in a goroutine @@ -440,7 +457,7 @@ func AddTeamMember(ctx context.Context, team *organization.Team, userID int64) e }(team.Repos) } - return nil + return teamUser, nil } func removeTeamMember(ctx context.Context, team *organization.Team, userID int64) error { diff --git a/models/org_team_test.go b/models/org_team_test.go index 730bc65f7d..5f7544e9ea 100644 --- a/models/org_team_test.go +++ b/models/org_team_test.go @@ -19,20 +19,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestTeam_AddMember(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - - test := func(teamID, userID int64) { - team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: teamID}) - require.NoError(t, AddTeamMember(db.DefaultContext, team, userID)) - unittest.AssertExistsAndLoadBean(t, &organization.TeamUser{UID: userID, TeamID: teamID}) - unittest.CheckConsistencyFor(t, &organization.Team{ID: teamID}, &user_model.User{ID: team.OrgID}) - } - test(1, 2) - test(1, 4) - test(3, 2) -} - func TestTeam_RemoveMember(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) @@ -132,6 +118,96 @@ func TestAddTeamMember(t *testing.T) { test(3, 2) } +func TestTeam_AddAndReturnTeamMember(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + for _, testCase := range []struct { + name string + alreadyMember bool + teamID int64 + userID int64 + }{ + { + name: "Already member of a team with repositories", + alreadyMember: true, + teamID: 1, + userID: 2, + }, + { + name: "New member of a team with repositories", + teamID: 1, + userID: 4, + }, + { + name: "New member of a team with no repositories", + teamID: 3, + userID: 2, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: testCase.teamID}) + teamUser, err := InsertTeamMember(db.DefaultContext, team, testCase.userID) + require.NoError(t, err) + if testCase.alreadyMember { + assert.Nil(t, teamUser) + } else { + require.NotNil(t, teamUser) + assert.Equal(t, testCase.teamID, teamUser.TeamID) + assert.Equal(t, testCase.userID, teamUser.UID) + } + unittest.AssertExistsAndLoadBean(t, &organization.TeamUser{UID: testCase.userID, TeamID: testCase.teamID}) + unittest.CheckConsistencyFor(t, &organization.Team{ID: testCase.teamID}, &user_model.User{ID: team.OrgID}) + }) + } +} + +func TestTeam_AddTeamRepository(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + for _, testCase := range []struct { + name string + teamID int64 + repoID int64 + }{ + { + name: "AddAndReturnTeamRepository", + teamID: 8, + repoID: 23, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: testCase.teamID}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: testCase.repoID}) + teamRepo, err := InsertTeamRepository(t.Context(), team, repo) + require.NoError(t, err) + require.NotNil(t, teamRepo) + assert.Equal(t, testCase.teamID, teamRepo.TeamID) + assert.Equal(t, testCase.repoID, teamRepo.RepoID) + unittest.AssertExistsAndLoadBean(t, &organization.TeamRepo{RepoID: testCase.repoID, TeamID: testCase.teamID}) + }) + } + + for _, testCase := range []struct { + name string + teamID int64 + repoID int64 + }{ + { + name: "AddTeamRepository", + teamID: 9, + repoID: 23, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: testCase.teamID}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: testCase.repoID}) + err := AddRepository(t.Context(), team, repo) + require.NoError(t, err) + unittest.AssertExistsAndLoadBean(t, &organization.TeamRepo{RepoID: testCase.repoID, TeamID: testCase.teamID}) + }) + } +} + func TestRemoveTeamMember(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) diff --git a/models/organization/team_repo.go b/models/organization/team_repo.go index 9331537e19..895d5a56d0 100644 --- a/models/organization/team_repo.go +++ b/models/organization/team_repo.go @@ -59,16 +59,6 @@ func GetTeamRepositories(ctx context.Context, opts *SearchTeamRepoOptions) (repo Find(&repos) } -// AddTeamRepo adds a repo for an organization's team -func AddTeamRepo(ctx context.Context, orgID, teamID, repoID int64) error { - _, err := db.GetEngine(ctx).Insert(&TeamRepo{ - OrgID: orgID, - TeamID: teamID, - RepoID: repoID, - }) - return err -} - // RemoveTeamRepo remove repository from team func RemoveTeamRepo(ctx context.Context, teamID, repoID int64) error { _, err := db.DeleteByBean(ctx, &TeamRepo{