Skip to content

move repository deletion to service layer #26948

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 13 commits into from
Sep 8, 2023
Merged
1 change: 1 addition & 0 deletions models/activities/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"code.gitea.io/gitea/models/unittest"

_ "code.gitea.io/gitea/models"
_ "code.gitea.io/gitea/models/actions"
)

func TestMain(m *testing.M) {
Expand Down
1 change: 1 addition & 0 deletions models/auth/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"code.gitea.io/gitea/models/unittest"

_ "code.gitea.io/gitea/models"
_ "code.gitea.io/gitea/models/actions"
_ "code.gitea.io/gitea/models/activities"
_ "code.gitea.io/gitea/models/auth"
_ "code.gitea.io/gitea/models/perm/access"
Expand Down
2 changes: 2 additions & 0 deletions models/git/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"code.gitea.io/gitea/models/unittest"

_ "code.gitea.io/gitea/models"
_ "code.gitea.io/gitea/models/actions"
_ "code.gitea.io/gitea/models/activities"
)

func TestMain(m *testing.M) {
Expand Down
2 changes: 2 additions & 0 deletions models/issues/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"code.gitea.io/gitea/models/unittest"

_ "code.gitea.io/gitea/models"
_ "code.gitea.io/gitea/models/actions"
_ "code.gitea.io/gitea/models/activities"
_ "code.gitea.io/gitea/models/repo"
_ "code.gitea.io/gitea/models/user"

Expand Down
1 change: 1 addition & 0 deletions models/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"

_ "code.gitea.io/gitea/models/actions"
_ "code.gitea.io/gitea/models/system"

"github.com/stretchr/testify/assert"
Expand Down
79 changes: 0 additions & 79 deletions models/org_team.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,85 +151,6 @@ func removeAllRepositories(ctx context.Context, t *organization.Team) (err error
return nil
}

// HasRepository returns true if given repository belong to team.
func HasRepository(t *organization.Team, repoID int64) bool {
return organization.HasTeamRepo(db.DefaultContext, t.OrgID, t.ID, repoID)
}

// removeRepository removes a repository from a team and recalculates access
// Note: Repository shall not be removed from team if it includes all repositories (unless the repository is deleted)
func removeRepository(ctx context.Context, t *organization.Team, repo *repo_model.Repository, recalculate bool) (err error) {
e := db.GetEngine(ctx)
if err = organization.RemoveTeamRepo(ctx, t.ID, repo.ID); err != nil {
return err
}

t.NumRepos--
if _, err = e.ID(t.ID).Cols("num_repos").Update(t); err != nil {
return err
}

// Don't need to recalculate when delete a repository from organization.
if recalculate {
if err = access_model.RecalculateTeamAccesses(ctx, repo, t.ID); err != nil {
return err
}
}

teamUsers, err := organization.GetTeamUsersByTeamID(ctx, t.ID)
if err != nil {
return fmt.Errorf("getTeamUsersByTeamID: %w", err)
}
for _, teamUser := range teamUsers {
has, err := access_model.HasAccess(ctx, teamUser.UID, repo)
if err != nil {
return err
} else if has {
continue
}

if err = repo_model.WatchRepo(ctx, teamUser.UID, repo.ID, false); err != nil {
return err
}

// Remove all IssueWatches a user has subscribed to in the repositories
if err := issues_model.RemoveIssueWatchersByRepoID(ctx, teamUser.UID, repo.ID); err != nil {
return err
}
}

return nil
}

// RemoveRepository removes repository from team of organization.
// If the team shall include all repositories the request is ignored.
func RemoveRepository(t *organization.Team, repoID int64) error {
if !HasRepository(t, repoID) {
return nil
}

if t.IncludesAllRepositories {
return nil
}

repo, err := repo_model.GetRepositoryByID(db.DefaultContext, repoID)
if err != nil {
return err
}

ctx, committer, err := db.TxContext(db.DefaultContext)
if err != nil {
return err
}
defer committer.Close()

if err = removeRepository(ctx, t, repo, true); err != nil {
return err
}

return committer.Commit()
}

// NewTeam creates a record of new team.
// It's caller's responsibility to assign organization ID.
func NewTeam(t *organization.Team) (err error) {
Expand Down
30 changes: 0 additions & 30 deletions models/org_team_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,36 +51,6 @@ func TestTeam_RemoveMember(t *testing.T) {
assert.True(t, organization.IsErrLastOrgOwner(err))
}

func TestTeam_HasRepository(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())

test := func(teamID, repoID int64, expected bool) {
team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: teamID})
assert.Equal(t, expected, HasRepository(team, repoID))
}
test(1, 1, false)
test(1, 3, true)
test(1, 5, true)
test(1, unittest.NonexistentID, false)

test(2, 3, true)
test(2, 5, false)
}

func TestTeam_RemoveRepository(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())

testSuccess := func(teamID, repoID int64) {
team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: teamID})
assert.NoError(t, RemoveRepository(team, repoID))
unittest.AssertNotExistsBean(t, &organization.TeamRepo{TeamID: teamID, RepoID: repoID})
unittest.CheckConsistencyFor(t, &organization.Team{ID: teamID}, &repo_model.Repository{ID: repoID})
}
testSuccess(2, 3)
testSuccess(2, 5)
testSuccess(1, unittest.NonexistentID)
}

func TestIsUsableTeamName(t *testing.T) {
assert.NoError(t, organization.IsUsableTeamName("usable"))
assert.True(t, db.IsErrNameReserved(organization.IsUsableTeamName("new")))
Expand Down
2 changes: 2 additions & 0 deletions models/organization/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"code.gitea.io/gitea/models/unittest"

_ "code.gitea.io/gitea/models"
_ "code.gitea.io/gitea/models/actions"
_ "code.gitea.io/gitea/models/activities"
_ "code.gitea.io/gitea/models/organization"
_ "code.gitea.io/gitea/models/repo"
_ "code.gitea.io/gitea/models/user"
Expand Down
2 changes: 2 additions & 0 deletions models/packages/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
user_model "code.gitea.io/gitea/models/user"

_ "code.gitea.io/gitea/models"
_ "code.gitea.io/gitea/models/actions"
_ "code.gitea.io/gitea/models/activities"

"github.com/stretchr/testify/assert"
)
Expand Down
2 changes: 2 additions & 0 deletions models/perm/access/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"code.gitea.io/gitea/models/unittest"

_ "code.gitea.io/gitea/models"
_ "code.gitea.io/gitea/models/actions"
_ "code.gitea.io/gitea/models/activities"
_ "code.gitea.io/gitea/models/repo"
_ "code.gitea.io/gitea/models/user"
)
Expand Down
Loading