chore: split AddRepository and AddTeamMember to return the inserted value (#11342)

Split [AddRepository][0] and [AddTeamMember][1] to functions that return the inserted value. These can be used by the F3 driver to obtain the ID of the TeamRepo and TeamUser rows inserted in the database.

Add test coverage for both functions and remove a duplicated test. Integration tests also already [make use of AddRepository and AddTeamMember][2] indirectly.

[0]: f8bee35e77/models/org_team.go (L26)
[1]: f8bee35e77/models/org_team.go (L359)
[2]: f8bee35e77/tests/integration/api_helper_for_declarative_test.go (L461-L483)

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11342
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: limiting-factor <limiting-factor@posteo.com>
Co-committed-by: limiting-factor <limiting-factor@posteo.com>
This commit is contained in:
limiting-factor 2026-03-11 03:40:32 +01:00 committed by Gusted
parent a581059606
commit d2acd4f14c
3 changed files with 125 additions and 42 deletions

View file

@ -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 {
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()
}
// 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 {
isAlreadyMember, err := organization.IsTeamMember(ctx, team.OrgID, team.ID, userID)
if err != nil || isAlreadyMember {
_, 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 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 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 {

View file

@ -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())

View file

@ -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{