diff --git a/modelmigration/migrations.go b/modelmigration/migrations.go index 1cc95bfca36..b83084ac810 100644 --- a/modelmigration/migrations.go +++ b/modelmigration/migrations.go @@ -444,6 +444,7 @@ func prepareMigrationTasks() []*migration { newMigration(365, "Normalize legacy repository unit and login source JSON", v29.NormalizeLegacyConfigJSON), newMigration(366, "Drop user_open_id table", v29.DropUserOpenIDTable), newMigration(367, "Add latest_release_id to repository table", v29.AddLatestReleaseIDToRepository), + newMigration(368, "Add unique index for team name", v29.AddUniqueTeamNameIndex), } return preparedMigrations } diff --git a/modelmigration/v29/v368.go b/modelmigration/v29/v368.go new file mode 100644 index 00000000000..ff541a9186c --- /dev/null +++ b/modelmigration/v29/v368.go @@ -0,0 +1,50 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v29 + +import ( + "context" + "fmt" + "strings" + + "gitea.dev/modelmigration/base" + "gitea.dev/modules/log" + "gitea.dev/modules/util" + + "xorm.io/xorm" +) + +func AddUniqueTeamNameIndex(_ctx context.Context, x base.EngineMigration) error { + type Team struct { + ID int64 `xorm:"pk autoincr"` + OrgID int64 `xorm:"INDEX UNIQUE(s)"` + LowerName string `xorm:"UNIQUE(s)"` + Name string + } + + var teams []Team + if err := x.Where("id NOT IN (SELECT MIN(id) FROM team GROUP BY org_id, lower_name)").Asc("id").Find(&teams); err != nil { + return err + } + for _, team := range teams { + for suffix := team.ID; ; suffix++ { + ending := fmt.Sprintf("-%d", suffix) + name := util.TruncateStringBytes(team.Name, 255-len(ending)) + ending + exists, err := x.Where("org_id = ? AND lower_name = ?", team.OrgID, strings.ToLower(name)).Exist(new(Team)) + if err != nil { + return err + } + if !exists { + if _, err := x.ID(team.ID).Cols("name", "lower_name").Update(&Team{Name: name, LowerName: strings.ToLower(name)}); err != nil { + return err + } + log.Warn("renamed duplicate team id=%d in org id=%d from %q to %q", team.ID, team.OrgID, team.Name, name) + break + } + } + } + + _, err := x.SyncWithOptions(xorm.SyncOptions{IgnoreDropIndices: true}, new(Team)) + return err +} diff --git a/modelmigration/v29/v368_test.go b/modelmigration/v29/v368_test.go new file mode 100644 index 00000000000..b14a14eb0e1 --- /dev/null +++ b/modelmigration/v29/v368_test.go @@ -0,0 +1,52 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v29 + +import ( + "strings" + "testing" + + "gitea.dev/modelmigration/migrationtest" + + "github.com/stretchr/testify/require" +) + +func TestAddUniqueTeamNameIndex(t *testing.T) { + type Team struct { + ID int64 `xorm:"pk autoincr"` + OrgID int64 `xorm:"INDEX"` + LowerName, Name string + } + + x, cleanup := migrationtest.PrepareTestEnv(t, 0, new(Team)) + defer cleanup() + if x == nil || t.Failed() { + return + } + + _, err := x.Insert( + &Team{OrgID: 1, Name: "Team", LowerName: "team"}, + &Team{OrgID: 1, Name: "Team", LowerName: "team"}, + &Team{OrgID: 1, Name: "Team-2", LowerName: "team-2"}, + &Team{OrgID: 2, Name: "Team", LowerName: "team"}, + &Team{OrgID: 1, Name: strings.Repeat("A", 255), LowerName: strings.Repeat("a", 255)}, + &Team{OrgID: 1, Name: strings.Repeat("A", 255), LowerName: strings.Repeat("a", 255)}, + ) + require.NoError(t, err) + for range 2 { + require.NoError(t, AddUniqueTeamNameIndex(t.Context(), x)) + var teams []Team + require.NoError(t, x.Asc("id").Find(&teams)) + require.Equal(t, []Team{ + {ID: 1, OrgID: 1, Name: "Team", LowerName: "team"}, + {ID: 2, OrgID: 1, Name: "Team-3", LowerName: "team-3"}, + {ID: 3, OrgID: 1, Name: "Team-2", LowerName: "team-2"}, + {ID: 4, OrgID: 2, Name: "Team", LowerName: "team"}, + {ID: 5, OrgID: 1, Name: strings.Repeat("A", 255), LowerName: strings.Repeat("a", 255)}, + {ID: 6, OrgID: 1, Name: strings.Repeat("A", 253) + "-6", LowerName: strings.Repeat("a", 253) + "-6"}, + }, teams) + } + _, err = x.Insert(&Team{OrgID: 1, Name: "TEAM", LowerName: "team"}) + require.Error(t, err) +} diff --git a/models/organization/team.go b/models/organization/team.go index d07a28c2452..10a444372f4 100644 --- a/models/organization/team.go +++ b/models/organization/team.go @@ -74,9 +74,9 @@ const OwnerTeamName = "Owners" // Team represents a organization team. type Team struct { - ID int64 `xorm:"pk autoincr"` - OrgID int64 `xorm:"INDEX"` - LowerName string + ID int64 `xorm:"pk autoincr"` + OrgID int64 `xorm:"INDEX UNIQUE(s)"` + LowerName string `xorm:"UNIQUE(s)"` Name string Description string AccessMode perm.AccessMode `xorm:"'authorize'"` diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index 8bd00976f2b..9fbab29092e 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -225,11 +225,7 @@ func CreateTeam(ctx *context.APIContext) { } if err := org_service.NewTeam(ctx, team); err != nil { - if organization.IsErrTeamAlreadyExist(err) { - ctx.APIError(http.StatusUnprocessableEntity, err.Error()) - } else { - ctx.APIErrorInternal(err) - } + ctx.APIErrorAuto(err) return } @@ -307,7 +303,7 @@ func EditTeam(ctx *context.APIContext) { } if err := org_service.UpdateTeam(ctx, team, isAuthChanged, isIncludeAllChanged); err != nil { - ctx.APIErrorInternal(err) + ctx.APIErrorAuto(err) return } diff --git a/services/org/team.go b/services/org/team.go index 0a2d2a0d68f..37cb482b0ee 100644 --- a/services/org/team.go +++ b/services/org/team.go @@ -38,6 +38,13 @@ func recordTeamMemberAudit(ctx context.Context, action audit_model.Action, team recordTeamAudit(ctx, action, team, "team", team.Name, "member", member.Name) } +func teamWriteError(ctx context.Context, team *organization.Team, err error) error { + if existing, _ := organization.GetTeam(ctx, team.OrgID, team.LowerName); existing != nil && existing.ID != team.ID { + return organization.ErrTeamAlreadyExist{OrgID: team.OrgID, Name: team.LowerName} + } + return err +} + // NewTeam creates a record of new team. // It's caller's responsibility to assign organization ID. func NewTeam(ctx context.Context, t *organization.Team) (err error) { @@ -58,17 +65,6 @@ func NewTeam(ctx context.Context, t *organization.Team) (err error) { } t.LowerName = strings.ToLower(t.Name) - has, err = db.Exist[organization.Team](ctx, builder.Eq{ - "org_id": t.OrgID, - "lower_name": t.LowerName, - }) - if err != nil { - return err - } - if has { - return organization.ErrTeamAlreadyExist{OrgID: t.OrgID, Name: t.LowerName} - } - if err = db.WithTx(ctx, func(ctx context.Context) error { if err = db.Insert(ctx, t); err != nil { return err @@ -96,7 +92,7 @@ func NewTeam(ctx context.Context, t *organization.Team) (err error) { _, err = db.Exec(ctx, "UPDATE `user` SET num_teams=num_teams+1 WHERE id = ?", t.OrgID) return err }); err != nil { - return err + return teamWriteError(ctx, t, err) } recordTeamAudit(ctx, audit_model.OrganizationTeamAdd, t, "team", t.Name) @@ -114,17 +110,6 @@ func UpdateTeam(ctx context.Context, t *organization.Team, authChanged, includeA if err = db.WithTx(ctx, func(ctx context.Context) error { t.LowerName = strings.ToLower(t.Name) - has, err := db.Exist[organization.Team](ctx, builder.Eq{ - "org_id": t.OrgID, - "lower_name": t.LowerName, - }.And(builder.Neq{"id": t.ID}), - ) - if err != nil { - return err - } else if has { - return organization.ErrTeamAlreadyExist{OrgID: t.OrgID, Name: t.LowerName} - } - sess := db.GetEngine(ctx) if _, err = sess.ID(t.ID).Cols("name", "lower_name", "description", "can_create_org_repo", "authorize", "includes_all_repositories", "visibility").Update(t); err != nil { @@ -173,7 +158,7 @@ func UpdateTeam(ctx context.Context, t *organization.Team, authChanged, includeA return nil }); err != nil { - return err + return teamWriteError(ctx, t, err) } recordTeamAudit(ctx, audit_model.OrganizationTeamUpdate, t, "team", t.Name) diff --git a/tests/integration/api_team_test.go b/tests/integration/api_team_test.go index be34f2f682e..cd4f70d9702 100644 --- a/tests/integration/api_team_test.go +++ b/tests/integration/api_team_test.go @@ -126,6 +126,9 @@ func TestAPITeam(t *testing.T) { checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, api.AccessLevelNameNone, expectedTeamUnitsMap) teamID := apiTeam.ID + MakeRequest(t, NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/orgs/%s/teams", org.Name), teamToCreate).AddTokenAuth(token), http.StatusConflict) + MakeRequest(t, NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d", teamID), api.EditTeamOption{Name: "team13NotCreators"}).AddTokenAuth(token), http.StatusConflict) + // Edit team. editDescription := "team 1" editFalse := false